Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ message HttpConnectionManager {
UNESCAPE_AND_FORWARD = 4;
}

// [#next-free-field: 10]
// [#next-free-field: 11]
message Tracing {
option (udpa.annotations.versioning).previous_message_type =
"envoy.config.filter.network.http_connection_manager.v2.HttpConnectionManager.Tracing";
Expand Down Expand Up @@ -193,6 +193,10 @@ message HttpConnectionManager {
// Such a constraint is inherent to OpenCensus itself. It cannot be overcome without changes
// on OpenCensus side.
config.trace.v3.Tracing.Http provider = 9;

// Trace reason is decided by request_id by default. If true, these sampling strategy is delegated
// to the configured tracing provider.
bool delegate_sampling = 10;
Comment thread
Shikugawa marked this conversation as resolved.
Outdated
}

message InternalAddressConfig {
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions docs/root/version_history/current.rst
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ Minor Behavior Changes
* listener: respect the :ref:`connection balance config <envoy_v3_api_field_config.listener.v3.Listener.connection_balance_config>`
defined within the listener where the sockets are redirected to. Clear that field to restore the previous behavior.
* tcp: switched to the new connection pool by default. Any unexpected behavioral changes can be reverted by setting runtime guard ``envoy.reloadable_features.new_tcp_connection_pool`` to false.
* tracing: add :ref:`delegate_sampling <envoy_v3_api_field_extensions.filters.network.http_connection_manager.v3.HttpConnectionManager.Tracing.delegate_sampling>` to bypass current UUID based sampling decision behavior.

Bug Fixes
---------
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions include/envoy/tracing/trace_driver.h
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,11 @@ class Config {
* for HTTP protocol tracing.
*/
virtual uint32_t maxPathTagLength() const PURE;

/**
* @return true if trace reason won't be decided by request_id value.
*/
virtual bool delegateSampling() const PURE;
};

/**
Expand Down
1 change: 1 addition & 0 deletions source/common/http/conn_manager_config.h
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ struct TracingConnectionManagerConfig {
envoy::type::v3::FractionalPercent overall_sampling_;
bool verbose_;
uint32_t max_path_tag_length_;
bool delegate_sampling_;
};

using TracingConnectionManagerConfigPtr = std::unique_ptr<TracingConnectionManagerConfig>;
Expand Down
4 changes: 4 additions & 0 deletions source/common/http/conn_manager_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1585,6 +1585,10 @@ uint32_t ConnectionManagerImpl::ActiveStream::maxPathTagLength() const {
return connection_manager_.config_.tracingConfig()->max_path_tag_length_;
}

bool ConnectionManagerImpl::ActiveStream::delegateSampling() const {
return connection_manager_.config_.tracingConfig()->delegate_sampling_;
}

const Router::RouteEntry::UpgradeMap* ConnectionManagerImpl::ActiveStream::upgradeMap() {
// We must check if the 'cached_route_' optional is populated since this function can be called
// early via sendLocalReply(), before the cached route is populated.
Expand Down
1 change: 1 addition & 0 deletions source/common/http/conn_manager_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,7 @@ class ConnectionManagerImpl : Logger::Loggable<Logger::Id::http>,
const Tracing::CustomTagMap* customTags() const override;
bool verbose() const override;
uint32_t maxPathTagLength() const override;
bool delegateSampling() const override;

// ScopeTrackedObject
void dumpState(std::ostream& os, int indent_level = 0) const override {
Expand Down
4 changes: 4 additions & 0 deletions source/common/http/conn_manager_utility.cc
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,10 @@ Tracing::Reason ConnectionManagerUtility::mutateTracingRequestHeader(
if (!config.tracingConfig()) {
return final_reason;
}
if (config.tracingConfig()->delegate_sampling_) {
final_reason = Tracing::Reason::Sampling;
return final_reason;
Comment thread
Shikugawa marked this conversation as resolved.
Outdated
}

auto rid_extension = config.requestIDExtension();
const auto rid_to_integer = rid_extension->toInteger(request_headers);
Expand Down
1 change: 1 addition & 0 deletions source/common/tracing/http_tracer_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ class EgressConfigImpl : public Config {
const CustomTagMap* customTags() const override { return nullptr; }
bool verbose() const override { return false; }
uint32_t maxPathTagLength() const override { return Tracing::DefaultMaxPathTagLength; }
bool delegateSampling() const override { return false; }
};

using EgressConfig = ConstSingleton<EgressConfigImpl>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -507,7 +507,7 @@ HttpConnectionManagerConfig::HttpConnectionManagerConfig(
tracing_config_ =
std::make_unique<Http::TracingConnectionManagerConfig>(Http::TracingConnectionManagerConfig{
tracing_operation_name, custom_tags, client_sampling, random_sampling, overall_sampling,
tracing_config.verbose(), max_path_tag_length});
tracing_config.verbose(), max_path_tag_length, tracing_config.delegate_sampling()});
}

for (const auto& access_log : config.access_log()) {
Expand Down
14 changes: 9 additions & 5 deletions test/common/http/conn_manager_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1350,7 +1350,7 @@ TEST_F(HttpConnectionManagerImplTest, StartAndFinishSpanNormalFlow) {
}
tracing_config_ = std::make_unique<TracingConnectionManagerConfig>(
TracingConnectionManagerConfig{Tracing::OperationName::Ingress, conn_tracing_tags, percent1,
percent2, percent1, false, 256});
percent2, percent1, false, 256, false});
NiceMock<Router::MockRouteTracing> route_tracing;
ON_CALL(route_tracing, getClientSampling()).WillByDefault(ReturnRef(percent1));
ON_CALL(route_tracing, getRandomSampling()).WillByDefault(ReturnRef(percent2));
Expand Down Expand Up @@ -1632,7 +1632,8 @@ TEST_F(HttpConnectionManagerImplTest, StartAndFinishSpanNormalFlowEgressDecorato
percent2,
percent1,
false,
256});
256,
false});

auto* span = new NiceMock<Tracing::MockSpan>();
EXPECT_CALL(*tracer_, startSpan_(_, _, _, _))
Expand Down Expand Up @@ -1714,7 +1715,8 @@ TEST_F(HttpConnectionManagerImplTest, StartAndFinishSpanNormalFlowEgressDecorato
percent2,
percent1,
false,
256});
256,
false});

auto* span = new NiceMock<Tracing::MockSpan>();
EXPECT_CALL(*tracer_, startSpan_(_, _, _, _))
Expand Down Expand Up @@ -1798,7 +1800,8 @@ TEST_F(HttpConnectionManagerImplTest, StartAndFinishSpanNormalFlowEgressDecorato
percent2,
percent1,
false,
256});
256,
false});

auto* span = new NiceMock<Tracing::MockSpan>();
EXPECT_CALL(*tracer_, startSpan_(_, _, _, _))
Expand Down Expand Up @@ -1874,7 +1877,8 @@ TEST_F(HttpConnectionManagerImplTest,
percent2,
percent1,
false,
256});
256,
false});

EXPECT_CALL(
runtime_.snapshot_,
Expand Down
3 changes: 2 additions & 1 deletion test/common/http/conn_manager_impl_test_base.cc
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,8 @@ void HttpConnectionManagerImplTest::setup(bool ssl, const std::string& server_na
percent2,
percent1,
false,
256});
256,
false});
}
}

Expand Down
11 changes: 10 additions & 1 deletion test/common/http/conn_manager_utility_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ class ConnectionManagerUtilityTest : public testing::Test {
percent2.set_numerator(10000);
percent2.set_denominator(envoy::type::v3::FractionalPercent::TEN_THOUSAND);
tracing_config_ = {
Tracing::OperationName::Ingress, {}, percent1, percent2, percent1, false, 256};
Tracing::OperationName::Ingress, {}, percent1, percent2, percent1, false, 256, false};
ON_CALL(config_, tracingConfig()).WillByDefault(Return(&tracing_config_));
ON_CALL(config_, localReply()).WillByDefault(ReturnRef(*local_reply_));

Expand Down Expand Up @@ -1230,6 +1230,15 @@ TEST_F(ConnectionManagerUtilityTest, SamplingWithoutRouteOverride) {
EXPECT_EQ(Tracing::Reason::Sampling, request_id_extension_->getTraceReason(request_headers));
}

TEST_F(ConnectionManagerUtilityTest, DelegateSamplingLogicToTracingProvider) {
tracing_config_.delegate_sampling_ = true;

Http::TestRequestHeaderMapImpl request_headers{
{"x-request-id", "125a4afb-6f55-44ba-ad80-413f09f48a28"}};
const auto ret = callMutateRequestHeaders(request_headers, Protocol::Http2);
EXPECT_EQ(Tracing::Reason::Sampling, ret.trace_reason_);
}

TEST_F(ConnectionManagerUtilityTest, SamplingWithRouteOverride) {
EXPECT_CALL(
runtime_.snapshot_,
Expand Down
1 change: 1 addition & 0 deletions test/mocks/tracing/mocks.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ MockConfig::MockConfig() {
ON_CALL(*this, customTags()).WillByDefault(Return(&custom_tags_));
ON_CALL(*this, verbose()).WillByDefault(Return(verbose_));
ON_CALL(*this, maxPathTagLength()).WillByDefault(Return(uint32_t(256)));
ON_CALL(*this, delegateSampling()).WillByDefault(Return(false));
}
MockConfig::~MockConfig() = default;

Expand Down
1 change: 1 addition & 0 deletions test/mocks/tracing/mocks.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ class MockConfig : public Config {
MOCK_METHOD(const CustomTagMap*, customTags, (), (const));
MOCK_METHOD(bool, verbose, (), (const));
MOCK_METHOD(uint32_t, maxPathTagLength, (), (const));
MOCK_METHOD(bool, delegateSampling, (), (const));

OperationName operation_name_{OperationName::Ingress};
CustomTagMap custom_tags_;
Expand Down