tracing: Enable decorated operation in outbound (egress) listener to … (

#1858)

tracing: Enable decorated operation in outbound (egress) listener to be passed to 
inbound (ingress) listener to override server span operation.

Signed-off-by: Gary Brown <gary@brownuk.com>

docs/configuration/http_conn_man/route_config/route.rst

@@ -450,7 +450,9 @@ Specifies the route's decorator.
 
 operation
   *(required, string)* The operation name associated with the request matched to this route. If tracing is
-  enabled, this information will be used as the span name reported for this request.
+  enabled, this information will be used as the span name reported for this request. NOTE: For ingress
+  (inbound) requests this value may be overridden by the
+  :ref:`x-envoy-decorator-operation <config_http_filters_router_x-envoy-decorator-operation>` request header.
 
 .. _config_http_conn_man_route_table_route_add_req_headers:
 

docs/configuration/http_filters/router_filter.rst

@@ -236,6 +236,14 @@ if a request was dropped due to either :ref:`maintenance mode
 <config_http_filters_router_runtime_maintenance_mode>` or upstream :ref:`circuit breaking
 <arch_overview_circuit_break>`.
 
+.. _config_http_filters_router_x-envoy-decorator-operation:
+
+x-envoy-decorator-operation
+^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+If this header is present on ingress requests, its value will override any locally defined
+operation (span) name on the server span generated by the tracing mechanism.
+
 .. _config_http_filters_router_stats:
 
 Statistics

docs/intro/arch_overview/tracing.rst

@@ -90,7 +90,9 @@ associated with it. Each span generated by Envoy contains the following data:
 * Tracing system-specific metadata.
 
 The span also includes a name (or operation) which by default is defined as the host of the invoked service.
-However this can be customized using a :ref:`config_http_conn_man_route_table_decorator` on the route.
+However this can be customized using a :ref:`config_http_conn_man_route_table_decorator` on the route. The
+name used for the server (ingress) span can also be overridden using the
+:ref:`config_http_filters_router_x-envoy-decorator-operation` header.
 
 Envoy automatically sends spans to tracing collectors. Depending on the tracing collector,
 multiple spans are stitched together using common information such as the globally unique

include/envoy/http/header_map.h

@@ -215,6 +215,7 @@ class HeaderEntry {
   HEADER_FUNC(ContentLength)                                                                       \
   HEADER_FUNC(ContentType)                                                                         \
   HEADER_FUNC(Date)                                                                                \
+  HEADER_FUNC(EnvoyDecoratorOperation)                                                             \
   HEADER_FUNC(EnvoyDownstreamServiceCluster)                                                       \
   HEADER_FUNC(EnvoyDownstreamServiceNode)                                                          \
   HEADER_FUNC(EnvoyExpectedRequestTimeoutMs)                                                       \

include/envoy/router/router.h

@@ -382,6 +382,12 @@ class Decorator {
    * @param Tracing::Span& the span.
    */
   virtual void apply(Tracing::Span& span) const PURE;
+
+  /**
+   * This method returns the operation name.
+   * @return the operation name
+   */
+  virtual const std::string& getOperation() const PURE;
 };
 
 typedef std::unique_ptr<const Decorator> DecoratorConstPtr;

source/common/http/conn_manager_impl.cc

@@ -540,25 +540,58 @@ void ConnectionManagerImpl::ActiveStream::decodeHeaders(HeaderMapPtr&& headers,
 
   // Check if tracing is enabled at all.
   if (connection_manager_.config_.tracingConfig()) {
-    Tracing::Decision tracing_decision =
-        Tracing::HttpTracerUtility::isTracing(request_info_, *request_headers_);
-    ConnectionManagerImpl::chargeTracingStats(tracing_decision.reason,
-                                              connection_manager_.config_.tracingStats());
-
-    if (tracing_decision.is_tracing) {
-      active_span_ = connection_manager_.tracer_.startSpan(*this, *request_headers_, request_info_);
-      if (cached_route_.value() && cached_route_.value()->decorator()) {
-        cached_route_.value()->decorator()->apply(*active_span_);
-      }
-      active_span_->injectContext(*request_headers_);
-    }
+    traceRequest();
   }
 
   // Set the trusted address for the connection by taking the last address in XFF.
   request_info_.downstream_address_ = Utility::getLastAddressFromXFF(*request_headers_);
   decodeHeaders(nullptr, *request_headers_, end_stream);
 }
 
+void ConnectionManagerImpl::ActiveStream::traceRequest() {
+  Tracing::Decision tracing_decision =
+      Tracing::HttpTracerUtility::isTracing(request_info_, *request_headers_);
+  ConnectionManagerImpl::chargeTracingStats(tracing_decision.reason,
+                                            connection_manager_.config_.tracingStats());
+
+  if (!tracing_decision.is_tracing) {
+    return;
+  }
+
+  active_span_ = connection_manager_.tracer_.startSpan(*this, *request_headers_, request_info_);
+
+  // If a decorator has been defined, apply it to the active span.
+  if (cached_route_.value() && cached_route_.value()->decorator()) {
+    cached_route_.value()->decorator()->apply(*active_span_);
+
+    // For egress (outbound) requests, pass the decorator's operation name (if defined)
+    // as a request header to enable the receiving service to use it in its server span.
+    if (connection_manager_.config_.tracingConfig()->operation_name_ ==
+            Tracing::OperationName::Egress &&
+        !cached_route_.value()->decorator()->getOperation().empty()) {
+      request_headers_->insertEnvoyDecoratorOperation().value(
+          cached_route_.value()->decorator()->getOperation());
+    }
+  }
+
+  const HeaderEntry* decorator_operation = request_headers_->EnvoyDecoratorOperation();
+
+  // For ingress (inbound) requests, if a decorator operation name has been provided, it
+  // should be used to override the active span's operation.
+  if (connection_manager_.config_.tracingConfig()->operation_name_ ==
+          Tracing::OperationName::Ingress &&
+      decorator_operation) {
+    if (!decorator_operation->value().empty()) {
+      active_span_->setOperation(decorator_operation->value().c_str());
+    }
+    // Remove header so not propagated to service
+    request_headers_->removeEnvoyDecoratorOperation();
+  }
+
+  // Inject the active span's tracing context into the request headers.
+  active_span_->injectContext(*request_headers_);
+}
+
 void ConnectionManagerImpl::ActiveStream::decodeHeaders(ActiveStreamDecoderFilter* filter,
                                                         HeaderMap& headers, bool end_stream) {
   std::list<ActiveStreamDecoderFilterPtr>::iterator entry;

source/common/http/conn_manager_impl.h

@@ -520,6 +520,8 @@ class ConnectionManagerImpl : Logger::Loggable<Logger::Id::http>,
     virtual Tracing::OperationName operationName() const override;
     virtual const std::vector<Http::LowerCaseString>& requestHeadersForTags() const override;
 
+    void traceRequest();
+
     // Pass on watermark callbacks to watermark subscribers.  This boils down to passing watermark
     // events for this stream and the downstream connection to the router filter.
     void callHighWatermarkCallbacks();

source/common/http/headers.h

@@ -51,6 +51,7 @@ class HeaderValues {
   const LowerCaseString EnvoyExpectedRequestTimeoutMs{"x-envoy-expected-rq-timeout-ms"};
   const LowerCaseString EnvoyUpstreamServiceTime{"x-envoy-upstream-service-time"};
   const LowerCaseString EnvoyUpstreamHealthCheckedCluster{"x-envoy-upstream-healthchecked-cluster"};
+  const LowerCaseString EnvoyDecoratorOperation{"x-envoy-decorator-operation"};
   const LowerCaseString Expect{"expect"};
   const LowerCaseString ForwardedClientCert{"x-forwarded-client-cert"};
   const LowerCaseString ForwardedFor{"x-forwarded-for"};

source/common/router/config_impl.cc

@@ -213,6 +213,8 @@ void DecoratorImpl::apply(Tracing::Span& span) const {
   }
 }
 
+const std::string& DecoratorImpl::getOperation() const { return operation_; }
+
 const uint64_t RouteEntryImplBase::WeightedClusterEntry::MAX_CLUSTER_WEIGHT = 100UL;
 
 RouteEntryImplBase::RouteEntryImplBase(const VirtualHostImpl& vhost,

source/common/router/config_impl.h

@@ -274,6 +274,9 @@ class DecoratorImpl : public Decorator {
   // Decorator::apply
   void apply(Tracing::Span& span) const override;
 
+  // Decorator::getOperation
+  const std::string& getOperation() const override;
+
 private:
   const std::string operation_;
 };

test/common/http/conn_manager_impl_test.cc

@@ -395,6 +395,70 @@ TEST_F(HttpConnectionManagerImplTest, StartAndFinishSpanNormalFlow) {
   EXPECT_CALL(*span, setTag(":method", "GET"));
   // Verify if the activeSpan interface returns reference to the current span.
   EXPECT_CALL(*span, setTag("service-cluster", "scoobydoo"));
+  EXPECT_CALL(runtime_.snapshot_, featureEnabled("tracing.global_enabled", 100, _))
+      .WillOnce(Return(true));
+  EXPECT_CALL(*span, setOperation("testOp"));
+
+  std::shared_ptr<MockStreamDecoderFilter> filter(new NiceMock<MockStreamDecoderFilter>());
+
+  EXPECT_CALL(filter_factory_, createFilterChain(_))
+      .WillRepeatedly(Invoke([&](FilterChainFactoryCallbacks& callbacks) -> void {
+        callbacks.addStreamDecoderFilter(filter);
+      }));
+
+  // Treat request as internal, otherwise x-request-id header will be overwritten.
+  use_remote_address_ = false;
+  EXPECT_CALL(random_, uuid()).Times(0);
+
+  StreamDecoder* decoder = nullptr;
+  NiceMock<MockStreamEncoder> encoder;
+  EXPECT_CALL(*codec_, dispatch(_)).WillRepeatedly(Invoke([&](Buffer::Instance& data) -> void {
+    decoder = &conn_manager_->newStream(encoder);
+
+    HeaderMapPtr headers{
+        new TestHeaderMapImpl{{":method", "GET"},
+                              {":authority", "host"},
+                              {":path", "/"},
+                              {"x-request-id", "125a4afb-6f55-a4ba-ad80-413f09f48a28"},
+                              {"x-envoy-decorator-operation", "testOp"}}};
+    decoder->decodeHeaders(std::move(headers), true);
+
+    HeaderMapPtr response_headers{new TestHeaderMapImpl{{":status", "200"}}};
+    filter->callbacks_->encodeHeaders(std::move(response_headers), true);
+    filter->callbacks_->activeSpan().setTag("service-cluster", "scoobydoo");
+
+    data.drain(4);
+  }));
+
+  Buffer::OwnedImpl fake_input("1234");
+  conn_manager_->onData(fake_input);
+
+  EXPECT_EQ(1UL, tracing_stats_.service_forced_.value());
+  EXPECT_EQ(0UL, tracing_stats_.random_sampling_.value());
+}
+
+TEST_F(HttpConnectionManagerImplTest, StartAndFinishSpanNormalFlowEgress) {
+  setup(false, "");
+  tracing_config_.reset(new TracingConnectionManagerConfig(
+      {Tracing::OperationName::Egress, {LowerCaseString(":method")}}));
+
+  NiceMock<Tracing::MockSpan>* span = new NiceMock<Tracing::MockSpan>();
+  EXPECT_CALL(tracer_, startSpan_(_, _, _))
+      .WillOnce(Invoke([&](const Tracing::Config& config, const HeaderMap&,
+                           const AccessLog::RequestInfo&) -> Tracing::Span* {
+        EXPECT_EQ(Tracing::OperationName::Egress, config.operationName());
+
+        return span;
+      }));
+  route_config_provider_.route_config_->route_->decorator_.operation_ = "testOp";
+  EXPECT_CALL(*route_config_provider_.route_config_->route_, decorator()).Times(4);
+  EXPECT_CALL(route_config_provider_.route_config_->route_->decorator_, apply(_))
+      .WillOnce(
+          Invoke([&](const Tracing::Span& applyToSpan) -> void { EXPECT_EQ(span, &applyToSpan); }));
+  EXPECT_CALL(*span, finishSpan(_))
+      .WillOnce(
+          Invoke([span](Tracing::SpanFinalizer& finalizer) -> void { finalizer.finalize(*span); }));
+  EXPECT_CALL(*span, setTag(_, _)).Times(testing::AnyNumber());
   EXPECT_CALL(runtime_.snapshot_, featureEnabled("tracing.global_enabled", 100, _))
       .WillOnce(Return(true));
 
@@ -428,6 +492,13 @@ TEST_F(HttpConnectionManagerImplTest, StartAndFinishSpanNormalFlow) {
     data.drain(4);
   }));
 
+  EXPECT_CALL(*filter, decodeHeaders(_, true))
+      .WillOnce(Invoke([](HeaderMap& headers, bool) -> FilterHeadersStatus {
+        EXPECT_NE(nullptr, headers.EnvoyDecoratorOperation());
+        EXPECT_STREQ("testOp", headers.EnvoyDecoratorOperation()->value().c_str());
+        return FilterHeadersStatus::StopIteration;
+      }));
+
   Buffer::OwnedImpl fake_input("1234");
   conn_manager_->onData(fake_input);
 

test/mocks/router/mocks.cc

@@ -78,7 +78,9 @@ MockConfig::MockConfig() : route_(new NiceMock<MockRoute>()) {
 
 MockConfig::~MockConfig() {}
 
-MockDecorator::MockDecorator() { ON_CALL(*this, operation()).WillByDefault(ReturnRef(operation_)); }
+MockDecorator::MockDecorator() {
+  ON_CALL(*this, getOperation()).WillByDefault(ReturnRef(operation_));
+}
 MockDecorator::~MockDecorator() {}
 
 MockRoute::MockRoute() {

test/mocks/router/mocks.h

@@ -228,7 +228,7 @@ class MockDecorator : public Decorator {
   ~MockDecorator();
 
   // Router::Decorator
-  MOCK_CONST_METHOD0(operation, const std::string&());
+  MOCK_CONST_METHOD0(getOperation, const std::string&());
   MOCK_CONST_METHOD1(apply, void(Tracing::Span& span));
 
   std::string operation_{"fake_operation"};