Tracing: revamp extract/inject span context

include/envoy/http/header_map.h

@@ -206,6 +206,7 @@ class HeaderEntry {
   HEADER_FUNC(Host)                                                                                \
   HEADER_FUNC(KeepAlive)                                                                           \
   HEADER_FUNC(Method)                                                                              \
+  HEADER_FUNC(OtSpanContext)                                                                       \
   HEADER_FUNC(Path)                                                                                \
   HEADER_FUNC(ProxyConnection)                                                                     \
   HEADER_FUNC(RequestId)                                                                           \

include/envoy/tracing/http_tracer.h

@@ -36,7 +36,11 @@ class Driver {
 public:
   virtual ~Driver() {}
 
-  virtual SpanPtr startSpan(const std::string& operation_name, SystemTime start_time) PURE;
+  /**
+   * Start driver specific span.
+   */
+  virtual SpanPtr startSpan(Http::HeaderMap& request_headers, const std::string& operation_name,
+                            SystemTime start_time) PURE;
 };
 
 typedef std::unique_ptr<Driver> DriverPtr;
@@ -49,7 +53,7 @@ class HttpTracer {
 public:
   virtual ~HttpTracer() {}
 
-  virtual SpanPtr startSpan(const Config& config, const Http::HeaderMap& request_headers,
+  virtual SpanPtr startSpan(const Config& config, Http::HeaderMap& request_headers,
                             const Http::AccessLog::RequestInfo& request_info) PURE;
 };
 

source/common/http/conn_manager_impl.cc

@@ -313,7 +313,7 @@ ConnectionManagerImpl::ActiveStream::~ActiveStream() {
     if (request_info_.healthCheck()) {
       connection_manager_.config_.tracingStats().health_check_.inc();
     } else {
-      Tracing::HttpTracerUtility::finalizeSpan(*active_span_, request_info_);
+      Tracing::HttpTracerUtility::finalizeSpan(*active_span_, *request_headers_, request_info_);
     }
   }
 }

source/common/http/headers.h

@@ -44,6 +44,7 @@ class Headers {
   const LowerCaseString KeepAlive{"keep-alive"};
   const LowerCaseString Location{"location"};
   const LowerCaseString Method{":method"};
+  const LowerCaseString OtSpanContext{"x-ot-span-context"};
   const LowerCaseString Path{":path"};
   const LowerCaseString ProxyConnection{"proxy-connection"};
   const LowerCaseString RequestId{"x-request-id"};

source/common/tracing/http_tracer_impl.cc

@@ -1,5 +1,6 @@
 #include "http_tracer_impl.h"
 
+#include "common/common/base64.h"
 #include "common/common/macros.h"
 #include "common/common/utility.h"
 #include "common/grpc/common.h"
@@ -18,7 +19,7 @@ static std::string buildRequestLine(const Http::HeaderMap& request_headers,
   std::string path = request_headers.EnvoyOriginalPath()
                          ? request_headers.EnvoyOriginalPath()->value().c_str()
                          : request_headers.Path()->value().c_str();
-  static const size_t max_path_length = 256;
+  static const size_t max_path_length = 128;
 
   if (path.length() > max_path_length) {
     path = path.substr(0, max_path_length);
@@ -98,9 +99,9 @@ Decision HttpTracerUtility::isTracing(const Http::AccessLog::RequestInfo& reques
   throw std::invalid_argument("Unknown trace_status");
 }
 
-void HttpTracerUtility::populateSpan(Span& active_span, const std::string& service_node,
-                                     const Http::HeaderMap& request_headers,
+void HttpTracerUtility::finalizeSpan(Span& active_span, const Http::HeaderMap& request_headers,
                                      const Http::AccessLog::RequestInfo& request_info) {
+  // Pre response data.
   active_span.setTag("guid:x-request-id",
                      std::string(request_headers.RequestId()->value().c_str()));
   active_span.setTag("request_line", buildRequestLine(request_headers, request_info));
@@ -109,16 +110,13 @@ void HttpTracerUtility::populateSpan(Span& active_span, const std::string& servi
   active_span.setTag("downstream_cluster",
                      valueOrDefault(request_headers.EnvoyDownstreamServiceCluster(), "-"));
   active_span.setTag("user_agent", valueOrDefault(request_headers.UserAgent(), "-"));
-  active_span.setTag("node_id", service_node);
 
   if (request_headers.ClientTraceId()) {
     active_span.setTag("guid:x-client-trace-id",
                        std::string(request_headers.ClientTraceId()->value().c_str()));
   }
-}
 
-void HttpTracerUtility::finalizeSpan(Span& active_span,
-                                     const Http::AccessLog::RequestInfo& request_info) {
+  // Post response data.
   active_span.setTag("response_code", buildResponseCode(request_info));
   active_span.setTag("response_size", std::to_string(request_info.bytesSent()));
   active_span.setTag("response_flags",
@@ -135,12 +133,13 @@ void HttpTracerUtility::finalizeSpan(Span& active_span,
 HttpTracerImpl::HttpTracerImpl(DriverPtr&& driver, const LocalInfo::LocalInfo& local_info)
     : driver_(std::move(driver)), local_info_(local_info) {}
 
-SpanPtr HttpTracerImpl::startSpan(const Config& config, const Http::HeaderMap& request_headers,
+SpanPtr HttpTracerImpl::startSpan(const Config& config, Http::HeaderMap& request_headers,
                                   const Http::AccessLog::RequestInfo& request_info) {
-  SpanPtr active_span = driver_->startSpan(config.operationName(), request_info.startTime());
+  SpanPtr active_span =
+      driver_->startSpan(request_headers, config.operationName(), request_info.startTime());
   if (active_span) {
-    HttpTracerUtility::populateSpan(*active_span, local_info_.nodeName(), request_headers,
-                                    request_info);
+    active_span->setTag("node_id", local_info_.nodeName());
+    active_span->setTag("zone", local_info_.zoneName());
   }
 
   return active_span;
@@ -244,11 +243,39 @@ LightStepDriver::LightStepDriver(const Json::Object& config,
   });
 }
 
-SpanPtr LightStepDriver::startSpan(const std::string& operation_name, SystemTime start_time) {
-  lightstep::Span span = tls_.getTyped<TlsLightStepTracer>(tls_slot_).tracer_.StartSpan(
-      operation_name, {lightstep::StartTimestamp(start_time)});
+SpanPtr LightStepDriver::startSpan(Http::HeaderMap& request_headers,
+                                   const std::string& operation_name, SystemTime start_time) {
+  lightstep::Tracer& tracer = tls_.getTyped<TlsLightStepTracer>(tls_slot_).tracer_;
+  LightStepSpanPtr active_span;
+
+  if (request_headers.OtSpanContext()) {
+    // Extract downstream context from HTTP carrier.
+    // This code is safe even if decode returns empty string or data is malformed.
+    std::string parent_context = Base64::decode(request_headers.OtSpanContext()->value().c_str());
+    lightstep::envoy::CarrierStruct ctx;
+    ctx.ParseFromString(parent_context);
+
+    lightstep::SpanContext parent_span_ctx = tracer.Extract(
+        lightstep::CarrierFormat::EnvoyProtoCarrier, lightstep::envoy::ProtoReader(ctx));
+    lightstep::Span ls_span =
+        tracer.StartSpan(operation_name, {lightstep::ChildOf(parent_span_ctx),
+                                          lightstep::StartTimestamp(start_time)});
+    active_span.reset(new LightStepSpan(ls_span));
+  } else {
+    lightstep::Span ls_span =
+        tracer.StartSpan(operation_name, {lightstep::StartTimestamp(start_time)});
+    active_span.reset(new LightStepSpan(ls_span));
+  }
+
+  // Inject newly created span context into HTTP carrier.
+  lightstep::envoy::CarrierStruct ctx;
+  tracer.Inject(active_span->context(), lightstep::CarrierFormat::EnvoyProtoCarrier,
+                lightstep::envoy::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 SpanPtr{new LightStepSpan(span)};
+  return std::move(active_span);
 }
 
 void LightStepRecorder::onFailure(Http::AsyncClient::FailureReason) {

source/common/tracing/http_tracer_impl.h

@@ -9,6 +9,7 @@
 #include "common/http/header_map_impl.h"
 #include "common/json/json_loader.h"
 
+#include "lightstep/envoy.h"
 #include "lightstep/tracer.h"
 
 namespace Tracing {
@@ -50,25 +51,18 @@ class HttpTracerUtility {
    */
   static void mutateHeaders(Http::HeaderMap& request_headers, Runtime::Loader& runtime);
 
-  /**
-   * Fill in span tags based on request.
-   */
-  static void populateSpan(Span& active_span, const std::string& service_node,
-                           const Http::HeaderMap& request_headers,
-                           const Http::AccessLog::RequestInfo& request_info);
-
   /**
    * 1) Fill in span tags based on the response headers.
    * 2) Finish active span.
    */
-  static void finalizeSpan(Span& active_span, const Http::AccessLog::RequestInfo& request_info);
+  static void finalizeSpan(Span& active_span, const Http::HeaderMap& request_headers,
+                           const Http::AccessLog::RequestInfo& request_info);
 };
 
 class HttpNullTracer : public HttpTracer {
 public:
   // Tracing::HttpTracer
-  SpanPtr startSpan(const Config&, const Http::HeaderMap&,
-                    const Http::AccessLog::RequestInfo&) override {
+  SpanPtr startSpan(const Config&, Http::HeaderMap&, const Http::AccessLog::RequestInfo&) override {
     return nullptr;
   }
 };
@@ -78,7 +72,7 @@ class HttpTracerImpl : public HttpTracer {
   HttpTracerImpl(DriverPtr&& driver, const LocalInfo::LocalInfo& local_info);
 
   // Tracing::HttpTracer
-  SpanPtr startSpan(const Config& config, const Http::HeaderMap& request_headers,
+  SpanPtr startSpan(const Config& config, Http::HeaderMap& request_headers,
                     const Http::AccessLog::RequestInfo& request_info) override;
 
 private:
@@ -94,10 +88,14 @@ class LightStepSpan : public Span {
   void finishSpan() override;
   void setTag(const std::string& name, const std::string& value) override;
 
+  lightstep::SpanContext context() { return span_.context(); }
+
 private:
   lightstep::Span span_;
 };
 
+typedef std::unique_ptr<LightStepSpan> LightStepSpanPtr;
+
 /**
  * LightStep (http://lightstep.com/) provides tracing capabilities, aggregation, visualization of
  * application trace data.
@@ -111,7 +109,8 @@ class LightStepDriver : public Driver {
                   std::unique_ptr<lightstep::TracerOptions> options);
 
   // Tracer::TracingDriver
-  SpanPtr startSpan(const std::string& operation_name, SystemTime start_time) override;
+  SpanPtr startSpan(Http::HeaderMap& request_headers, const std::string& operation_name,
+                    SystemTime start_time) override;
 
   Upstream::ClusterManager& clusterManager() { return cm_; }
   Upstream::ClusterInfoPtr cluster() { return cluster_; }

test/common/http/conn_manager_impl_test.cc

@@ -229,7 +229,8 @@ TEST_F(HttpConnectionManagerImplTest, StartAndFinishSpanNormalFlow) {
         decoder = &conn_manager_->newStream(encoder);
 
         Http::HeaderMapPtr headers{
-            new TestHeaderMapImpl{{":authority", "host"},
+            new TestHeaderMapImpl{{":method", "GET"},
+                                  {":authority", "host"},
                                   {":path", "/"},
                                   {"x-request-id", "125a4afb-6f55-a4ba-ad80-413f09f48a28"}}};
         decoder->decodeHeaders(std::move(headers), true);
@@ -275,7 +276,8 @@ TEST_F(HttpConnectionManagerImplTest, TestAccessLog) {
         decoder = &conn_manager_->newStream(encoder);
 
         Http::HeaderMapPtr headers{
-            new TestHeaderMapImpl{{":authority", "host"},
+            new TestHeaderMapImpl{{":method", "GET"},
+                                  {":authority", "host"},
                                   {":path", "/"},
                                   {"x-request-id", "125a4afb-6f55-a4ba-ad80-413f09f48a28"}}};
         decoder->decodeHeaders(std::move(headers), true);
@@ -314,7 +316,8 @@ TEST_F(HttpConnectionManagerImplTest, DoNotStartSpanIfTracingIsNotEnabled) {
         decoder = &conn_manager_->newStream(encoder);
 
         Http::HeaderMapPtr headers{
-            new TestHeaderMapImpl{{":authority", "host"},
+            new TestHeaderMapImpl{{":method", "GET"},
+                                  {":authority", "host"},
                                   {":path", "/"},
                                   {"x-request-id", "125a4afb-6f55-a4ba-ad80-413f09f48a28"}}};
         decoder->decodeHeaders(std::move(headers), true);
@@ -333,8 +336,10 @@ TEST_F(HttpConnectionManagerImplTest, StartSpanOnlyHealthCheckRequest) {
   setup(false, "");
 
   NiceMock<Tracing::MockSpan>* span = new NiceMock<Tracing::MockSpan>();
+
   EXPECT_CALL(tracer_, startSpan_(_, _, _)).WillOnce(Return(span));
   EXPECT_CALL(*span, finishSpan()).Times(0);
+
   EXPECT_CALL(runtime_.snapshot_, featureEnabled("tracing.global_enabled", 100, _))
       .WillOnce(Return(true));
 
@@ -358,7 +363,8 @@ TEST_F(HttpConnectionManagerImplTest, StartSpanOnlyHealthCheckRequest) {
         decoder = &conn_manager_->newStream(encoder);
 
         Http::HeaderMapPtr headers{
-            new TestHeaderMapImpl{{":authority", "host"},
+            new TestHeaderMapImpl{{":method", "GET"},
+                                  {":authority", "host"},
                                   {":path", "/healthcheck"},
                                   {"x-request-id", "125a4afb-6f55-94ba-ad80-413f09f48a28"}}};
         decoder->decodeHeaders(std::move(headers), true);