Egress support v2 for tracing (#157)

docs/configuration/http_conn_man/http_conn_man.rst

@@ -17,7 +17,7 @@ HTTP connection manager
       "route_config": "{...}",
       "filters": [],
       "add_user_agent": "...",
-      "tracing_enabled": "...",
+      "tracing": "{...}",
       "http_codec_options": "...",
       "server_name": "...",
       "idle_timeout_s": "...",
@@ -71,12 +71,12 @@ add_user_agent
   :ref:`config_http_conn_man_headers_downstream-service-cluster` headers. See the linked
   documentation for more information. Defaults to false.
 
-.. _config_http_conn_man_tracing_enabled:
-
-tracing_enabled
-  *(optional, boolean)* Whether the connection manager emits :ref:`tracing <arch_overview_tracing>`
-  data to the :ref:`configured tracing provider <config_tracing>`. Defaults to false.
+.. _config_http_conn_man_tracing:
 
+:ref:`tracing <config_http_conn_man_tracing_format>`
+  *(optional, object)* Presence of the object defines whether the connection manager
+  emits :ref:`tracing <arch_overview_tracing>` data to the :ref:`configured tracing provider <config_tracing>`.
+
 .. _config_http_conn_man_http_codec_options:
 
 http_codec_options
@@ -137,6 +137,32 @@ generate_request_id
   *true*. Generating a random UUID4 is expensive so in high throughput scenarios where this
   feature is not desired it can be disabled.
 
+.. _config_http_conn_man_tracing_format:
+
+Tracing configuration
+^^^^^^^^^^^^^^^^^^^^^
+.. code-block:: json
+  
+  {
+    "tracing": {
+      "operation_name": "...",
+      "type": "..."
+    }
+  }
+ 
+operation_name
+  *(required, string)* Span name that will be emitted on completed request.
+
+type
+  *(optional, string)* Allows filtering of requests so that only some of them are traced. Default 
+  value is *all*. Possible values are:
+
+  all
+    Trace all requests.
+
+  upstream_failure
+    Trace only requests for which an upstream failure occurred.
+
 .. toctree::
   :hidden:
 

docs/intro/arch_overview/tracing.rst

@@ -22,8 +22,8 @@ sources of latency. Envoy supports three features related to system wide tracing
 
 How to initiate a trace
 -----------------------
-The HTTP connection manager that handles the request must have the :ref:`tracing_enabled
-<config_http_conn_man_tracing_enabled>` option set. There are several ways tracing can be 
+The HTTP connection manager that handles the request must have the :ref:`tracing
+<config_http_conn_man_tracing>` object set. There are several ways tracing can be 
 initiated:
 
 * By an external client via the :ref:`config_http_conn_man_headers_x-client-trace-id`

include/envoy/tracing/http_tracer.h

@@ -6,6 +6,16 @@
 
 namespace Tracing {
 
+/*
+ * Context used by tracers, it carries additional data needed to populate the trace.
+ */
+class TracingContext {
+public:
+  virtual ~TracingContext() {}
+
+  virtual const std::string& operationName() const PURE;
+};
+
 /**
  * Http sink for traces. Sink is responsible for delivering trace to the collector.
  */
@@ -15,7 +25,8 @@ class HttpSink {
 
   virtual void flushTrace(const Http::HeaderMap& request_headers,
                           const Http::HeaderMap& response_headers,
-                          const Http::AccessLog::RequestInfo& request_info) PURE;
+                          const Http::AccessLog::RequestInfo& request_info,
+                          const TracingContext& tracing_context) PURE;
 };
 
 typedef std::unique_ptr<HttpSink> HttpSinkPtr;
@@ -30,7 +41,8 @@ class HttpTracer {
   virtual void addSink(HttpSinkPtr&& sink) PURE;
   virtual void trace(const Http::HeaderMap* request_headers,
                      const Http::HeaderMap* response_headers,
-                     const Http::AccessLog::RequestInfo& request_info) PURE;
+                     const Http::AccessLog::RequestInfo& request_info,
+                     const TracingContext& tracing_context) PURE;
 };
 
 typedef std::unique_ptr<HttpTracer> HttpTracerPtr;

source/common/http/conn_manager_impl.cc

