Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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
5 changes: 5 additions & 0 deletions api/envoy/extensions/request_id/uuid/v3/uuid.proto
Original file line number Diff line number Diff line change
Expand Up @@ -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<config_http_conn_man_headers_x-request-id>` for sampling or not.
// This defaults to true. See the :ref:`context propagation <arch_overview_tracing_context_propagation>`
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// This defaults to true. See the :ref:`context propagation <arch_overview_tracing_context_propagation>`
// This defaults to true. See the :ref:`context propagation <arch_overview_tracing_context_propagation>` overview

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

// overview for more information.
google.protobuf.BoolValue use_request_id_for_trace_sampling = 2;
}
10 changes: 10 additions & 0 deletions docs/root/intro/arch_overview/observability/tracing.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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 <config_http_conn_man_headers_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 <envoy_v3_api_field_extensions.request_id.uuid.v3.UuidRequestIdConfig.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
Expand Down
1 change: 1 addition & 0 deletions docs/root/version_history/current.rst
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,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 option :ref:`use_request_id_for_trace_sampling <envoy_v3_api_field_extensions.request_id.uuid.v3.UuidRequestIdConfig.use_request_id_for_trace_sampling>` whether to use sampling policy based on :ref:`x-request-id<config_http_conn_man_headers_x-request-id>` or not.

Bug Fixes
---------
Expand Down
6 changes: 6 additions & 0 deletions envoy/http/request_id_extension.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<RequestIDExtension>;
Expand Down

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

3 changes: 3 additions & 0 deletions source/common/http/conn_manager_utility.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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()) {
Expand Down
6 changes: 5 additions & 1 deletion source/extensions/request_id/uuid/config.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<UUIDRequestIDExtension>(
Expand All @@ -32,10 +34,12 @@ class UUIDRequestIDExtension : public Http::RequestIDExtension {
absl::optional<uint64_t> 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;
Expand Down
10 changes: 10 additions & 0 deletions test/common/http/conn_manager_utility_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -63,13 +63,15 @@ 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));
MOCK_METHOD(void, setInResponse, (Http::ResponseHeaderMap&, const Http::RequestHeaderMap&));
MOCK_METHOD(absl::optional<uint64_t>, 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_;
Expand Down Expand Up @@ -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_,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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) {
Expand All @@ -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";
Expand All @@ -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) {
Expand Down