Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 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
4 changes: 3 additions & 1 deletion envoy/tracing/trace_driver.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,10 @@ class Span {
* Mutate the provided headers with the context necessary to propagate this
* (implementation-specific) trace.
* @param request_headers the headers to which propagation context will be added
* @param upstream connecting host description
*/
virtual void injectContext(TraceContext& trace_conext) PURE;
virtual void injectContext(TraceContext& trace_conext,
const Upstream::HostDescriptionConstSharedPtr& upstream) PURE;

/**
* Create and start a child Span, with this Span as its parent in the trace.
Expand Down
2 changes: 1 addition & 1 deletion source/common/grpc/async_client_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ void AsyncRequestImpl::cancel() {
}

void AsyncRequestImpl::onCreateInitialMetadata(Http::RequestHeaderMap& metadata) {
current_span_->injectContext(metadata);
current_span_->injectContext(metadata, nullptr);
callbacks_.onCreateInitialMetadata(metadata);
}

Expand Down
2 changes: 1 addition & 1 deletion source/common/grpc/google_async_client_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -455,7 +455,7 @@ void GoogleAsyncRequestImpl::cancel() {
}

void GoogleAsyncRequestImpl::onCreateInitialMetadata(Http::RequestHeaderMap& metadata) {
current_span_->injectContext(metadata);
current_span_->injectContext(metadata, nullptr);
callbacks_.onCreateInitialMetadata(metadata);
}

Expand Down
2 changes: 1 addition & 1 deletion source/common/http/async_client_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@ AsyncRequestImpl::AsyncRequestImpl(RequestMessagePtr&& request, AsyncClientImpl&
}

void AsyncRequestImpl::initialize() {
child_span_->injectContext(request_->headers());
child_span_->injectContext(request_->headers(), nullptr);
sendHeaders(request_->headers(), request_->body().length() == 0);
if (request_->body().length() != 0) {
// It's possible this will be a no-op due to a local response synchronously generated in
Expand Down
8 changes: 5 additions & 3 deletions source/common/router/upstream_request.cc
Original file line number Diff line number Diff line change
Expand Up @@ -483,14 +483,16 @@ void UpstreamRequest::onPoolReady(
Http::Utility::updateAuthority(*parent_.downstreamHeaders(), host->hostname(),
parent_.routeEntry()->appendXfh());
}

const auto host = stream_info_.upstreamInfo() != nullptr
? stream_info_.upstreamInfo()->upstreamHost()
: nullptr;
if (span_ != nullptr) {
span_->injectContext(*parent_.downstreamHeaders());
span_->injectContext(*parent_.downstreamHeaders(), host);
} 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());
parent_.callbacks()->activeSpan().injectContext(*parent_.downstreamHeaders(), host);
}
Comment thread
Shikugawa marked this conversation as resolved.
Outdated

upstreamTiming().onFirstUpstreamTxByteSent(parent_.callbacks()->dispatcher().timeSource());
Expand Down
3 changes: 2 additions & 1 deletion source/common/tracing/null_span_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ class NullSpan : public Span {
void setTag(absl::string_view, absl::string_view) override {}
void log(SystemTime, const std::string&) override {}
void finishSpan() override {}
void injectContext(Tracing::TraceContext&) override {}
void injectContext(Tracing::TraceContext&,
const Upstream::HostDescriptionConstSharedPtr&) override {}
void setBaggage(absl::string_view, absl::string_view) override {}
std::string getBaggage(absl::string_view) override { return EMPTY_STRING; }
std::string getTraceIdAsHex() const override { return EMPTY_STRING; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,8 @@ std::string OpenTracingSpan::getBaggage(absl::string_view key) {
return span_->BaggageItem({key.data(), key.length()});
}

void OpenTracingSpan::injectContext(Tracing::TraceContext& trace_context) {
void OpenTracingSpan::injectContext(Tracing::TraceContext& trace_context,
const Upstream::HostDescriptionConstSharedPtr&) {
if (driver_.propagationMode() == OpenTracingDriver::PropagationMode::SingleHeader) {
// Inject the span context using Envoy's single-header format.
std::ostringstream oss;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ class OpenTracingSpan : public Tracing::Span, Logger::Loggable<Logger::Id::traci
void setOperation(absl::string_view operation) override;
void setTag(absl::string_view name, const absl::string_view) override;
void log(SystemTime timestamp, const std::string& event) override;
void injectContext(Tracing::TraceContext& trace_context) override;
void injectContext(Tracing::TraceContext& trace_context,
const Upstream::HostDescriptionConstSharedPtr&) override;
Tracing::SpanPtr spawnChild(const Tracing::Config& config, const std::string& name,
SystemTime start_time) override;
void setSampled(bool) override;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,8 @@ class Span : public Tracing::Span {
void setTag(absl::string_view name, absl::string_view value) override;
void log(SystemTime timestamp, const std::string& event) override;
void finishSpan() override;
void injectContext(Tracing::TraceContext& trace_context) override;
void injectContext(Tracing::TraceContext& trace_context,
const Upstream::HostDescriptionConstSharedPtr&) override;
Tracing::SpanPtr spawnChild(const Tracing::Config& config, const std::string& name,
SystemTime start_time) override;
void setSampled(bool sampled) override;
Expand Down Expand Up @@ -201,7 +202,8 @@ void Span::log(SystemTime /*timestamp*/, const std::string& event) {

void Span::finishSpan() { span_.End(); }

void Span::injectContext(Tracing::TraceContext& trace_context) {
void Span::injectContext(Tracing::TraceContext& trace_context,
const Upstream::HostDescriptionConstSharedPtr&) {
using OpenCensusConfig = envoy::config::trace::v3::OpenCensusConfig;
const auto& ctx = span_.context();
for (const auto& outgoing : oc_config_.outgoing_trace_context()) {
Expand Down
7 changes: 5 additions & 2 deletions source/extensions/tracers/skywalking/tracer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -44,13 +44,16 @@ void Span::finishSpan() {
parent_tracer_.sendSegment(tracing_context_);
}

void Span::injectContext(Tracing::TraceContext& trace_context) {
void Span::injectContext(Tracing::TraceContext& trace_context,
const Upstream::HostDescriptionConstSharedPtr& upstream) {
const auto remote_address = upstream != nullptr ? upstream->address()->asString()
: std::string(trace_context.authority());
// TODO(wbpcode): Due to https://github.com/SkyAPM/cpp2sky/issues/83 in cpp2sky, it is necessary
// to ensure that there is '\0' at the end of the string_view parameter to ensure that the
// corresponding trace header is generated correctly. For this reason, we cannot directly use host
// as argument. We need create a copy of std::string based on host and std::string will
// automatically add '\0' to the end of the string content.
auto sw8_header = tracing_context_->createSW8HeaderValue(std::string(trace_context.authority()));
auto sw8_header = tracing_context_->createSW8HeaderValue(remote_address);
if (sw8_header.has_value()) {
trace_context.setByReferenceKey(skywalkingPropagationHeaderKey(), sw8_header.value());
}
Expand Down
3 changes: 2 additions & 1 deletion source/extensions/tracers/skywalking/tracer.h
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,8 @@ class Span : public Tracing::Span {
void setTag(absl::string_view name, absl::string_view value) override;
void log(SystemTime timestamp, const std::string& event) override;
void finishSpan() override;
void injectContext(Tracing::TraceContext& trace_context) override;
void injectContext(Tracing::TraceContext& trace_context,
const Upstream::HostDescriptionConstSharedPtr& upstream) override;
Tracing::SpanPtr spawnChild(const Tracing::Config& config, const std::string& name,
Comment thread
Shikugawa marked this conversation as resolved.
SystemTime start_time) override;
void setSampled(bool do_sample) override;
Expand Down
3 changes: 2 additions & 1 deletion source/extensions/tracers/xray/tracer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,8 @@ void Span::finishSpan() {
broker_.send(json);
} // namespace XRay

void Span::injectContext(Tracing::TraceContext& trace_context) {
void Span::injectContext(Tracing::TraceContext& trace_context,
const Upstream::HostDescriptionConstSharedPtr&) {
const std::string xray_header_value =
fmt::format("Root={};Parent={};Sampled={}", traceId(), id(), sampled() ? "1" : "0");
trace_context.setByReferenceKey(XRayTraceHeader, xray_header_value);
Expand Down
3 changes: 2 additions & 1 deletion source/extensions/tracers/xray/tracer.h
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,8 @@ class Span : public Tracing::Span, Logger::Loggable<Logger::Id::config> {
/**
* Adds X-Ray trace header to the set of outgoing headers.
*/
void injectContext(Tracing::TraceContext& trace_context) override;
void injectContext(Tracing::TraceContext& trace_context,
const Upstream::HostDescriptionConstSharedPtr&) override;

/**
* Gets the start time of this Span.
Expand Down
3 changes: 2 additions & 1 deletion source/extensions/tracers/zipkin/zipkin_tracer_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ void ZipkinSpan::log(SystemTime timestamp, const std::string& event) {
void ZipkinSpan::setBaggage(absl::string_view, absl::string_view) {}
std::string ZipkinSpan::getBaggage(absl::string_view) { return EMPTY_STRING; }

void ZipkinSpan::injectContext(Tracing::TraceContext& trace_context) {
void ZipkinSpan::injectContext(Tracing::TraceContext& trace_context,
const Upstream::HostDescriptionConstSharedPtr&) {
// Set the trace-id and span-id headers properly, based on the newly-created span structure.
trace_context.setByReferenceKey(ZipkinCoreConstants::get().X_B3_TRACE_ID,
span_.traceIdAsHexString());
Expand Down
3 changes: 2 additions & 1 deletion source/extensions/tracers/zipkin/zipkin_tracer_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,8 @@ class ZipkinSpan : public Tracing::Span {

void log(SystemTime timestamp, const std::string& event) override;

void injectContext(Tracing::TraceContext& trace_context) override;
void injectContext(Tracing::TraceContext& trace_context,
const Upstream::HostDescriptionConstSharedPtr&) override;
Tracing::SpanPtr spawnChild(const Tracing::Config&, const std::string& name,
SystemTime start_time) override;

Expand Down
4 changes: 2 additions & 2 deletions test/common/grpc/async_client_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ TEST_F(EnvoyAsyncClientImplTest, RequestHttpStartFail) {
EXPECT_CALL(*child_span, setTag(Eq(Tracing::Tags::get().GrpcStatusCode), Eq("14")));
EXPECT_CALL(*child_span, setTag(Eq(Tracing::Tags::get().Error), Eq(Tracing::Tags::get().True)));
EXPECT_CALL(*child_span, finishSpan());
EXPECT_CALL(*child_span, injectContext(_)).Times(0);
EXPECT_CALL(*child_span, injectContext(_, _)).Times(0);

auto* grpc_request = grpc_client_->send(*method_descriptor_, request_msg, grpc_callbacks,
active_span, Http::AsyncClient::RequestOptions());
Expand Down Expand Up @@ -249,7 +249,7 @@ TEST_F(EnvoyAsyncClientImplTest, RequestHttpSendHeadersFail) {
setTag(Eq(Tracing::Tags::get().Component), Eq(Tracing::Tags::get().Proxy)));
EXPECT_CALL(*child_span, setTag(Eq(Tracing::Tags::get().UpstreamCluster), Eq("test_cluster")));
EXPECT_CALL(*child_span, setTag(Eq(Tracing::Tags::get().UpstreamAddress), Eq("test_cluster")));
EXPECT_CALL(*child_span, injectContext(_));
EXPECT_CALL(*child_span, injectContext(_, _));
EXPECT_CALL(*child_span, setTag(Eq(Tracing::Tags::get().GrpcStatusCode), Eq("13")));
EXPECT_CALL(*child_span, setTag(Eq(Tracing::Tags::get().Error), Eq(Tracing::Tags::get().True)));
EXPECT_CALL(*child_span, finishSpan());
Expand Down
2 changes: 1 addition & 1 deletion test/common/grpc/google_async_client_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ TEST_F(EnvoyGoogleAsyncClientImplTest, RequestHttpStartFail) {
EXPECT_CALL(*child_span, setTag(Eq(Tracing::Tags::get().GrpcStatusCode), Eq("14")));
EXPECT_CALL(*child_span, setTag(Eq(Tracing::Tags::get().Error), Eq(Tracing::Tags::get().True)));
EXPECT_CALL(*child_span, finishSpan());
EXPECT_CALL(*child_span, injectContext(_));
EXPECT_CALL(*child_span, injectContext(_, _));

auto* grpc_request = grpc_client_->send(*method_descriptor_, request_msg, grpc_callbacks,
active_span, Http::AsyncClient::RequestOptions());
Expand Down
2 changes: 1 addition & 1 deletion test/common/grpc/grpc_client_integration_test_harness.h
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,7 @@ class GrpcClientIntegrationTest : public GrpcClientIntegrationParamTest {
.Times(AtMost(1));
EXPECT_CALL(*request->child_span_,
setTag(Eq(Tracing::Tags::get().Component), Eq(Tracing::Tags::get().Proxy)));
EXPECT_CALL(*request->child_span_, injectContext(_));
EXPECT_CALL(*request->child_span_, injectContext(_, _));

request->grpc_request_ = grpc_client_->send(*method_descriptor_, request_msg, *request,
active_span, Http::AsyncClient::RequestOptions());
Expand Down
12 changes: 6 additions & 6 deletions test/common/http/async_client_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ TEST_F(AsyncClientImplTracingTest, Basic) {

AsyncClient::RequestOptions options = AsyncClient::RequestOptions().setParentSpan(parent_span_);
EXPECT_CALL(*child_span, setSampled(true));
EXPECT_CALL(*child_span, injectContext(_));
EXPECT_CALL(*child_span, injectContext(_, _));

auto* request = client_.send(std::move(message_), callbacks_, options);
EXPECT_NE(request, nullptr);
Expand Down Expand Up @@ -280,7 +280,7 @@ TEST_F(AsyncClientImplTracingTest, BasicNamedChildSpan) {
.setChildSpanName(child_span_name_)
.setSampled(false);
EXPECT_CALL(*child_span, setSampled(false));
EXPECT_CALL(*child_span, injectContext(_));
EXPECT_CALL(*child_span, injectContext(_, _));

auto* request = client_.send(std::move(message_), callbacks_, options);
EXPECT_NE(request, nullptr);
Expand Down Expand Up @@ -331,7 +331,7 @@ TEST_F(AsyncClientImplTracingTest, BasicNamedChildSpanKeepParentSampling) {
.setChildSpanName(child_span_name_)
.setSampled(absl::nullopt);
EXPECT_CALL(*child_span, setSampled(_)).Times(0);
EXPECT_CALL(*child_span, injectContext(_));
EXPECT_CALL(*child_span, injectContext(_, _));

auto* request = client_.send(std::move(message_), callbacks_, options);
EXPECT_NE(request, nullptr);
Expand Down Expand Up @@ -1245,7 +1245,7 @@ TEST_F(AsyncClientImplTracingTest, CancelRequest) {

AsyncClient::RequestOptions options = AsyncClient::RequestOptions().setParentSpan(parent_span_);
EXPECT_CALL(*child_span, setSampled(true));
EXPECT_CALL(*child_span, injectContext(_));
EXPECT_CALL(*child_span, injectContext(_, _));
EXPECT_CALL(callbacks_, onBeforeFinalizeUpstreamSpan(_, _))
.WillOnce(Invoke([](Tracing::Span& span, const Http::ResponseHeaderMap* response_headers) {
span.setTag("onBeforeFinalizeUpstreamSpan", "called");
Expand Down Expand Up @@ -1331,7 +1331,7 @@ TEST_F(AsyncClientImplTracingTest, DestroyWithActiveRequest) {

AsyncClient::RequestOptions options = AsyncClient::RequestOptions().setParentSpan(parent_span_);
EXPECT_CALL(*child_span, setSampled(true));
EXPECT_CALL(*child_span, injectContext(_));
EXPECT_CALL(*child_span, injectContext(_, _));

auto* request = client_.send(std::move(message_), callbacks_, options);
EXPECT_NE(request, nullptr);
Expand Down Expand Up @@ -1531,7 +1531,7 @@ TEST_F(AsyncClientImplTracingTest, RequestTimeout) {
.setParentSpan(parent_span_)
.setTimeout(std::chrono::milliseconds(40));
EXPECT_CALL(*child_span, setSampled(true));
EXPECT_CALL(*child_span, injectContext(_));
EXPECT_CALL(*child_span, injectContext(_, _));

auto* request = client_.send(std::move(message_), callbacks_, options);
EXPECT_NE(request, nullptr);
Expand Down
10 changes: 5 additions & 5 deletions test/common/router/router_2_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,7 @@ TEST_F(RouterTestChildSpan, BasicFlow) {
const Http::ConnectionPool::Instance::StreamOptions&)
-> Http::ConnectionPool::Cancellable* {
response_decoder = &decoder;
EXPECT_CALL(*child_span, injectContext(_));
EXPECT_CALL(*child_span, injectContext(_, _));
callbacks.onPoolReady(encoder, cm_.thread_local_cluster_.conn_pool_.host_,
upstream_stream_info_, Http::Protocol::Http10);
return nullptr;
Expand Down Expand Up @@ -357,7 +357,7 @@ TEST_F(RouterTestChildSpan, ResetFlow) {
const Http::ConnectionPool::Instance::StreamOptions&)
-> Http::ConnectionPool::Cancellable* {
response_decoder = &decoder;
EXPECT_CALL(*child_span, injectContext(_));
EXPECT_CALL(*child_span, injectContext(_, _));
callbacks.onPoolReady(encoder, cm_.thread_local_cluster_.conn_pool_.host_,
upstream_stream_info_, Http::Protocol::Http10);
return nullptr;
Expand Down Expand Up @@ -409,7 +409,7 @@ TEST_F(RouterTestChildSpan, CancelFlow) {
.WillOnce(Invoke([&](Http::StreamDecoder&, Http::ConnectionPool::Callbacks& callbacks,
const Http::ConnectionPool::Instance::StreamOptions&)
-> Http::ConnectionPool::Cancellable* {
EXPECT_CALL(*child_span, injectContext(_));
EXPECT_CALL(*child_span, injectContext(_, _));
callbacks.onPoolReady(encoder, cm_.thread_local_cluster_.conn_pool_.host_,
upstream_stream_info_, Http::Protocol::Http10);
return nullptr;
Expand Down Expand Up @@ -455,7 +455,7 @@ TEST_F(RouterTestChildSpan, ResetRetryFlow) {
const Http::ConnectionPool::Instance::StreamOptions&)
-> Http::ConnectionPool::Cancellable* {
response_decoder = &decoder;
EXPECT_CALL(*child_span_1, injectContext(_));
EXPECT_CALL(*child_span_1, injectContext(_, _));
callbacks.onPoolReady(encoder1, cm_.thread_local_cluster_.conn_pool_.host_,
upstream_stream_info_, Http::Protocol::Http10);
return nullptr;
Expand Down Expand Up @@ -499,7 +499,7 @@ TEST_F(RouterTestChildSpan, ResetRetryFlow) {
const Http::ConnectionPool::Instance::StreamOptions&)
-> Http::ConnectionPool::Cancellable* {
response_decoder = &decoder;
EXPECT_CALL(*child_span_2, injectContext(_));
EXPECT_CALL(*child_span_2, injectContext(_, _));
EXPECT_CALL(*router_.retry_state_, onHostAttempted(_));
callbacks.onPoolReady(encoder2, cm_.thread_local_cluster_.conn_pool_.host_,
upstream_stream_info_, Http::Protocol::Http10);
Expand Down
2 changes: 2 additions & 0 deletions test/common/router/router_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -412,6 +412,7 @@ TEST_F(RouterTest, Http1Upstream) {
Http::TestRequestHeaderMapImpl headers;
HttpTestUtility::addDefaultHeaders(headers);
EXPECT_CALL(callbacks_.route_->route_entry_, finalizeRequestHeaders(_, _, true));

router_.decodeHeaders(headers, true);
EXPECT_EQ("10", headers.get_("x-envoy-expected-rq-timeout-ms"));

Expand All @@ -431,6 +432,7 @@ TEST_F(RouterTest, Http2Upstream) {

Http::TestRequestHeaderMapImpl headers;
HttpTestUtility::addDefaultHeaders(headers);

router_.decodeHeaders(headers, true);

// When the router filter gets reset we should cancel the pool request.
Expand Down
4 changes: 3 additions & 1 deletion test/common/tracing/http_tracer_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -739,6 +739,8 @@ TEST(HttpNullTracerTest, BasicFunctionality) {
Http::TestRequestHeaderMapImpl request_headers;
Http::TestResponseHeaderMapImpl response_headers;
Http::TestResponseTrailerMapImpl response_trailers;
Upstream::HostDescriptionConstSharedPtr host{
new testing::NiceMock<Upstream::MockHostDescription>()};

SpanPtr span_ptr =
null_tracer.startSpan(config, request_headers, stream_info, {Reason::Sampling, true});
Expand All @@ -749,7 +751,7 @@ TEST(HttpNullTracerTest, BasicFunctionality) {
span_ptr->setBaggage("key", "value");
ASSERT_EQ("", span_ptr->getBaggage("baggage_key"));
ASSERT_EQ(span_ptr->getTraceIdAsHex(), "");
span_ptr->injectContext(request_headers);
span_ptr->injectContext(request_headers, host);
span_ptr->log(SystemTime(), "fake_event");

EXPECT_NE(nullptr, span_ptr->spawnChild(config, "foo", SystemTime()));
Expand Down
Loading