@@ -286,9 +286,10 @@ ConnectionManagerImpl::ActiveStream::~ActiveStream() {
     access_log->log(request_headers_.get(), response_headers_.get(), request_info_);
   }
 
-  if (connection_manager_.config_.isTracing()) {
+  if (ConnectionManagerUtility::shouldTraceRequest(request_info_,
+                                                   connection_manager_.config_.tracingConfig())) {
     connection_manager_.tracer_.trace(request_headers_.get(), response_headers_.get(),
-                                      request_info_);
+                                      request_info_, *this);
   }
 }
 
@@ -640,6 +641,10 @@ void ConnectionManagerImpl::ActiveStream::onResetStream(StreamResetReason) {
       removeFromList(connection_manager_.streams_));
 }
 
+const std::string& ConnectionManagerImpl::ActiveStream::operationName() const {
+  return connection_manager_.config_.tracingConfig().value().operation_name_;
+}
+
 void ConnectionManagerImpl::ActiveStreamFilterBase::addResetStreamCallback(
     std::function<void()> callback) {
   parent_.reset_callbacks_.push_back(callback);

source/common/http/conn_manager_impl.h

@@ -76,6 +76,23 @@ struct ConnectionManagerStats {
   Stats::Store& store_;
 };
 
+enum class TracingType {
+  // Trace all traceable requests.
+  All,
+  // Trace only when there is an upstream failure reason.
+  UpstreamFailure
+};
+
+/**
+ * Configuration for tracing which is set on the connection manager level.
+ * Http Tracing can be enabled/disabled on a per connection manager basis.
+ * Here we specify some specific for connection manager settings.
+ */
+struct TracingConnectionManagerConfig {
+  std::string operation_name_;
+  TracingType tracing_type_;
+};
+
 /**
  * Abstract configuration for the connection manager.
  */
@@ -159,9 +176,9 @@ class ConnectionManagerConfig {
   virtual const Optional<std::string>& userAgent() PURE;
 
   /**
-   * @return true if tracing is enabled otherwise it returns false;
+   * @return tracing config.
    */
-  virtual bool isTracing() PURE;
+  virtual const Optional<TracingConnectionManagerConfig>& tracingConfig() PURE;
 };
 
 /**
@@ -319,7 +336,8 @@ class ConnectionManagerImpl : Logger::Loggable<Logger::Id::http>,
                         public Event::DeferredDeletable,
                         public StreamCallbacks,
                         public StreamDecoder,
-                        public FilterChainFactoryCallbacks {
+                        public FilterChainFactoryCallbacks,
+                        public Tracing::TracingContext {
     ActiveStream(ConnectionManagerImpl& connection_manager);
     ~ActiveStream();
 
@@ -349,6 +367,9 @@ class ConnectionManagerImpl : Logger::Loggable<Logger::Id::http>,
     void addStreamEncoderFilter(StreamEncoderFilterPtr filter) override;
     void addStreamFilter(StreamFilterPtr filter) override;
 
+    // Tracing::TracingContext
+    virtual const std::string& operationName() const override;
+
     static DateFormatter date_formatter_;
 
     // All state for the stream. Put here for readability. We could move this to a bit field

source/common/http/conn_manager_utility.cc

@@ -104,7 +104,7 @@ void ConnectionManagerUtility::mutateRequestHeaders(Http::HeaderMap& request_hea
     }
   }
 
-  if (config.isTracing()) {
+  if (config.tracingConfig().valid()) {
     Tracing::HttpTracerUtility::mutateHeaders(request_headers, runtime);
   }
 }
@@ -131,4 +131,23 @@ void ConnectionManagerUtility::mutateResponseHeaders(Http::HeaderMap& response_h
   }
 }
 
+bool ConnectionManagerUtility::shouldTraceRequest(
+    const Http::AccessLog::RequestInfo& request_info,
+    const Optional<TracingConnectionManagerConfig>& config) {
+  if (!config.valid()) {
+    return false;
+  }
+
+  switch (config.value().tracing_type_) {
+  case Http::TracingType::All:
+    return true;
+  case Http::TracingType::UpstreamFailure:
+    return request_info.failureReason() != Http::AccessLog::FailureReason::None;
+  }
+
+  // Compiler enforces switch above to cover all the cases and it's impossible to be here,
+  // but compiler complains on missing return statement, this is to make compiler happy.
+  NOT_IMPLEMENTED;
+}
+
 } // Http

source/common/http/conn_manager_utility.h

@@ -19,6 +19,9 @@ class ConnectionManagerUtility {
   static void mutateResponseHeaders(Http::HeaderMap& response_headers,
                                     const Http::HeaderMap& request_headers,
                                     ConnectionManagerConfig& config);
+
+  static bool shouldTraceRequest(const Http::AccessLog::RequestInfo& request_info,
+                                 const Optional<TracingConnectionManagerConfig>& config);
 };
 
 } // Http

source/common/tracing/http_tracer_impl.cc

@@ -3,6 +3,7 @@
 #include "common/common/macros.h"
 #include "common/common/utility.h"
 #include "common/grpc/common.h"
+#include "common/http/access_log/access_log_formatter.h"
 #include "common/http/codes.h"
 #include "common/http/headers.h"
 #include "common/http/header_map_impl.h"
@@ -72,7 +73,8 @@ void HttpTracerImpl::addSink(HttpSinkPtr&& sink) { sinks_.push_back(std::move(si
 
 void HttpTracerImpl::trace(const Http::HeaderMap* request_headers,
                            const Http::HeaderMap* response_headers,
-                           const Http::AccessLog::RequestInfo& request_info) {
+                           const Http::AccessLog::RequestInfo& request_info,
+                           const TracingContext& tracing_context) {
   static const Http::HeaderMapImpl empty_headers;
   if (!request_headers) {
     request_headers = &empty_headers;
@@ -90,7 +92,7 @@ void HttpTracerImpl::trace(const Http::HeaderMap* request_headers,
     stats_.doing_tracing_.inc();
 
     for (HttpSinkPtr& sink : sinks_) {
-      sink->flushTrace(*request_headers, *response_headers, request_info);
+      sink->flushTrace(*request_headers, *response_headers, request_info, tracing_context);
     }
   }
 }
@@ -226,29 +228,34 @@ std::string LightStepSink::buildResponseCode(const Http::AccessLog::RequestInfo&
 }
 
 void LightStepSink::flushTrace(const Http::HeaderMap& request_headers, const Http::HeaderMap&,
-                               const Http::AccessLog::RequestInfo& request_info) {
+                               const Http::AccessLog::RequestInfo& request_info,
+                               const TracingContext& tracing_context) {
   lightstep::Span span = tls_.getTyped<TlsLightStepTracer>(tls_slot_).tracer_.StartSpan(
-      "full request",
-      {
-       lightstep::StartTimestamp(request_info.startTime()),
+      tracing_context.operationName(),
+      {lightstep::StartTimestamp(request_info.startTime()),
        lightstep::SetTag("join:x-request-id", request_headers.get(Http::Headers::get().RequestId)),
        lightstep::SetTag("request line", buildRequestLine(request_headers, request_info)),
        lightstep::SetTag("response code", buildResponseCode(request_info)),
+       lightstep::SetTag("host header", request_headers.get(Http::Headers::get().Host)),
        lightstep::SetTag(
            "downstream cluster",
            StringUtil::valueOrDefault(
                request_headers.get(Http::Headers::get().EnvoyDownstreamServiceCluster), "-")),
        lightstep::SetTag(
            "user agent",
            StringUtil::valueOrDefault(request_headers.get(Http::Headers::get().UserAgent), "-")),
-       lightstep::SetTag("node id", service_node_),
-      });
+       lightstep::SetTag("node id", service_node_)});
 
   if (request_info.responseCode().valid() &&
       Http::CodeUtility::is5xx(request_info.responseCode().value())) {
     span.SetTag("error", "true");
   }
 
+  if (request_info.failureReason() != Http::AccessLog::FailureReason::None) {
+    span.SetTag("failure reason",
+                Http::AccessLog::FilterReasonUtils::toShortString(request_info.failureReason()));
+  }
+
   if (request_headers.has(Http::Headers::get().ClientTraceId)) {
     span.SetTag("join:x-client-trace-id", request_headers.get(Http::Headers::get().ClientTraceId));
   }

source/common/tracing/http_tracer_impl.h

@@ -40,8 +40,8 @@ class HttpNullTracer : public HttpTracer {
 public:
   // Tracing::HttpTracer
   void addSink(HttpSinkPtr&&) override {}
-  void trace(const Http::HeaderMap*, const Http::HeaderMap*,
-             const Http::AccessLog::RequestInfo&) override {}
+  void trace(const Http::HeaderMap*, const Http::HeaderMap*, const Http::AccessLog::RequestInfo&,
+             const TracingContext&) override {}
 };
 
 enum class Reason {
@@ -81,7 +81,8 @@ class HttpTracerImpl : public HttpTracer {
   // Tracing::HttpTracer
   void addSink(HttpSinkPtr&& sink) override;
   void trace(const Http::HeaderMap* request_headers, const Http::HeaderMap* response_headers,
-             const Http::AccessLog::RequestInfo& request_info) override;
+             const Http::AccessLog::RequestInfo& request_info,
+             const TracingContext& tracing_context) override;
 
 private:
   void populateStats(const Decision& decision);
@@ -105,7 +106,8 @@ class LightStepSink : public HttpSink {
 
   // Tracer::HttpSink
   void flushTrace(const Http::HeaderMap& request_headers, const Http::HeaderMap& response_headers,
-                  const Http::AccessLog::RequestInfo& request_info) override;
+                  const Http::AccessLog::RequestInfo& request_info,
+                  const TracingContext& tracing_context) override;
 
   Upstream::ClusterManager& clusterManager() { return cm_; }
   const std::string& collectorCluster() { return collector_cluster_; }

source/server/config/network/http_connection_manager.cc

@@ -86,7 +86,21 @@ HttpConnectionManagerConfig::HttpConnectionManagerConfig(const Json::Object& con
     user_agent_.value(server.options().serviceClusterName());
   }
 
-  is_tracing_ = config.getBoolean("tracing_enabled", false);
+  if (config.hasObject("tracing")) {
+    const std::string operation_name = config.getObject("tracing").getString("operation_name");
+
+    std::string tracing_type = config.getObject("tracing").getString("type", "all");
+    Http::TracingType type;
+    if (tracing_type == "all") {
+      type = Http::TracingType::All;
+    } else if (tracing_type == "upstream_failure") {
+      type = Http::TracingType::UpstreamFailure;
+    } else {
+      throw EnvoyException(fmt::format("unsupported tracing type '{}'", tracing_type));
+    }
+
+    tracing_config_.value({operation_name, type});
+  }
 
   if (config.hasObject("idle_timeout_s")) {
     idle_timeout_.value(std::chrono::seconds(config.getInteger("idle_timeout_s")));

source/server/config/network/http_connection_manager.h

@@ -70,7 +70,9 @@ class HttpConnectionManagerConfig : Logger::Loggable<Logger::Id::config>,
   const std::string& serverName() override { return server_name_; }
   Http::ConnectionManagerStats& stats() override { return stats_; }
   bool useRemoteAddress() override { return use_remote_address_; }
-  bool isTracing() override { return is_tracing_; }
+  const Optional<Http::TracingConnectionManagerConfig>& tracingConfig() override {
+    return tracing_config_;
+  }
   const std::string& localAddress() override;
   const Optional<std::string>& userAgent() override { return user_agent_; }
 
@@ -99,7 +101,7 @@ class HttpConnectionManagerConfig : Logger::Loggable<Logger::Id::config>,
   CodecType codec_type_;
   const uint64_t codec_options_;
   std::string server_name_;
-  bool is_tracing_;
+  Optional<Http::TracingConnectionManagerConfig> tracing_config_;
   Optional<std::string> user_agent_;
   Optional<std::chrono::milliseconds> idle_timeout_;
   Router::ConfigPtr route_config_;

source/server/configuration_impl.cc

@@ -92,12 +92,12 @@ void MainImpl::initializeTracers(const Json::Object& tracing_configuration_) {
               server_.options().serviceNodeName(), server_.threadLocal(), server_.runtime(),
               std::move(opts))});
         } else {
-          throw EnvoyException(fmt::format("Unsupported sink type: '{}'", type));
+          throw EnvoyException(fmt::format("unsupported sink type: '{}'", type));
         }
       }
     }
   } else {
-    throw EnvoyException("Incorrect tracing configuration");
+    throw EnvoyException("incorrect tracing configuration");
   }
 }