From a0c8036cb7883b98467d64b28010fbcb583e611b Mon Sep 17 00:00:00 2001 From: Mike Schore Date: Tue, 9 May 2017 15:58:15 -0700 Subject: [PATCH 1/8] tracing: add direct support for a span to spawn a child --- include/envoy/tracing/http_tracer.h | 8 ++++++-- source/common/tracing/http_tracer_impl.h | 5 +++++ .../common/tracing/lightstep_tracer_impl.cc | 19 +++++++++++++++---- source/common/tracing/lightstep_tracer_impl.h | 4 +++- test/mocks/tracing/mocks.h | 8 +++++++- 5 files changed, 36 insertions(+), 8 deletions(-) diff --git a/include/envoy/tracing/http_tracer.h b/include/envoy/tracing/http_tracer.h index 295d24c9bca4..cc9e2a47f560 100644 --- a/include/envoy/tracing/http_tracer.h +++ b/include/envoy/tracing/http_tracer.h @@ -33,16 +33,20 @@ class Config { /* * Basic abstraction for span. */ +class Span; + +typedef std::unique_ptr SpanPtr; + class Span { public: virtual ~Span() {} virtual void setTag(const std::string& name, const std::string& value) PURE; virtual void finishSpan() PURE; + // virtual void inject(Http::HeaderMap& request_headers) PURE; + virtual SpanPtr spawnChild(const std::string& name, SystemTime start_time) PURE; }; -typedef std::unique_ptr SpanPtr; - /** * Tracing driver is responsible for span creation. */ diff --git a/source/common/tracing/http_tracer_impl.h b/source/common/tracing/http_tracer_impl.h index c2a3ba4c909b..3291ecf96046 100644 --- a/source/common/tracing/http_tracer_impl.h +++ b/source/common/tracing/http_tracer_impl.h @@ -87,6 +87,11 @@ class NullSpan : public Tracing::Span { // Tracing::Span void setTag(const std::string&, const std::string&) override {} void finishSpan() override {} + SpanPtr spawnChild(const std::string&, SystemTime) override { + SpanPtr nullSpan; + nullSpan.reset(new NullSpan()); + return std::move(nullSpan); + } }; } // Tracing diff --git a/source/common/tracing/lightstep_tracer_impl.cc b/source/common/tracing/lightstep_tracer_impl.cc index f0aefbe493b6..888beac38a45 100644 --- a/source/common/tracing/lightstep_tracer_impl.cc +++ b/source/common/tracing/lightstep_tracer_impl.cc @@ -14,7 +14,8 @@ namespace Tracing { -LightStepSpan::LightStepSpan(lightstep::Span& span) : span_(span) {} +LightStepSpan::LightStepSpan(lightstep::Span& span, lightstep::Tracer& tracer) + : span_(span), tracer_(tracer) {} void LightStepSpan::finishSpan() { span_.Finish(); } @@ -22,6 +23,15 @@ void LightStepSpan::setTag(const std::string& name, const std::string& value) { span_.SetTag(name, value); } +SpanPtr LightStepSpan::spawnChild(const std::string& name, SystemTime start_time) { + SpanPtr child_span; + lightstep::Span ls_span = tracer_.StartSpan( + name, {lightstep::ChildOf(span_.context()), lightstep::StartTimestamp(start_time)}); + child_span.reset(new LightStepSpan(ls_span, tracer_)); + + return std::move(child_span); +} + LightStepRecorder::LightStepRecorder(const lightstep::TracerImpl& tracer, LightStepDriver& driver, Event::Dispatcher& dispatcher) : builder_(tracer), driver_(driver) { @@ -133,20 +143,21 @@ SpanPtr LightStepDriver::startSpan(Http::HeaderMap& request_headers, lightstep::Span ls_span = tracer.StartSpan(operation_name, {lightstep::ChildOf(parent_span_ctx), lightstep::StartTimestamp(start_time)}); - active_span.reset(new LightStepSpan(ls_span)); + active_span.reset(new LightStepSpan(ls_span, tracer)); } else { lightstep::Span ls_span = tracer.StartSpan(operation_name, {lightstep::StartTimestamp(start_time)}); - active_span.reset(new LightStepSpan(ls_span)); + active_span.reset(new LightStepSpan(ls_span, tracer)); } - // Inject newly created span context into HTTP carrier. + // Inject newly created span context into HTTP carrier. TODO: move to inject() lightstep::BinaryCarrier ctx; tracer.Inject(active_span->context(), lightstep::CarrierFormat::LightStepBinaryCarrier, lightstep::ProtoWriter(&ctx)); const std::string current_span_context = ctx.SerializeAsString(); request_headers.insertOtSpanContext().value( Base64::encode(current_span_context.c_str(), current_span_context.length())); + // end TODO return std::move(active_span); } diff --git a/source/common/tracing/lightstep_tracer_impl.h b/source/common/tracing/lightstep_tracer_impl.h index 8c44f9d3168e..f01c03923112 100644 --- a/source/common/tracing/lightstep_tracer_impl.h +++ b/source/common/tracing/lightstep_tracer_impl.h @@ -28,16 +28,18 @@ struct LightstepTracerStats { class LightStepSpan : public Span { public: - LightStepSpan(lightstep::Span& span); + LightStepSpan(lightstep::Span& span, lightstep::Tracer& tracer); // Tracing::Span void finishSpan() override; void setTag(const std::string& name, const std::string& value) override; + SpanPtr spawnChild(const std::string& name, SystemTime start_time) override; lightstep::SpanContext context() { return span_.context(); } private: lightstep::Span span_; + lightstep::Tracer& tracer_; }; typedef std::unique_ptr LightStepSpanPtr; diff --git a/test/mocks/tracing/mocks.h b/test/mocks/tracing/mocks.h index c0181932f32a..97dcdfc2cf4c 100644 --- a/test/mocks/tracing/mocks.h +++ b/test/mocks/tracing/mocks.h @@ -33,6 +33,12 @@ class MockSpan : public Span { MOCK_METHOD2(setTag, void(const std::string& name, const std::string& value)); MOCK_METHOD0(finishSpan, void()); + + SpanPtr spawnChild(const std::string& name, SystemTime start_time) override { + return SpanPtr{spawnChild_(name, start_time)}; + } + + MOCK_METHOD2(spawnChild_, Span*(const std::string& name, SystemTime start_time)); }; class MockHttpTracer : public HttpTracer { @@ -63,4 +69,4 @@ class MockDriver : public Driver { const std::string& operation_name, SystemTime start_time)); }; -} // Tracing \ No newline at end of file +} // Tracing From d483a33fa7c3e9acea795ffaf92c7ea98c8d4aac Mon Sep 17 00:00:00 2001 From: Mike Schore Date: Mon, 15 May 2017 09:52:48 -0700 Subject: [PATCH 2/8] add inject to Span interface and test --- include/envoy/tracing/http_tracer.h | 2 +- source/common/http/conn_manager_impl.cc | 1 + source/common/tracing/http_tracer_impl.h | 1 + .../common/tracing/lightstep_tracer_impl.cc | 18 ++++++------- source/common/tracing/lightstep_tracer_impl.h | 1 + .../tracing/lightstep_tracer_impl_test.cc | 25 +++++++++++++++++++ test/mocks/tracing/mocks.h | 1 + 7 files changed, 39 insertions(+), 10 deletions(-) diff --git a/include/envoy/tracing/http_tracer.h b/include/envoy/tracing/http_tracer.h index cc9e2a47f560..f05fdc7b2b86 100644 --- a/include/envoy/tracing/http_tracer.h +++ b/include/envoy/tracing/http_tracer.h @@ -43,7 +43,7 @@ class Span { virtual void setTag(const std::string& name, const std::string& value) PURE; virtual void finishSpan() PURE; - // virtual void inject(Http::HeaderMap& request_headers) PURE; + virtual void injectContext(Http::HeaderMap& request_headers) PURE; virtual SpanPtr spawnChild(const std::string& name, SystemTime start_time) PURE; }; diff --git a/source/common/http/conn_manager_impl.cc b/source/common/http/conn_manager_impl.cc index 52045a617964..a4beb8481e50 100644 --- a/source/common/http/conn_manager_impl.cc +++ b/source/common/http/conn_manager_impl.cc @@ -463,6 +463,7 @@ void ConnectionManagerImpl::ActiveStream::decodeHeaders(HeaderMapPtr&& headers, if (tracing_decision.is_tracing) { active_span_ = connection_manager_.tracer_.startSpan(*this, *request_headers_, request_info_); + active_span_->injectContext(*request_headers_); } } diff --git a/source/common/tracing/http_tracer_impl.h b/source/common/tracing/http_tracer_impl.h index 3291ecf96046..bc44cbdffe49 100644 --- a/source/common/tracing/http_tracer_impl.h +++ b/source/common/tracing/http_tracer_impl.h @@ -87,6 +87,7 @@ class NullSpan : public Tracing::Span { // Tracing::Span void setTag(const std::string&, const std::string&) override {} void finishSpan() override {} + void injectContext(Http::HeaderMap&) override {} SpanPtr spawnChild(const std::string&, SystemTime) override { SpanPtr nullSpan; nullSpan.reset(new NullSpan()); diff --git a/source/common/tracing/lightstep_tracer_impl.cc b/source/common/tracing/lightstep_tracer_impl.cc index 888beac38a45..6cdaa5908af1 100644 --- a/source/common/tracing/lightstep_tracer_impl.cc +++ b/source/common/tracing/lightstep_tracer_impl.cc @@ -23,6 +23,15 @@ void LightStepSpan::setTag(const std::string& name, const std::string& value) { span_.SetTag(name, value); } +void LightStepSpan::injectContext(Http::HeaderMap& request_headers) { + lightstep::BinaryCarrier ctx; + tracer_.Inject(context(), lightstep::CarrierFormat::LightStepBinaryCarrier, + lightstep::ProtoWriter(&ctx)); + const std::string current_span_context = ctx.SerializeAsString(); + request_headers.insertOtSpanContext().value( + Base64::encode(current_span_context.c_str(), current_span_context.length())); +} + SpanPtr LightStepSpan::spawnChild(const std::string& name, SystemTime start_time) { SpanPtr child_span; lightstep::Span ls_span = tracer_.StartSpan( @@ -150,15 +159,6 @@ SpanPtr LightStepDriver::startSpan(Http::HeaderMap& request_headers, active_span.reset(new LightStepSpan(ls_span, tracer)); } - // Inject newly created span context into HTTP carrier. TODO: move to inject() - lightstep::BinaryCarrier ctx; - tracer.Inject(active_span->context(), lightstep::CarrierFormat::LightStepBinaryCarrier, - lightstep::ProtoWriter(&ctx)); - const std::string current_span_context = ctx.SerializeAsString(); - request_headers.insertOtSpanContext().value( - Base64::encode(current_span_context.c_str(), current_span_context.length())); - // end TODO - return std::move(active_span); } diff --git a/source/common/tracing/lightstep_tracer_impl.h b/source/common/tracing/lightstep_tracer_impl.h index f01c03923112..e7bb6a4e20c4 100644 --- a/source/common/tracing/lightstep_tracer_impl.h +++ b/source/common/tracing/lightstep_tracer_impl.h @@ -33,6 +33,7 @@ class LightStepSpan : public Span { // Tracing::Span void finishSpan() override; void setTag(const std::string& name, const std::string& value) override; + void injectContext(Http::HeaderMap& request_headers) override; SpanPtr spawnChild(const std::string& name, SystemTime start_time) override; lightstep::SpanContext context() { return span_.context(); } diff --git a/test/common/tracing/lightstep_tracer_impl_test.cc b/test/common/tracing/lightstep_tracer_impl_test.cc index c5b1a6273132..4f40ce0b8453 100644 --- a/test/common/tracing/lightstep_tracer_impl_test.cc +++ b/test/common/tracing/lightstep_tracer_impl_test.cc @@ -274,6 +274,7 @@ TEST_F(LightStepDriverTest, SerializeAndDeserializeContext) { // Supply empty context. request_headers_.removeOtSpanContext(); SpanPtr span = driver_->startSpan(request_headers_, operation_name_, start_time_); + span->injectContext(request_headers_); injected_ctx = request_headers_.OtSpanContext()->value().c_str(); EXPECT_FALSE(injected_ctx.empty()); @@ -285,8 +286,32 @@ TEST_F(LightStepDriverTest, SerializeAndDeserializeContext) { // Supply parent context, request_headers has properly populated x-ot-span-context. SpanPtr span_with_parent = driver_->startSpan(request_headers_, operation_name_, start_time_); + request_headers_.removeOtSpanContext(); + span_with_parent->injectContext(request_headers_); injected_ctx = request_headers_.OtSpanContext()->value().c_str(); EXPECT_FALSE(injected_ctx.empty()); } +TEST_F(LightStepDriverTest, SpawnChild) { + setupValidDriver(); + + SpanPtr parent = driver_->startSpan(request_headers_, operation_name_, start_time_); + parent->injectContext(request_headers_); + + SpanPtr childViaHeaders = driver_->startSpan(request_headers_, operation_name_, start_time_); + SpanPtr childViaSpawn = parent->spawnChild(operation_name_, start_time_); + + Http::TestHeaderMapImpl base1{{":path", "/"}, {":method", "GET"}, {"x-request-id", "foo"}}; + Http::TestHeaderMapImpl base2{{":path", "/"}, {":method", "GET"}, {"x-request-id", "foo"}}; + + childViaHeaders->injectContext(base1); + childViaSpawn->injectContext(base2); + + std::string base1_context = Base64::decode(base1.OtSpanContext()->value().c_str()); + std::string base2_context = Base64::decode(base2.OtSpanContext()->value().c_str()); + + EXPECT_FALSE(base1_context.empty()); + EXPECT_FALSE(base2_context.empty()); +} + } // Tracing diff --git a/test/mocks/tracing/mocks.h b/test/mocks/tracing/mocks.h index 97dcdfc2cf4c..6312b77203c1 100644 --- a/test/mocks/tracing/mocks.h +++ b/test/mocks/tracing/mocks.h @@ -33,6 +33,7 @@ class MockSpan : public Span { MOCK_METHOD2(setTag, void(const std::string& name, const std::string& value)); MOCK_METHOD0(finishSpan, void()); + MOCK_METHOD1(injectContext, void(Http::HeaderMap& request_headers)); SpanPtr spawnChild(const std::string& name, SystemTime start_time) override { return SpanPtr{spawnChild_(name, start_time)}; From 4badb3b42bfb27b686b6fe6891f612c77689fe74 Mon Sep 17 00:00:00 2001 From: Mike Schore Date: Tue, 16 May 2017 14:51:25 -0700 Subject: [PATCH 3/8] add zipkin support --- .../tracing/zipkin/zipkin_tracer_impl.cc | 47 ++++++++++++------- .../tracing/zipkin/zipkin_tracer_impl.h | 5 +- .../tracing/lightstep_tracer_impl_test.cc | 2 + .../tracing/zipkin/zipkin_tracer_impl_test.cc | 3 ++ 4 files changed, 39 insertions(+), 18 deletions(-) diff --git a/source/common/tracing/zipkin/zipkin_tracer_impl.cc b/source/common/tracing/zipkin/zipkin_tracer_impl.cc index fa7adbcfdef6..e2cfdc30df8f 100644 --- a/source/common/tracing/zipkin/zipkin_tracer_impl.cc +++ b/source/common/tracing/zipkin/zipkin_tracer_impl.cc @@ -11,7 +11,7 @@ namespace Zipkin { -ZipkinSpan::ZipkinSpan(Zipkin::Span& span) : span_(span) {} +ZipkinSpan::ZipkinSpan(Zipkin::Span& span, Zipkin::Tracer& tracer) : span_(span), tracer_(tracer) {} void ZipkinSpan::finishSpan() { span_.finish(); } @@ -21,6 +21,33 @@ void ZipkinSpan::setTag(const std::string& name, const std::string& value) { } } +void ZipkinSpan::injectContext(Http::HeaderMap& request_headers) { + // Set the trace-id and span-id headers properly, based on the newly-created span structure. + request_headers.insertXB3TraceId().value(span_.traceIdAsHexString()); + request_headers.insertXB3SpanId().value(span_.idAsHexString()); + + // Set the parent-span header properly, based on the newly-created span structure. + if (span_.isSetParentId()) { + request_headers.insertXB3ParentSpanId().value(span_.parentIdAsHexString()); + } + + // Set the sampled header. + request_headers.insertXB3Sampled().value(ZipkinCoreConstants::get().ALWAYS_SAMPLE); + + // Set the ot-span-context header with the new context. + SpanContext context(span_); + request_headers.insertOtSpanContext().value(context.serializeToString()); +} + +Tracing::SpanPtr ZipkinSpan::spawnChild(const std::string& name, SystemTime start_time) { + Tracing::SpanPtr child_span; + SpanContext context; + new SpanContext(span_); + child_span.reset(new ZipkinSpan(*tracer_.startSpan(name, start_time, context), tracer_)); + + return std::move(child_span); +} + bool ZipkinSpan::hasCSAnnotation() { auto annotations = span_.annotations(); if (annotations.size() > 0) { @@ -93,24 +120,10 @@ Tracing::SpanPtr Driver::startSpan(Http::HeaderMap& request_headers, const std:: new_zipkin_span = tracer.startSpan(request_headers.Host()->value().c_str(), start_time); } - // Set the trace-id and span-id headers properly, based on the newly-created span structure. - request_headers.insertXB3TraceId().value(new_zipkin_span->traceIdAsHexString()); - request_headers.insertXB3SpanId().value(new_zipkin_span->idAsHexString()); - - // Set the parent-span header properly, based on the newly-created span structure. - if (new_zipkin_span->isSetParentId()) { - request_headers.insertXB3ParentSpanId().value(new_zipkin_span->parentIdAsHexString()); - } - - // Set the sampled header. - request_headers.insertXB3Sampled().value(ZipkinCoreConstants::get().ALWAYS_SAMPLE); - - // Set the ot-span-context header with the new context. - SpanContext new_span_context(*new_zipkin_span); - request_headers.insertOtSpanContext().value(new_span_context.serializeToString()); + // TODO(MS): REMOVE THIS: MOVED TO INJECT ZipkinSpanPtr active_span; - active_span.reset(new ZipkinSpan(*new_zipkin_span)); + active_span.reset(new ZipkinSpan(*new_zipkin_span, tracer)); return std::move(active_span); } diff --git a/source/common/tracing/zipkin/zipkin_tracer_impl.h b/source/common/tracing/zipkin/zipkin_tracer_impl.h index 9f7c128d7004..3dc3c72a420b 100644 --- a/source/common/tracing/zipkin/zipkin_tracer_impl.h +++ b/source/common/tracing/zipkin/zipkin_tracer_impl.h @@ -34,7 +34,7 @@ class ZipkinSpan : public Tracing::Span { * * @param span to be wrapped. */ - ZipkinSpan(Zipkin::Span& span); + ZipkinSpan(Zipkin::Span& span, Zipkin::Tracer& tracer); /** * Calls Zipkin::Span::finishSpan() to perform all actions needed to finalize the span. @@ -58,6 +58,8 @@ class ZipkinSpan : public Tracing::Span { */ void setTag(const std::string& name, const std::string& value) override; + void injectContext(Http::HeaderMap& request_headers) override; + Tracing::SpanPtr spawnChild(const std::string& name, SystemTime start_time) override; /** * @return true if this span has a CS (Client Send) basic annotation, or false otherwise. */ @@ -70,6 +72,7 @@ class ZipkinSpan : public Tracing::Span { private: Zipkin::Span span_; + Zipkin::Tracer& tracer_; }; typedef std::unique_ptr ZipkinSpanPtr; diff --git a/test/common/tracing/lightstep_tracer_impl_test.cc b/test/common/tracing/lightstep_tracer_impl_test.cc index 4f40ce0b8453..77aa81e8745d 100644 --- a/test/common/tracing/lightstep_tracer_impl_test.cc +++ b/test/common/tracing/lightstep_tracer_impl_test.cc @@ -274,6 +274,8 @@ TEST_F(LightStepDriverTest, SerializeAndDeserializeContext) { // Supply empty context. request_headers_.removeOtSpanContext(); SpanPtr span = driver_->startSpan(request_headers_, operation_name_, start_time_); + + EXPECT_EQ(nullptr, request_headers_.OtSpanContext()); span->injectContext(request_headers_); injected_ctx = request_headers_.OtSpanContext()->value().c_str(); diff --git a/test/common/tracing/zipkin/zipkin_tracer_impl_test.cc b/test/common/tracing/zipkin/zipkin_tracer_impl_test.cc index 1d49471767bc..d4f4ec0f0c97 100644 --- a/test/common/tracing/zipkin/zipkin_tracer_impl_test.cc +++ b/test/common/tracing/zipkin/zipkin_tracer_impl_test.cc @@ -251,6 +251,9 @@ TEST_F(ZipkinDriverTest, SerializeAndDeserializeContext) { request_headers_.removeOtSpanContext(); Tracing::SpanPtr span = driver_->startSpan(request_headers_, operation_name_, start_time_); + EXPECT_EQ(nullptr, request_headers_.OtSpanContext()); + span->injectContext(request_headers_); + injected_ctx = request_headers_.OtSpanContext()->value().c_str(); EXPECT_FALSE(injected_ctx.empty()); From f6069c3db8954898383ca589a2ebda35652ae904 Mon Sep 17 00:00:00 2001 From: Mike Schore Date: Wed, 17 May 2017 10:44:13 -0700 Subject: [PATCH 4/8] fix --- source/common/tracing/zipkin/zipkin_tracer_impl.cc | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/source/common/tracing/zipkin/zipkin_tracer_impl.cc b/source/common/tracing/zipkin/zipkin_tracer_impl.cc index b47a4d249e0e..e8ab1ea23727 100644 --- a/source/common/tracing/zipkin/zipkin_tracer_impl.cc +++ b/source/common/tracing/zipkin/zipkin_tracer_impl.cc @@ -42,11 +42,9 @@ void ZipkinSpan::injectContext(Http::HeaderMap& request_headers) { Tracing::SpanPtr ZipkinSpan::spawnChild(const std::string& name, SystemTime start_time) { Tracing::SpanPtr child_span; - SpanContext context; - new SpanContext(span_); - child_span.reset(new ZipkinSpan(*tracer_.startSpan(name, start_time, context), tracer_)); - - return std::move(child_span); + std::unique_ptr context{new SpanContext(span_)}; + child_span.reset(new ZipkinSpan(*tracer_.startSpan(name, start_time, *context), tracer_)); + return child_span; } bool ZipkinSpan::hasCSAnnotation() { @@ -121,8 +119,6 @@ Tracing::SpanPtr Driver::startSpan(Http::HeaderMap& request_headers, const std:: new_zipkin_span = tracer.startSpan(request_headers.Host()->value().c_str(), start_time); } - // TODO(MS): REMOVE THIS: MOVED TO INJECT - ZipkinSpanPtr active_span; active_span.reset(new ZipkinSpan(*new_zipkin_span, tracer)); From 366264534edb0728525da03d36717ace7a7ed980 Mon Sep 17 00:00:00 2001 From: Mike Schore Date: Wed, 17 May 2017 13:03:53 -0700 Subject: [PATCH 5/8] switch syntax --- source/common/tracing/http_tracer_impl.h | 2 +- source/common/tracing/lightstep_tracer_impl.cc | 2 +- source/common/tracing/zipkin/zipkin_tracer_impl.cc | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/source/common/tracing/http_tracer_impl.h b/source/common/tracing/http_tracer_impl.h index ebce1b376648..0f74bebbea5c 100644 --- a/source/common/tracing/http_tracer_impl.h +++ b/source/common/tracing/http_tracer_impl.h @@ -89,7 +89,7 @@ class NullSpan : public Tracing::Span { void setTag(const std::string&, const std::string&) override {} void finishSpan() override {} void injectContext(Http::HeaderMap&) override {} - SpanPtr spawnChild(const std::string&, SystemTime) override { return SpanPtr(new NullSpan()); } + SpanPtr spawnChild(const std::string&, SystemTime) override { return SpanPtr{new NullSpan()}; } }; } // Tracing diff --git a/source/common/tracing/lightstep_tracer_impl.cc b/source/common/tracing/lightstep_tracer_impl.cc index 11ce37daa57d..d2bf732969c2 100644 --- a/source/common/tracing/lightstep_tracer_impl.cc +++ b/source/common/tracing/lightstep_tracer_impl.cc @@ -36,7 +36,7 @@ void LightStepSpan::injectContext(Http::HeaderMap& request_headers) { SpanPtr LightStepSpan::spawnChild(const std::string& name, SystemTime start_time) { lightstep::Span ls_span = tracer_.StartSpan( name, {lightstep::ChildOf(span_.context()), lightstep::StartTimestamp(start_time)}); - return SpanPtr(new LightStepSpan(ls_span, tracer_)); + return SpanPtr{new LightStepSpan(ls_span, tracer_)}; } LightStepRecorder::LightStepRecorder(const lightstep::TracerImpl& tracer, LightStepDriver& driver, diff --git a/source/common/tracing/zipkin/zipkin_tracer_impl.cc b/source/common/tracing/zipkin/zipkin_tracer_impl.cc index 1b5792ba1aa4..8dced014b341 100644 --- a/source/common/tracing/zipkin/zipkin_tracer_impl.cc +++ b/source/common/tracing/zipkin/zipkin_tracer_impl.cc @@ -42,7 +42,7 @@ void ZipkinSpan::injectContext(Http::HeaderMap& request_headers) { Tracing::SpanPtr ZipkinSpan::spawnChild(const std::string& name, SystemTime start_time) { std::unique_ptr context{new SpanContext(span_)}; - return Tracing::SpanPtr(new ZipkinSpan(*tracer_.startSpan(name, start_time, *context), tracer_)); + return Tracing::SpanPtr{new ZipkinSpan(*tracer_.startSpan(name, start_time, *context), tracer_)}; } bool ZipkinSpan::hasCSAnnotation() { From e40a1b094a023d9865769ed43180e8c9e7fc2175 Mon Sep 17 00:00:00 2001 From: Mike Schore Date: Wed, 17 May 2017 16:17:57 -0700 Subject: [PATCH 6/8] allocate context on stack --- source/common/tracing/zipkin/zipkin_tracer_impl.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source/common/tracing/zipkin/zipkin_tracer_impl.cc b/source/common/tracing/zipkin/zipkin_tracer_impl.cc index 8dced014b341..674324a7e642 100644 --- a/source/common/tracing/zipkin/zipkin_tracer_impl.cc +++ b/source/common/tracing/zipkin/zipkin_tracer_impl.cc @@ -41,8 +41,8 @@ void ZipkinSpan::injectContext(Http::HeaderMap& request_headers) { } Tracing::SpanPtr ZipkinSpan::spawnChild(const std::string& name, SystemTime start_time) { - std::unique_ptr context{new SpanContext(span_)}; - return Tracing::SpanPtr{new ZipkinSpan(*tracer_.startSpan(name, start_time, *context), tracer_)}; + SpanContext context(span_); + return Tracing::SpanPtr{new ZipkinSpan(*tracer_.startSpan(name, start_time, context), tracer_)}; } bool ZipkinSpan::hasCSAnnotation() { From 917698070f359c79bd18d5ab2ebd3d9b594eb516 Mon Sep 17 00:00:00 2001 From: Mike Schore Date: Wed, 17 May 2017 16:37:45 -0700 Subject: [PATCH 7/8] add Span documentation --- include/envoy/tracing/http_tracer.h | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/include/envoy/tracing/http_tracer.h b/include/envoy/tracing/http_tracer.h index 450d0beb94af..b55c8c4c492c 100644 --- a/include/envoy/tracing/http_tracer.h +++ b/include/envoy/tracing/http_tracer.h @@ -42,9 +42,31 @@ class Span { public: virtual ~Span() {} + /** + * Attach metadata to a Span, to be handled in an implementation-dependent fashion. + * @param name the name of the tag + * @param value the value to associate with the tag + */ virtual void setTag(const std::string& name, const std::string& value) PURE; + + /** + * Capture the final duration for this Span and carry out any work necessary to complete it. + * Once this method is called, the Span may be safely discarded. + */ virtual void finishSpan() PURE; + + /** + * 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 + */ virtual void injectContext(Http::HeaderMap& request_headers) PURE; + + /** + * Create and start a child Span, with this Span as its parent in the trace. + * @param name operation name captured by the spawned child + * @param start_time initial start time for the operation captured by the child + */ virtual SpanPtr spawnChild(const std::string& name, SystemTime start_time) PURE; }; From 683687a08a3448857803c23217bf3605757418fa Mon Sep 17 00:00:00 2001 From: Mike Schore Date: Thu, 18 May 2017 14:40:41 -0700 Subject: [PATCH 8/8] remove unnecessary reset --- source/common/tracing/zipkin/zipkin_tracer_impl.cc | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/source/common/tracing/zipkin/zipkin_tracer_impl.cc b/source/common/tracing/zipkin/zipkin_tracer_impl.cc index 674324a7e642..30dfcc658a10 100644 --- a/source/common/tracing/zipkin/zipkin_tracer_impl.cc +++ b/source/common/tracing/zipkin/zipkin_tracer_impl.cc @@ -117,9 +117,7 @@ Tracing::SpanPtr Driver::startSpan(Http::HeaderMap& request_headers, const std:: new_zipkin_span = tracer.startSpan(request_headers.Host()->value().c_str(), start_time); } - ZipkinSpanPtr active_span; - active_span.reset(new ZipkinSpan(*new_zipkin_span, tracer)); - + ZipkinSpanPtr active_span(new ZipkinSpan(*new_zipkin_span, tracer)); return std::move(active_span); }