From 9ad7d4ce5923fb053506cb76d47f93049a509a4c Mon Sep 17 00:00:00 2001 From: Matthew Grossman Date: Mon, 10 Aug 2020 21:52:30 -0700 Subject: [PATCH] tracing: add baggage methods to Tracing::Span (#12260) * WIP add some header info on baggage Signed-off-by: Matthew Grossman * messing w/ stuff Signed-off-by: Matthew Grossman * get a compiling solution Signed-off-by: Matthew Grossman * lint Signed-off-by: Matthew Grossman * format again Signed-off-by: Matthew Grossman * formatting Signed-off-by: Matthew Grossman * change the test slightly Signed-off-by: Matthew Grossman * s/std::string/string_view Signed-off-by: Matthew Grossman * add note about baggage and child/parent spans Signed-off-by: Matthew Grossman * add in test for OpenTracingDriverTest Signed-off-by: Matthew Grossman * add in test for OpenCensus Signed-off-by: Matthew Grossman * add in baggage test for xray Signed-off-by: Matthew Grossman * add in zipkin tests Signed-off-by: Matthew Grossman * add in http tracer impl test Signed-off-by: Matthew Grossman * fix test for nulltracer Signed-off-by: Matthew Grossman * Add additional TODO for zipkin impl and add in comments for tests Signed-off-by: Matthew Grossman * add in draft docs Signed-off-by: Matthew Grossman * fix doc link Signed-off-by: Matthew Grossman * Inline empty methods per mklein's nit Signed-off-by: Jake Kaufman Co-authored-by: Jake Kaufman --- .../arch_overview/observability/tracing.rst | 14 ++++++++++++++ include/envoy/tracing/http_tracer.h | 16 ++++++++++++++++ source/common/tracing/http_tracer_impl.h | 2 ++ .../tracers/common/ot/opentracing_driver_impl.cc | 8 ++++++++ .../tracers/common/ot/opentracing_driver_impl.h | 2 ++ .../tracers/opencensus/opencensus_tracer_impl.cc | 4 ++++ source/extensions/tracers/xray/tracer.h | 4 ++++ .../tracers/zipkin/zipkin_tracer_impl.cc | 4 ++++ .../tracers/zipkin/zipkin_tracer_impl.h | 4 ++++ test/common/tracing/http_tracer_impl_test.cc | 2 ++ .../common/ot/opentracing_driver_impl_test.cc | 14 ++++++++++++++ .../lightstep/lightstep_tracer_impl_test.cc | 11 +++++++++++ .../extensions/tracers/opencensus/tracer_test.cc | 4 ++++ test/extensions/tracers/xray/tracer_test.cc | 10 ++++++++++ .../tracers/zipkin/zipkin_tracer_impl_test.cc | 8 ++++++++ test/mocks/tracing/mocks.h | 2 ++ 16 files changed, 109 insertions(+) diff --git a/docs/root/intro/arch_overview/observability/tracing.rst b/docs/root/intro/arch_overview/observability/tracing.rst index 26f057468d76..958b003a5d9a 100644 --- a/docs/root/intro/arch_overview/observability/tracing.rst +++ b/docs/root/intro/arch_overview/observability/tracing.rst @@ -113,3 +113,17 @@ request ID :ref:`config_http_conn_man_headers_x-request-id` (LightStep) or the trace ID configuration (Zipkin and Datadog). See :ref:`v3 API reference ` for more information on how to setup tracing in Envoy. + +Baggage +----------------------------- +Baggage provides a mechanism for data to be available throughout the entirety of a trace. +While metadata such as tags are usually communicated to collectors out-of-band, baggage data is injected into the actual +request context and available to applications during the duration of the request. This enables metadata to transparently +travel from the beginning of the request throughout your entire mesh without relying on application-specific modifications for +propagation. See `OpenTracing's documentation `_ for more information about baggage. + +Tracing providers have varying level of support for getting and setting baggage: + +* Lightstep (and any OpenTracing-compliant tracer) can read/write baggage +* Zipkin support is not yet implemented +* X-Ray and OpenCensus don't support baggage diff --git a/include/envoy/tracing/http_tracer.h b/include/envoy/tracing/http_tracer.h index 63da639e84ee..22b024ac97e0 100644 --- a/include/envoy/tracing/http_tracer.h +++ b/include/envoy/tracing/http_tracer.h @@ -158,6 +158,22 @@ class Span { * @param sampled whether the span and any subsequent child spans should be sampled */ virtual void setSampled(bool sampled) PURE; + + /** + * Retrieve a key's value from the span's baggage. + * This baggage data could've been set by this span or any parent spans. + * @param key baggage key + * @return the baggage's value for the given input key + */ + virtual std::string getBaggage(absl::string_view key) PURE; + + /** + * Set a key/value pair in the current span's baggage. + * All subsequent child spans will have access to this baggage. + * @param key baggage key + * @param key baggage value + */ + virtual void setBaggage(absl::string_view key, absl::string_view value) PURE; }; /** diff --git a/source/common/tracing/http_tracer_impl.h b/source/common/tracing/http_tracer_impl.h index 14cb47cbeb6a..760b4ed2bf3e 100644 --- a/source/common/tracing/http_tracer_impl.h +++ b/source/common/tracing/http_tracer_impl.h @@ -169,6 +169,8 @@ class NullSpan : public Span { void log(SystemTime, const std::string&) override {} void finishSpan() override {} void injectContext(Http::RequestHeaderMap&) override {} + void setBaggage(absl::string_view, absl::string_view) override {} + std::string getBaggage(absl::string_view) override { return std::string(); } SpanPtr spawnChild(const Config&, const std::string&, SystemTime) override { return SpanPtr{new NullSpan()}; } diff --git a/source/extensions/tracers/common/ot/opentracing_driver_impl.cc b/source/extensions/tracers/common/ot/opentracing_driver_impl.cc index 080742af2ddd..cad01b83bb83 100644 --- a/source/extensions/tracers/common/ot/opentracing_driver_impl.cc +++ b/source/extensions/tracers/common/ot/opentracing_driver_impl.cc @@ -102,6 +102,14 @@ void OpenTracingSpan::log(SystemTime timestamp, const std::string& event) { finish_options_.log_records.emplace_back(std::move(record)); } +void OpenTracingSpan::setBaggage(absl::string_view key, absl::string_view value) { + span_->SetBaggageItem({key.data(), key.length()}, {value.data(), value.length()}); +} + +std::string OpenTracingSpan::getBaggage(absl::string_view key) { + return span_->BaggageItem({key.data(), key.length()}); +} + void OpenTracingSpan::injectContext(Http::RequestHeaderMap& request_headers) { if (driver_.propagationMode() == OpenTracingDriver::PropagationMode::SingleHeader) { // Inject the span context using Envoy's single-header format. diff --git a/source/extensions/tracers/common/ot/opentracing_driver_impl.h b/source/extensions/tracers/common/ot/opentracing_driver_impl.h index d99ad7444dc5..2bfbddfe1886 100644 --- a/source/extensions/tracers/common/ot/opentracing_driver_impl.h +++ b/source/extensions/tracers/common/ot/opentracing_driver_impl.h @@ -40,6 +40,8 @@ class OpenTracingSpan : public Tracing::Span, Logger::Loggable { */ void log(Envoy::SystemTime, const std::string&) override {} + // X-Ray doesn't support baggage, so noop these OpenTracing functions. + void setBaggage(absl::string_view, absl::string_view) override {} + std::string getBaggage(absl::string_view) override { return std::string(); } + /** * Creates a child span. * In X-Ray terms this creates a sub-segment and sets its parent ID to the current span's ID. diff --git a/source/extensions/tracers/zipkin/zipkin_tracer_impl.cc b/source/extensions/tracers/zipkin/zipkin_tracer_impl.cc index a96d91aab565..8cf176d1fabc 100644 --- a/source/extensions/tracers/zipkin/zipkin_tracer_impl.cc +++ b/source/extensions/tracers/zipkin/zipkin_tracer_impl.cc @@ -35,6 +35,10 @@ void ZipkinSpan::log(SystemTime timestamp, const std::string& event) { span_.log(timestamp, event); } +// TODO(#11622): Implement baggage storage for zipkin spans +void ZipkinSpan::setBaggage(absl::string_view, absl::string_view) {} +std::string ZipkinSpan::getBaggage(absl::string_view) { return std::string(); } + void ZipkinSpan::injectContext(Http::RequestHeaderMap& request_headers) { // Set the trace-id and span-id headers properly, based on the newly-created span structure. request_headers.setReferenceKey(ZipkinCoreConstants::get().X_B3_TRACE_ID, diff --git a/source/extensions/tracers/zipkin/zipkin_tracer_impl.h b/source/extensions/tracers/zipkin/zipkin_tracer_impl.h index 1624ddf59cc5..9cb39ea27e92 100644 --- a/source/extensions/tracers/zipkin/zipkin_tracer_impl.h +++ b/source/extensions/tracers/zipkin/zipkin_tracer_impl.h @@ -76,6 +76,10 @@ class ZipkinSpan : public Tracing::Span { void setSampled(bool sampled) override; + // TODO(#11622): Implement baggage storage for zipkin spans + void setBaggage(absl::string_view, absl::string_view) override; + std::string getBaggage(absl::string_view) override; + /** * @return a reference to the Zipkin::Span object. */ diff --git a/test/common/tracing/http_tracer_impl_test.cc b/test/common/tracing/http_tracer_impl_test.cc index 6bb18da079f8..ef1686bcc688 100644 --- a/test/common/tracing/http_tracer_impl_test.cc +++ b/test/common/tracing/http_tracer_impl_test.cc @@ -744,6 +744,8 @@ TEST(HttpNullTracerTest, BasicFunctionality) { span_ptr->setOperation("foo"); span_ptr->setTag("foo", "bar"); + span_ptr->setBaggage("key", "value"); + ASSERT_EQ("", span_ptr->getBaggage("baggage_key")); span_ptr->injectContext(request_headers); EXPECT_NE(nullptr, span_ptr->spawnChild(config, "foo", SystemTime())); diff --git a/test/extensions/tracers/common/ot/opentracing_driver_impl_test.cc b/test/extensions/tracers/common/ot/opentracing_driver_impl_test.cc index 102bc6a2086c..011030dff5a4 100644 --- a/test/extensions/tracers/common/ot/opentracing_driver_impl_test.cc +++ b/test/extensions/tracers/common/ot/opentracing_driver_impl_test.cc @@ -99,6 +99,20 @@ TEST_F(OpenTracingDriverTest, FlushSpanWithLog) { EXPECT_EQ(expected_logs, driver_->recorder().top().logs); } +TEST_F(OpenTracingDriverTest, FlushSpanWithBaggage) { + setupValidDriver(); + + Tracing::SpanPtr first_span = driver_->startSpan(config_, request_headers_, operation_name_, + start_time_, {Tracing::Reason::Sampling, true}); + first_span->setBaggage("abc", "123"); + first_span->finishSpan(); + + const std::map expected_baggage = {{"abc", "123"}}; + + EXPECT_EQ(1, driver_->recorder().spans().size()); + EXPECT_EQ(expected_baggage, driver_->recorder().top().span_context.baggage); +} + TEST_F(OpenTracingDriverTest, TagSamplingFalseByDecision) { setupValidDriver(OpenTracingDriver::PropagationMode::TracerNative, {}); diff --git a/test/extensions/tracers/lightstep/lightstep_tracer_impl_test.cc b/test/extensions/tracers/lightstep/lightstep_tracer_impl_test.cc index caba59fce679..ef657d6d54f5 100644 --- a/test/extensions/tracers/lightstep/lightstep_tracer_impl_test.cc +++ b/test/extensions/tracers/lightstep/lightstep_tracer_impl_test.cc @@ -718,6 +718,17 @@ TEST_F(LightStepDriverTest, SpawnChild) { EXPECT_FALSE(base2_context.empty()); } +TEST_F(LightStepDriverTest, GetAndSetBaggage) { + setupValidDriver(); + Tracing::SpanPtr span = driver_->startSpan(config_, request_headers_, operation_name_, + start_time_, {Tracing::Reason::Sampling, true}); + + std::string key = "key1"; + std::string value = "value1"; + span->setBaggage(key, value); + EXPECT_EQ(span->getBaggage(key), value); +} + } // namespace } // namespace Lightstep } // namespace Tracers diff --git a/test/extensions/tracers/opencensus/tracer_test.cc b/test/extensions/tracers/opencensus/tracer_test.cc index 6bc91183341a..88ed7f2f5983 100644 --- a/test/extensions/tracers/opencensus/tracer_test.cc +++ b/test/extensions/tracers/opencensus/tracer_test.cc @@ -123,6 +123,10 @@ TEST(OpenCensusTracerTest, Span) { child->finishSpan(); span->setSampled(false); // Abandon tracer. span->finishSpan(); + + // Baggage methods are a noop in opencensus and won't affect events. + span->setBaggage("baggage_key", "baggage_value"); + ASSERT_EQ("", span->getBaggage("baggage_key")); } // Retrieve SpanData from the OpenCensus trace exporter. diff --git a/test/extensions/tracers/xray/tracer_test.cc b/test/extensions/tracers/xray/tracer_test.cc index c82e73056a5f..caeb153def47 100644 --- a/test/extensions/tracers/xray/tracer_test.cc +++ b/test/extensions/tracers/xray/tracer_test.cc @@ -100,6 +100,16 @@ TEST_F(XRayTracerTest, NonSampledSpansNotSerialized) { span->finishSpan(); } +TEST_F(XRayTracerTest, BaggageNotImplemented) { + Tracer tracer{"" /*span name*/, std::move(broker_), server_.timeSource()}; + auto span = tracer.createNonSampledSpan(); + span->setBaggage("baggage_key", "baggage_value"); + span->finishSpan(); + + // Baggage isn't supported so getBaggage should always return empty + ASSERT_EQ("", span->getBaggage("baggage_key")); +} + TEST_F(XRayTracerTest, ChildSpanHasParentInfo) { NiceMock config; constexpr auto expected_span_name = "Service 1"; diff --git a/test/extensions/tracers/zipkin/zipkin_tracer_impl_test.cc b/test/extensions/tracers/zipkin/zipkin_tracer_impl_test.cc index bd51f1493e5c..0d1488e63bff 100644 --- a/test/extensions/tracers/zipkin/zipkin_tracer_impl_test.cc +++ b/test/extensions/tracers/zipkin/zipkin_tracer_impl_test.cc @@ -689,6 +689,14 @@ TEST_F(ZipkinDriverTest, ZipkinSpanTest) { EXPECT_FALSE(zipkin_zipkin_span4.annotations().empty()); EXPECT_EQ(timestamp_count, zipkin_zipkin_span4.annotations().back().timestamp()); EXPECT_EQ("abc", zipkin_zipkin_span4.annotations().back().value()); + + // ==== + // Test baggage noop + // ==== + Tracing::SpanPtr span5 = driver_->startSpan(config_, request_headers_, operation_name_, + start_time_, {Tracing::Reason::Sampling, true}); + span5->setBaggage("baggage_key", "baggage_value"); + EXPECT_EQ("", span5->getBaggage("baggage_key")); } TEST_F(ZipkinDriverTest, ZipkinSpanContextFromB3HeadersTest) { diff --git a/test/mocks/tracing/mocks.h b/test/mocks/tracing/mocks.h index 8531027c7543..98a7a96ac513 100644 --- a/test/mocks/tracing/mocks.h +++ b/test/mocks/tracing/mocks.h @@ -37,6 +37,8 @@ class MockSpan : public Span { MOCK_METHOD(void, finishSpan, ()); MOCK_METHOD(void, injectContext, (Http::RequestHeaderMap & request_headers)); MOCK_METHOD(void, setSampled, (const bool sampled)); + MOCK_METHOD(void, setBaggage, (absl::string_view key, absl::string_view value)); + MOCK_METHOD(std::string, getBaggage, (absl::string_view key)); SpanPtr spawnChild(const Config& config, const std::string& name, SystemTime start_time) override {