diff --git a/docs/configuration/http_conn_man/headers.rst b/docs/configuration/http_conn_man/headers.rst index b13c98efb48c..a25cff234b2a 100644 --- a/docs/configuration/http_conn_man/headers.rst +++ b/docs/configuration/http_conn_man/headers.rst @@ -70,11 +70,13 @@ x-envoy-force-trace ------------------- If an internal request sets this header, Envoy will modify the generated -:ref:`config_http_conn_man_headers_x-request-id` such that it forces traces to be collected. If this -request ID is then propagated to other hosts, traces will also be collected on those hosts which -will provide a consistent trace for an entire request flow. See the :ref:`tracing.global_enabled -` and :ref:`tracing.random_sampling -` runtime configuration settings. +:ref:`config_http_conn_man_headers_x-request-id` such that it forces traces to be collected. +This also forces :ref:`config_http_conn_man_headers_x-request-id` to be returned in the response +headers. If this request ID is then propagated to other hosts, traces will also be collected on +those hosts which will provide a consistent trace for an entire request flow. See the +:ref:`tracing.global_enabled ` and +:ref:`tracing.random_sampling ` runtime +configuration settings. .. _config_http_conn_man_headers_x-envoy-internal: diff --git a/source/common/http/conn_manager_impl.cc b/source/common/http/conn_manager_impl.cc index 9e2e8ba1006c..39f4479e3afd 100644 --- a/source/common/http/conn_manager_impl.cc +++ b/source/common/http/conn_manager_impl.cc @@ -508,7 +508,8 @@ void ConnectionManagerImpl::ActiveStream::encodeHeaders(ActiveStreamEncoderFilte headers.replaceViaMoveValue(Headers::get().Date, date_formatter_.now()); headers.replaceViaCopy(Headers::get().EnvoyProtocolVersion, connection_manager_.codec_->protocolString()); - ConnectionManagerUtility::mutateResponseHeaders(headers, connection_manager_.config_); + ConnectionManagerUtility::mutateResponseHeaders(headers, *request_headers_, + connection_manager_.config_); // See if we want to drain/close the connection. Send the go away frame prior to encoding the // header block. diff --git a/source/common/http/conn_manager_utility.cc b/source/common/http/conn_manager_utility.cc index f4922ef49619..222f47f85640 100644 --- a/source/common/http/conn_manager_utility.cc +++ b/source/common/http/conn_manager_utility.cc @@ -4,6 +4,7 @@ #include "common/http/utility.h" #include "common/network/utility.h" #include "common/runtime/uuid_util.h" +#include "common/tracing/http_tracer_impl.h" namespace Http { @@ -65,6 +66,7 @@ void ConnectionManagerUtility::mutateRequestHeaders(Http::HeaderMap& request_hea request_headers.remove(Headers::get().EnvoyUpstreamRequestTimeoutMs); request_headers.remove(Headers::get().EnvoyUpstreamRequestPerTryTimeoutMs); request_headers.remove(Headers::get().EnvoyExpectedRequestTimeoutMs); + request_headers.remove(Headers::get().EnvoyForceTrace); for (const Http::LowerCaseString& header : config.routeConfig().internalOnlyHeaders()) { request_headers.remove(header); @@ -107,8 +109,8 @@ void ConnectionManagerUtility::mutateRequestHeaders(Http::HeaderMap& request_hea } } - // Make request traceable if it's internal and ForceTrace header is set. - if (internal_request && request_headers.has(Headers::get().ForceTrace)) { + // Make request traceable if x-envoy-force-trace header is set. + if (request_headers.has(Headers::get().EnvoyForceTrace)) { std::string uuid = request_headers.get(Headers::get().RequestId); UuidUtils::setTraceableUuid(uuid); request_headers.replaceViaMoveValue(Headers::get().RequestId, std::move(uuid)); @@ -116,6 +118,7 @@ void ConnectionManagerUtility::mutateRequestHeaders(Http::HeaderMap& request_hea } void ConnectionManagerUtility::mutateResponseHeaders(Http::HeaderMap& response_headers, + const Http::HeaderMap& request_headers, ConnectionManagerConfig& config) { response_headers.remove(Headers::get().Connection); response_headers.remove(Headers::get().TransferEncoding); @@ -129,6 +132,11 @@ void ConnectionManagerUtility::mutateResponseHeaders(Http::HeaderMap& response_h config.routeConfig().responseHeadersToAdd()) { response_headers.addViaCopy(to_add.first, to_add.second); } + + if (request_headers.has(Headers::get().EnvoyForceTrace)) { + response_headers.replaceViaCopy(Headers::get().RequestId, + request_headers.get(Headers::get().RequestId)); + } } } // Http diff --git a/source/common/http/conn_manager_utility.h b/source/common/http/conn_manager_utility.h index 8d285fb83d3b..b5f59697eb3d 100644 --- a/source/common/http/conn_manager_utility.h +++ b/source/common/http/conn_manager_utility.h @@ -17,6 +17,7 @@ class ConnectionManagerUtility { Runtime::RandomGenerator& random, Runtime::Loader& runtime); static void mutateResponseHeaders(Http::HeaderMap& response_headers, + const Http::HeaderMap& request_headers, ConnectionManagerConfig& config); }; diff --git a/source/common/http/headers.h b/source/common/http/headers.h index c067951a1c57..87bf913380dc 100644 --- a/source/common/http/headers.h +++ b/source/common/http/headers.h @@ -19,6 +19,7 @@ class Headers { const LowerCaseString Date{"date"}; const LowerCaseString EnvoyDownstreamServiceCluster{"x-envoy-downstream-service-cluster"}; const LowerCaseString EnvoyExternalAddress{"x-envoy-external-address"}; + const LowerCaseString EnvoyForceTrace{"x-envoy-force-trace"}; const LowerCaseString EnvoyInternalRequest{"x-envoy-internal"}; const LowerCaseString EnvoyMaxRetries{"x-envoy-max-retries"}; const LowerCaseString EnvoyOriginalPath{"x-envoy-original-path"}; @@ -33,7 +34,6 @@ class Headers { const LowerCaseString EnvoyUpstreamServiceTime{"x-envoy-upstream-service-time"}; const LowerCaseString EnvoyUpstreamHealthCheckedCluster{"x-envoy-upstream-healthchecked-cluster"}; const LowerCaseString Expect{"expect"}; - const LowerCaseString ForceTrace{"x-envoy-force-trace"}; const LowerCaseString ForwardedFor{"x-forwarded-for"}; const LowerCaseString ForwardedProto{"x-forwarded-proto"}; const LowerCaseString Host{":authority"}; diff --git a/source/common/tracing/http_tracer_impl.cc b/source/common/tracing/http_tracer_impl.cc index a95f1eab2841..f545d119558e 100644 --- a/source/common/tracing/http_tracer_impl.cc +++ b/source/common/tracing/http_tracer_impl.cc @@ -37,7 +37,7 @@ Decision HttpTracerUtility::isTracing(const Http::AccessLog::RequestInfo& reques return {Reason::ClientForced, true}; } - if (request_headers.has(Http::Headers::get().ForceTrace)) { + if (request_headers.has(Http::Headers::get().EnvoyForceTrace)) { return {Reason::ServiceForced, true}; } diff --git a/test/common/http/conn_manager_utility_test.cc b/test/common/http/conn_manager_utility_test.cc index 8a89c0766154..88aaf36eb56e 100644 --- a/test/common/http/conn_manager_utility_test.cc +++ b/test/common/http/conn_manager_utility_test.cc @@ -103,13 +103,14 @@ TEST_F(ConnectionManagerUtilityTest, InternalServiceForceTrace) { } { - // Not internal request, force trace should be ignored + // Not internal request, force trace header should be cleaned. HeaderMapImpl headers{{"x-forwarded-for", external_remote_address}, {"x-request-id", uuid}, {"x-envoy-force-trace", "true"}}; ConnectionManagerUtility::mutateRequestHeaders(headers, connection_, config_, random_, runtime_); EXPECT_EQ(uuid, headers.get(Headers::get().RequestId)); + EXPECT_FALSE(headers.has(Headers::get().EnvoyForceTrace)); } } @@ -312,14 +313,25 @@ TEST_F(ConnectionManagerUtilityTest, MutateResponseHeaders) { config_.route_config_.response_headers_to_remove_.push_back("custom_header"); config_.route_config_.response_headers_to_add_.push_back({"to_add", "foo"}); - HeaderMapImpl headers{{"connection", "foo"}, - {"transfer-encoding", "foo"}, - {":version", "foo"}, - {"custom_header", "foo"}}; - ConnectionManagerUtility::mutateResponseHeaders(headers, config_); + HeaderMapImpl response_headers{{"connection", "foo"}, + {"transfer-encoding", "foo"}, + {":version", "foo"}, + {"custom_header", "foo"}}; + HeaderMapImpl request_headers{{"x-request-id", "request-id"}}; + + ConnectionManagerUtility::mutateResponseHeaders(response_headers, request_headers, config_); + + EXPECT_EQ(1UL, response_headers.size()); + EXPECT_EQ("foo", response_headers.get("to_add")); + EXPECT_FALSE(response_headers.has("x-request-id")); +} + +TEST_F(ConnectionManagerUtilityTest, MutateResponseHeadersReturnXRequestId) { + HeaderMapImpl response_headers; + HeaderMapImpl request_headers{{"x-request-id", "request-id"}, {"x-envoy-force-trace", "true"}}; - EXPECT_EQ(1UL, headers.size()); - EXPECT_EQ("foo", headers.get("to_add")); + ConnectionManagerUtility::mutateResponseHeaders(response_headers, request_headers, config_); + EXPECT_EQ("request-id", response_headers.get("x-request-id")); } } // Http