diff --git a/api/envoy/extensions/request_id/uuid/v3/uuid.proto b/api/envoy/extensions/request_id/uuid/v3/uuid.proto index 9b8b7a45f5607..5c3f00da28d71 100644 --- a/api/envoy/extensions/request_id/uuid/v3/uuid.proto +++ b/api/envoy/extensions/request_id/uuid/v3/uuid.proto @@ -40,4 +40,9 @@ message UuidRequestIdConfig { // stable sampling of traces, access logs, etc. will no longer work and only random sampling will // be possible. google.protobuf.BoolValue pack_trace_reason = 1; + + // Set whether to use :ref:`x-request-id` for sampling or not. + // This defaults to true. See the :ref:`context propagation ` + // overview for more information. + google.protobuf.BoolValue use_request_id_for_trace_sampling = 2; } diff --git a/docs/root/intro/arch_overview/observability/tracing.rst b/docs/root/intro/arch_overview/observability/tracing.rst index ed717e077522a..ece8d4e8b807c 100644 --- a/docs/root/intro/arch_overview/observability/tracing.rst +++ b/docs/root/intro/arch_overview/observability/tracing.rst @@ -69,6 +69,16 @@ to be correlated. field can be used to disable this behavior at the expense of also disabling stable trace reason propagation and associated features within a deployment. +.. attention:: + + The sampling policy for Envoy is determined by the value of :ref:`x-request-id ` by default. + However, such a sampling policy is only valid for a fleet of Envoys. If a service proxy + that is not Envoy is present in the fleet, sampling is performed without considering the policy of that proxy. + For meshes consisting of multiple service proxies such as this, it is more effective to + bypass Envoy's sampling policy and sample based on the trace provider's sampling policy. This can be achieved by setting + :ref:`use_request_id_for_trace_sampling ` + to false. + The tracing providers also require additional context, to enable the parent/child relationships between the spans (logical units of work) to be understood. This can be achieved by using the LightStep (via OpenTracing API) or Zipkin tracer directly within the service itself, to extract the diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index 3e88b63f3c90b..e314d9b41289e 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -36,6 +36,7 @@ Minor Behavior Changes * listener: respect the :ref:`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 option :ref:`use_request_id_for_trace_sampling ` whether to use sampling policy based on :ref:`x-request-id` or not. Bug Fixes --------- diff --git a/envoy/http/request_id_extension.h b/envoy/http/request_id_extension.h index 8c2faf9e3a027..efdfd1b8cbec2 100644 --- a/envoy/http/request_id_extension.h +++ b/envoy/http/request_id_extension.h @@ -63,6 +63,12 @@ class RequestIDExtension : public RequestIdStreamInfoProvider { * @param status the trace reason that should be set for this request. */ virtual void setTraceReason(Http::RequestHeaderMap& request_headers, Tracing::Reason reason) PURE; + + /** + * Get whether to use request_id based sampling policy or not. + * @return whether to use request_id based sampling policy or not. + */ + virtual bool useRequestIdForTraceSampling() const PURE; }; using RequestIDExtensionSharedPtr = std::shared_ptr; diff --git a/generated_api_shadow/envoy/extensions/request_id/uuid/v3/uuid.proto b/generated_api_shadow/envoy/extensions/request_id/uuid/v3/uuid.proto index 9b8b7a45f5607..5c3f00da28d71 100644 --- a/generated_api_shadow/envoy/extensions/request_id/uuid/v3/uuid.proto +++ b/generated_api_shadow/envoy/extensions/request_id/uuid/v3/uuid.proto @@ -40,4 +40,9 @@ message UuidRequestIdConfig { // stable sampling of traces, access logs, etc. will no longer work and only random sampling will // be possible. google.protobuf.BoolValue pack_trace_reason = 1; + + // Set whether to use :ref:`x-request-id` for sampling or not. + // This defaults to true. See the :ref:`context propagation ` + // overview for more information. + google.protobuf.BoolValue use_request_id_for_trace_sampling = 2; } diff --git a/source/common/http/conn_manager_utility.cc b/source/common/http/conn_manager_utility.cc index 4c0bddcd6a61a..048a5ff088e96 100644 --- a/source/common/http/conn_manager_utility.cc +++ b/source/common/http/conn_manager_utility.cc @@ -283,6 +283,9 @@ Tracing::Reason ConnectionManagerUtility::mutateTracingRequestHeader( } auto rid_extension = config.requestIDExtension(); + if (!rid_extension->useRequestIdForTraceSampling()) { + return Tracing::Reason::Sampling; + } const auto rid_to_integer = rid_extension->toInteger(request_headers); // Skip if request-id is corrupted, or non-existent if (!rid_to_integer.has_value()) { diff --git a/source/extensions/request_id/uuid/config.h b/source/extensions/request_id/uuid/config.h index bd21f9eaa6bc2..61cb67a5c5ea7 100644 --- a/source/extensions/request_id/uuid/config.h +++ b/source/extensions/request_id/uuid/config.h @@ -16,7 +16,9 @@ class UUIDRequestIDExtension : public Http::RequestIDExtension { UUIDRequestIDExtension(const envoy::extensions::request_id::uuid::v3::UuidRequestIdConfig& config, Random::RandomGenerator& random) : random_(random), - pack_trace_reason_(PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, pack_trace_reason, true)) {} + pack_trace_reason_(PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, pack_trace_reason, true)), + use_request_id_for_trace_sampling_( + PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, use_request_id_for_trace_sampling, true)) {} static Http::RequestIDExtensionSharedPtr defaultInstance(Random::RandomGenerator& random) { return std::make_shared( @@ -32,10 +34,12 @@ class UUIDRequestIDExtension : public Http::RequestIDExtension { absl::optional toInteger(const Http::RequestHeaderMap& request_headers) const override; Tracing::Reason getTraceReason(const Http::RequestHeaderMap& request_headers) override; void setTraceReason(Http::RequestHeaderMap& request_headers, Tracing::Reason status) override; + bool useRequestIdForTraceSampling() const override { return use_request_id_for_trace_sampling_; } private: Envoy::Random::RandomGenerator& random_; const bool pack_trace_reason_; + const bool use_request_id_for_trace_sampling_; // Byte on this position has predefined value of 4 for UUID4. static const int TRACE_BYTE_POSITION = 14; diff --git a/test/common/http/conn_manager_utility_test.cc b/test/common/http/conn_manager_utility_test.cc index 7948e3d19def4..25d47f3c0f12c 100644 --- a/test/common/http/conn_manager_utility_test.cc +++ b/test/common/http/conn_manager_utility_test.cc @@ -63,6 +63,7 @@ class MockRequestIDExtension : public RequestIDExtension { [this](Http::RequestHeaderMap& request_headers, Tracing::Reason trace_status) { real_->setTraceReason(request_headers, trace_status); }); + ON_CALL(*this, useRequestIdForTraceSampling()).WillByDefault(Return(true)); } MOCK_METHOD(void, set, (Http::RequestHeaderMap&, bool)); @@ -70,6 +71,7 @@ class MockRequestIDExtension : public RequestIDExtension { MOCK_METHOD(absl::optional, toInteger, (const Http::RequestHeaderMap&), (const)); MOCK_METHOD(Tracing::Reason, getTraceReason, (const Http::RequestHeaderMap&)); MOCK_METHOD(void, setTraceReason, (Http::RequestHeaderMap&, Tracing::Reason)); + MOCK_METHOD(bool, useRequestIdForTraceSampling, (), (const)); private: RequestIDExtensionSharedPtr real_; @@ -1229,6 +1231,14 @@ TEST_F(ConnectionManagerUtilityTest, SamplingWithoutRouteOverride) { EXPECT_EQ(Tracing::Reason::Sampling, request_id_extension_->getTraceReason(request_headers)); } +TEST_F(ConnectionManagerUtilityTest, CheckSamplingDecisionWithBypassSamplingWithRequestId) { + EXPECT_CALL(*request_id_extension_, useRequestIdForTraceSampling()).WillOnce(Return(false)); + 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_, diff --git a/test/extensions/filters/network/http_connection_manager/config_test.cc b/test/extensions/filters/network/http_connection_manager/config_test.cc index 2477db6bf4fc2..bcc6bb14fbee4 100644 --- a/test/extensions/filters/network/http_connection_manager/config_test.cc +++ b/test/extensions/filters/network/http_connection_manager/config_test.cc @@ -1721,7 +1721,7 @@ class TestRequestIDExtension : public Http::RequestIDExtension { return Tracing::Reason::Sampling; } void setTraceReason(Http::RequestHeaderMap&, Tracing::Reason) override {} - + bool useRequestIdForTraceSampling() const override { return true; } std::string testField() { return config_.test_field(); } private: @@ -1836,6 +1836,7 @@ TEST_F(HttpConnectionManagerConfigTest, DefaultRequestIDExtension) { config.requestIDExtension().get()); ASSERT_NE(nullptr, request_id_extension); EXPECT_TRUE(request_id_extension->packTraceReason()); + EXPECT_EQ(request_id_extension->useRequestIdForTraceSampling(), true); } TEST_F(HttpConnectionManagerConfigTest, DefaultRequestIDExtensionWithParams) { @@ -1847,6 +1848,7 @@ TEST_F(HttpConnectionManagerConfigTest, DefaultRequestIDExtensionWithParams) { typed_config: "@type": type.googleapis.com/envoy.extensions.request_id.uuid.v3.UuidRequestIdConfig pack_trace_reason: false + use_request_id_for_trace_sampling: false http_filters: - name: envoy.filters.http.router )EOF"; @@ -1859,6 +1861,7 @@ TEST_F(HttpConnectionManagerConfigTest, DefaultRequestIDExtensionWithParams) { config.requestIDExtension().get()); ASSERT_NE(nullptr, request_id_extension); EXPECT_FALSE(request_id_extension->packTraceReason()); + EXPECT_EQ(request_id_extension->useRequestIdForTraceSampling(), false); } TEST_F(HttpConnectionManagerConfigTest, UnknownOriginalIPDetectionExtension) {