diff --git a/source/common/router/router.cc b/source/common/router/router.cc index cd461bacb7ca0..27d670149f0bf 100644 --- a/source/common/router/router.cc +++ b/source/common/router/router.cc @@ -652,9 +652,6 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::RequestHeaderMap& headers, } callbacks_->streamInfo().setAttemptCount(attempt_count_); - // Inject the active span's tracing context into the request headers. - callbacks_->activeSpan().injectContext(headers); - route_entry_->finalizeRequestHeaders(headers, callbacks_->streamInfo(), !config_.suppress_envoy_headers_); FilterUtility::setUpstreamScheme( diff --git a/source/common/router/upstream_request.cc b/source/common/router/upstream_request.cc index 25f763e17e7c0..c336eaffaf46c 100644 --- a/source/common/router/upstream_request.cc +++ b/source/common/router/upstream_request.cc @@ -486,6 +486,11 @@ void UpstreamRequest::onPoolReady( if (span_ != nullptr) { span_->injectContext(*parent_.downstreamHeaders()); + } else { + // No independent child span for current upstream request then inject the parent span's tracing + // context into the request headers. + // The injectContext() of the parent span may be called repeatedly when the request is retried. + parent_.callbacks()->activeSpan().injectContext(*parent_.downstreamHeaders()); } upstreamTiming().onFirstUpstreamTxByteSent(parent_.callbacks()->dispatcher().timeSource()); diff --git a/test/common/router/router_2_test.cc b/test/common/router/router_2_test.cc index 2f1f3c50bd061..a6a096d45b446 100644 --- a/test/common/router/router_2_test.cc +++ b/test/common/router/router_2_test.cc @@ -532,6 +532,43 @@ TEST_F(RouterTestChildSpan, ResetRetryFlow) { response_decoder->decodeHeaders(std::move(response_headers), true); } +class RouterTestNoChildSpan : public RouterTestBase { +public: + RouterTestNoChildSpan() + : RouterTestBase(false, false, false, Protobuf::RepeatedPtrField{}) {} +}; + +TEST_F(RouterTestNoChildSpan, BasicFlow) { + EXPECT_CALL(callbacks_.route_->route_entry_, timeout()) + .WillOnce(Return(std::chrono::milliseconds(0))); + EXPECT_CALL(callbacks_.dispatcher_, createTimer_(_)).Times(0); + + NiceMock encoder; + Http::ResponseDecoder* response_decoder = nullptr; + EXPECT_CALL(cm_.thread_local_cluster_.conn_pool_, newStream(_, _, _)) + .WillOnce( + Invoke([&](Http::ResponseDecoder& decoder, Http::ConnectionPool::Callbacks& callbacks, + const Http::ConnectionPool::Instance::StreamOptions&) + -> Http::ConnectionPool::Cancellable* { + response_decoder = &decoder; + EXPECT_CALL(callbacks_.active_span_, injectContext(_)); + callbacks.onPoolReady(encoder, cm_.thread_local_cluster_.conn_pool_.host_, + upstream_stream_info_, Http::Protocol::Http10); + return nullptr; + })); + + Http::TestRequestHeaderMapImpl headers; + HttpTestUtility::addDefaultHeaders(headers); + router_.decodeHeaders(headers, true); + EXPECT_EQ(1U, + callbacks_.route_->route_entry_.virtual_cluster_.stats().upstream_rq_total_.value()); + + Http::ResponseHeaderMapPtr response_headers( + new Http::TestResponseHeaderMapImpl{{":status", "200"}}); + ASSERT(response_decoder); + response_decoder->decodeHeaders(std::move(response_headers), true); +} + namespace { Protobuf::RepeatedPtrField protobufStrList(const std::vector& v) { diff --git a/test/common/router/router_test.cc b/test/common/router/router_test.cc index 24fee216cae12..178c12db1f5e4 100644 --- a/test/common/router/router_test.cc +++ b/test/common/router/router_test.cc @@ -412,7 +412,6 @@ TEST_F(RouterTest, Http1Upstream) { Http::TestRequestHeaderMapImpl headers; HttpTestUtility::addDefaultHeaders(headers); EXPECT_CALL(callbacks_.route_->route_entry_, finalizeRequestHeaders(_, _, true)); - EXPECT_CALL(span_, injectContext(_)); router_.decodeHeaders(headers, true); EXPECT_EQ("10", headers.get_("x-envoy-expected-rq-timeout-ms")); @@ -432,7 +431,6 @@ TEST_F(RouterTest, Http2Upstream) { Http::TestRequestHeaderMapImpl headers; HttpTestUtility::addDefaultHeaders(headers); - EXPECT_CALL(span_, injectContext(_)); router_.decodeHeaders(headers, true); // When the router filter gets reset we should cancel the pool request. @@ -739,7 +737,6 @@ TEST_F(RouterTest, MaintenanceMode) { EXPECT_CALL(callbacks_, encodeHeaders_(HeaderMapEqualRef(&response_headers), false)); EXPECT_CALL(callbacks_, encodeData(_, true)); EXPECT_CALL(callbacks_.stream_info_, setResponseFlag(StreamInfo::ResponseFlag::UpstreamOverflow)); - EXPECT_CALL(span_, injectContext(_)).Times(0); Http::TestRequestHeaderMapImpl headers; HttpTestUtility::addDefaultHeaders(headers); @@ -4556,7 +4553,6 @@ TEST_F(RouterTest, DirectResponse) { Http::TestResponseHeaderMapImpl response_headers{{":status", "200"}}; EXPECT_CALL(callbacks_, encodeHeaders_(HeaderMapEqualRef(&response_headers), true)); - EXPECT_CALL(span_, injectContext(_)).Times(0); Http::TestRequestHeaderMapImpl headers; HttpTestUtility::addDefaultHeaders(headers); router_.decodeHeaders(headers, true); @@ -4606,7 +4602,6 @@ TEST_F(RouterTest, DirectResponseWithLocation) { Http::TestResponseHeaderMapImpl response_headers{{":status", "201"}, {"location", "http://host/"}}; EXPECT_CALL(callbacks_, encodeHeaders_(HeaderMapEqualRef(&response_headers), true)); - EXPECT_CALL(span_, injectContext(_)).Times(0); Http::TestRequestHeaderMapImpl headers; HttpTestUtility::addDefaultHeaders(headers); router_.decodeHeaders(headers, true); @@ -4630,7 +4625,6 @@ TEST_F(RouterTest, DirectResponseWithoutLocation) { Http::TestResponseHeaderMapImpl response_headers{{":status", "200"}}; EXPECT_CALL(callbacks_, encodeHeaders_(HeaderMapEqualRef(&response_headers), true)); - EXPECT_CALL(span_, injectContext(_)).Times(0); Http::TestRequestHeaderMapImpl headers; HttpTestUtility::addDefaultHeaders(headers); router_.decodeHeaders(headers, true); @@ -5653,7 +5647,6 @@ TEST_F(RouterTest, ApplicationProtocols) { Http::TestRequestHeaderMapImpl headers; HttpTestUtility::addDefaultHeaders(headers); - EXPECT_CALL(span_, injectContext(_)); router_.decodeHeaders(headers, true); // When the router filter gets reset we should cancel the pool request.