Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tracing: add direct support for a span to spawn a child #932

Merged
merged 11 commits into from
May 18, 2017
8 changes: 6 additions & 2 deletions include/envoy/tracing/http_tracer.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,16 +34,20 @@ class Config {
/*
* Basic abstraction for span.
*/
class Span;

typedef std::unique_ptr<Span> SpanPtr;

class Span {
public:
virtual ~Span() {}

virtual void setTag(const std::string& name, const std::string& value) PURE;
Copy link
Member

@RomanDzhabarov RomanDzhabarov May 17, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comments here in all these methods, sorry I've not put it in the first place

virtual void finishSpan() PURE;
virtual void injectContext(Http::HeaderMap& request_headers) PURE;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about overrides for zipkin spans?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated

virtual SpanPtr spawnChild(const std::string& name, SystemTime start_time) PURE;
};

typedef std::unique_ptr<Span> SpanPtr;

/**
* Tracing driver is responsible for span creation.
*/
Expand Down
1 change: 1 addition & 0 deletions source/common/http/conn_manager_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -476,6 +476,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_);
}
}

Expand Down
2 changes: 2 additions & 0 deletions source/common/tracing/http_tracer_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,8 @@ 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 { return SpanPtr{new NullSpan()}; }
};

} // Tracing
Expand Down
30 changes: 19 additions & 11 deletions source/common/tracing/lightstep_tracer_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,30 @@
namespace Envoy {
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(); }

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) {
lightstep::Span ls_span = tracer_.StartSpan(
name, {lightstep::ChildOf(span_.context()), lightstep::StartTimestamp(start_time)});
return SpanPtr{new LightStepSpan(ls_span, tracer_)};
}

LightStepRecorder::LightStepRecorder(const lightstep::TracerImpl& tracer, LightStepDriver& driver,
Event::Dispatcher& dispatcher)
: builder_(tracer), driver_(driver) {
Expand Down Expand Up @@ -134,21 +150,13 @@ 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.
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()));

return std::move(active_span);
}

Expand Down
5 changes: 4 additions & 1 deletion source/common/tracing/lightstep_tracer_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,16 +29,19 @@ 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;
void injectContext(Http::HeaderMap& request_headers) 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<LightStepSpan> LightStepSpanPtr;
Expand Down
43 changes: 25 additions & 18 deletions source/common/tracing/zipkin/zipkin_tracer_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
namespace Envoy {
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(); }

Expand All @@ -22,6 +22,29 @@ 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) {
std::unique_ptr<SpanContext> context{new SpanContext(span_)};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this can be allocated on a stack. SpanContext context(span_);

return Tracing::SpanPtr{new ZipkinSpan(*tracer_.startSpan(name, start_time, *context), tracer_)};
}

bool ZipkinSpan::hasCSAnnotation() {
auto annotations = span_.annotations();
if (annotations.size() > 0) {
Expand Down Expand Up @@ -94,24 +117,8 @@ 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());

ZipkinSpanPtr active_span;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor, ZipkinSpanPtr active_span(new ZipkinSpan(*new_zipkin_span, tracer));
and remove reset.

active_span.reset(new ZipkinSpan(*new_zipkin_span));
active_span.reset(new ZipkinSpan(*new_zipkin_span, tracer));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: ZipkinSpanPtr active_span(new ZipkinSpan(*new_zipkin_span, tracer))

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think this is still a valid comment :)


return std::move(active_span);
}
Expand Down
5 changes: 4 additions & 1 deletion source/common/tracing/zipkin/zipkin_tracer_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,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.
Expand All @@ -59,6 +59,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.
*/
Expand All @@ -71,6 +73,7 @@ class ZipkinSpan : public Tracing::Span {

private:
Zipkin::Span span_;
Zipkin::Tracer& tracer_;
};

typedef std::unique_ptr<ZipkinSpan> ZipkinSpanPtr;
Expand Down
27 changes: 27 additions & 0 deletions test/common/tracing/lightstep_tracer_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,9 @@ TEST_F(LightStepDriverTest, SerializeAndDeserializeContext) {
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();
EXPECT_FALSE(injected_ctx.empty());

Expand All @@ -286,9 +289,33 @@ 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
} // Envoy
3 changes: 3 additions & 0 deletions test/common/tracing/zipkin/zipkin_tracer_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,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());

Expand Down
7 changes: 7 additions & 0 deletions test/mocks/tracing/mocks.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,13 @@ 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)};
}

MOCK_METHOD2(spawnChild_, Span*(const std::string& name, SystemTime start_time));
};

class MockHttpTracer : public HttpTracer {
Expand Down