tracing: add option to choice mutation policy for RequestID extension

api/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto

@@ -128,7 +128,7 @@ message HttpConnectionManager {
     UNESCAPE_AND_FORWARD = 4;
   }
 
-  // [#next-free-field: 10]
+  // [#next-free-field: 11]
   message Tracing {
     option (udpa.annotations.versioning).previous_message_type =
         "envoy.config.filter.network.http_connection_manager.v2.HttpConnectionManager.Tracing";
@@ -141,6 +141,21 @@ message HttpConnectionManager {
       EGRESS = 1;
     }
 
+    // This option is used to change the behavior of the sampling strategy decided
+    // based on the :ref:`x-request-id<config_http_conn_man_headers_x-request-id>`.
+    enum TraceRequestIdSampleDecisionPolicy {
+      // Default value. if specified as DEFAULT, the sampling strategy will be based
+      // on the value of :ref:`x-request-id<config_http_conn_man_headers_x-request-id>`.
+      DEFAULT = 0;
+
+      // Bypasses the :ref:`x-request-id<config_http_conn_man_headers_x-request-id>` to
+      // determine the sampling strategy and leaves all decisions to the tracing provider.
+      // If this value is set, the generated :ref:`x-request-id<config_http_conn_man_headers_x-request-id>`
+      // will not be rewritten. Also, it will no longer be affected by runtime flags
+      // for tracing decisions.
+      BYPASS = 1;
+    }
+
     reserved 1, 2;
 
     reserved "operation_name", "request_headers_for_tags";
@@ -193,6 +208,10 @@ message HttpConnectionManager {
     //   Such a constraint is inherent to OpenCensus itself. It cannot be overcome without changes
     //   on OpenCensus side.
     config.trace.v3.Tracing.Http provider = 9;
+
+    // How to determine the sampling strategy by
+    // :ref:`x-request-id<config_http_conn_man_headers_x-request-id>`.
+    TraceRequestIdSampleDecisionPolicy trace_request_id_sample_decision_policy = 10;
   }
 
   message InternalAddressConfig {

docs/root/version_history/current.rst

@@ -36,6 +36,7 @@ Minor Behavior Changes
 * listener: respect the :ref:`connection balance config <envoy_v3_api_field_config.listener.v3.Listener.connection_balance_config>`
   defined within the listener where the sockets are redirected to. Clear that field to restore the previous behavior.
 * tcp: switched to the new connection pool by default. Any unexpected behavioral changes can be reverted by setting runtime guard ``envoy.reloadable_features.new_tcp_connection_pool`` to false.
+* tracing: add option :ref:`trace_request_id_sample_decision_policy <envoy_v3_api_field_extensions.filters.network.http_connection_manager.v3.HttpConnectionManager.Tracing.trace_request_id_sample_decision_policy>` to determine :ref:`x-request-id<config_http_conn_man_headers_x-request-id>` based sampling decision policy.
 
 Bug Fixes
 ---------

envoy/tracing/trace_driver.h

@@ -18,6 +18,8 @@ constexpr uint32_t DefaultMaxPathTagLength = 256;
 
 enum class OperationName { Ingress, Egress };
 
+enum class TraceRequestIdSampleDecisionPolicy { Default, ByPass };
+
 /**
  * The context for the custom tag to obtain the tag value.
  */
@@ -77,6 +79,11 @@ class Config {
    * for HTTP protocol tracing.
    */
   virtual uint32_t maxPathTagLength() const PURE;
+
+  /**
+   * @return sampling decision policy based on x-request-id.
+   */
+  virtual TraceRequestIdSampleDecisionPolicy traceRequestIdSampleDecisionPolicy() const PURE;
 };
 
 /**

source/common/http/conn_manager_config.h

@@ -132,6 +132,7 @@ struct TracingConnectionManagerConfig {
   envoy::type::v3::FractionalPercent overall_sampling_;
   bool verbose_;
   uint32_t max_path_tag_length_;
+  Tracing::TraceRequestIdSampleDecisionPolicy trace_request_id_sample_decision_policy_;
 };
 
 using TracingConnectionManagerConfigPtr = std::unique_ptr<TracingConnectionManagerConfig>;

source/common/http/conn_manager_impl.cc

@@ -1585,6 +1585,11 @@ uint32_t ConnectionManagerImpl::ActiveStream::maxPathTagLength() const {
   return connection_manager_.config_.tracingConfig()->max_path_tag_length_;
 }
 
+Tracing::TraceRequestIdSampleDecisionPolicy
+ConnectionManagerImpl::ActiveStream::traceRequestIdSampleDecisionPolicy() const {
+  return connection_manager_.config_.tracingConfig()->trace_request_id_sample_decision_policy_;
+}
+
 const Router::RouteEntry::UpgradeMap* ConnectionManagerImpl::ActiveStream::upgradeMap() {
   // We must check if the 'cached_route_' optional is populated since this function can be called
   // early via sendLocalReply(), before the cached route is populated.

source/common/http/conn_manager_impl.h

@@ -201,6 +201,7 @@ class ConnectionManagerImpl : Logger::Loggable<Logger::Id::http>,
     const Tracing::CustomTagMap* customTags() const override;
     bool verbose() const override;
     uint32_t maxPathTagLength() const override;
+    Tracing::TraceRequestIdSampleDecisionPolicy traceRequestIdSampleDecisionPolicy() const override;
 
     // ScopeTrackedObject
     void dumpState(std::ostream& os, int indent_level = 0) const override {

source/common/http/conn_manager_utility.cc

@@ -281,6 +281,10 @@ Tracing::Reason ConnectionManagerUtility::mutateTracingRequestHeader(
   if (!config.tracingConfig()) {
     return final_reason;
   }
+  if (config.tracingConfig()->trace_request_id_sample_decision_policy_ ==
+      Tracing::TraceRequestIdSampleDecisionPolicy::ByPass) {
+    return Tracing::Reason::Sampling;
+  }
 
   auto rid_extension = config.requestIDExtension();
   const auto rid_to_integer = rid_extension->toInteger(request_headers);

source/common/tracing/http_tracer_impl.h

@@ -81,6 +81,9 @@ class EgressConfigImpl : public Config {
   const CustomTagMap* customTags() const override { return nullptr; }
   bool verbose() const override { return false; }
   uint32_t maxPathTagLength() const override { return Tracing::DefaultMaxPathTagLength; }
+  TraceRequestIdSampleDecisionPolicy traceRequestIdSampleDecisionPolicy() const override {
+    return Tracing::TraceRequestIdSampleDecisionPolicy::Default;
+  }
 };
 
 using EgressConfig = ConstSingleton<EgressConfigImpl>;

source/extensions/filters/network/http_connection_manager/config.cc

@@ -504,10 +504,25 @@ HttpConnectionManagerConfig::HttpConnectionManagerConfig(
     const uint32_t max_path_tag_length = PROTOBUF_GET_WRAPPED_OR_DEFAULT(
         tracing_config, max_path_tag_length, Tracing::DefaultMaxPathTagLength);
 
+    Tracing::TraceRequestIdSampleDecisionPolicy decision_policy;
+
+    switch (tracing_config.trace_request_id_sample_decision_policy()) {
+    case envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager::
+        Tracing::DEFAULT:
+      decision_policy = Tracing::TraceRequestIdSampleDecisionPolicy::Default;
+      break;
+    case envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager::
+        Tracing::BYPASS:
+      decision_policy = Tracing::TraceRequestIdSampleDecisionPolicy::ByPass;
+      break;
+    default:
+      NOT_REACHED_GCOVR_EXCL_LINE;
+    }
+
     tracing_config_ =
         std::make_unique<Http::TracingConnectionManagerConfig>(Http::TracingConnectionManagerConfig{
             tracing_operation_name, custom_tags, client_sampling, random_sampling, overall_sampling,
-            tracing_config.verbose(), max_path_tag_length});
+            tracing_config.verbose(), max_path_tag_length, decision_policy});
   }
 
   for (const auto& access_log : config.access_log()) {

test/common/http/conn_manager_impl_test.cc

@@ -1372,9 +1372,9 @@ TEST_F(HttpConnectionManagerImplTest, StartAndFinishSpanNormalFlow) {
       s.custom_tags.pop_front();
     }
   }
-  tracing_config_ = std::make_unique<TracingConnectionManagerConfig>(
-      TracingConnectionManagerConfig{Tracing::OperationName::Ingress, conn_tracing_tags, percent1,
-                                     percent2, percent1, false, 256});
+  tracing_config_ = std::make_unique<TracingConnectionManagerConfig>(TracingConnectionManagerConfig{
+      Tracing::OperationName::Ingress, conn_tracing_tags, percent1, percent2, percent1, false, 256,
+      Tracing::TraceRequestIdSampleDecisionPolicy::Default});
   NiceMock<Router::MockRouteTracing> route_tracing;
   ON_CALL(route_tracing, getClientSampling()).WillByDefault(ReturnRef(percent1));
   ON_CALL(route_tracing, getRandomSampling()).WillByDefault(ReturnRef(percent2));
@@ -1656,7 +1656,8 @@ TEST_F(HttpConnectionManagerImplTest, StartAndFinishSpanNormalFlowEgressDecorato
                                      percent2,
                                      percent1,
                                      false,
-                                     256});
+                                     256,
+                                     Tracing::TraceRequestIdSampleDecisionPolicy::Default});
 
   auto* span = new NiceMock<Tracing::MockSpan>();
   EXPECT_CALL(*tracer_, startSpan_(_, _, _, _))
@@ -1738,7 +1739,8 @@ TEST_F(HttpConnectionManagerImplTest, StartAndFinishSpanNormalFlowEgressDecorato
                                      percent2,
                                      percent1,
                                      false,
-                                     256});
+                                     256,
+                                     Tracing::TraceRequestIdSampleDecisionPolicy::Default});
 
   auto* span = new NiceMock<Tracing::MockSpan>();
   EXPECT_CALL(*tracer_, startSpan_(_, _, _, _))
@@ -1822,7 +1824,8 @@ TEST_F(HttpConnectionManagerImplTest, StartAndFinishSpanNormalFlowEgressDecorato
                                      percent2,
                                      percent1,
                                      false,
-                                     256});
+                                     256,
+                                     Tracing::TraceRequestIdSampleDecisionPolicy::Default});
 
   auto* span = new NiceMock<Tracing::MockSpan>();
   EXPECT_CALL(*tracer_, startSpan_(_, _, _, _))
@@ -1898,7 +1901,8 @@ TEST_F(HttpConnectionManagerImplTest,
                                      percent2,
                                      percent1,
                                      false,
-                                     256});
+                                     256,
+                                     Tracing::TraceRequestIdSampleDecisionPolicy::Default});
 
   EXPECT_CALL(
       runtime_.snapshot_,

test/common/http/conn_manager_impl_test_base.cc

@@ -88,7 +88,8 @@ void HttpConnectionManagerImplTest::setup(bool ssl, const std::string& server_na
                                        percent2,
                                        percent1,
                                        false,
-                                       256});
+                                       256,
+                                       Tracing::TraceRequestIdSampleDecisionPolicy::Default});
   }
 }
 

test/common/http/conn_manager_utility_test.cc

@@ -98,8 +98,14 @@ class ConnectionManagerUtilityTest : public testing::Test {
     envoy::type::v3::FractionalPercent percent2;
     percent2.set_numerator(10000);
     percent2.set_denominator(envoy::type::v3::FractionalPercent::TEN_THOUSAND);
-    tracing_config_ = {
-        Tracing::OperationName::Ingress, {}, percent1, percent2, percent1, false, 256};
+    tracing_config_ = {Tracing::OperationName::Ingress,
+                       {},
+                       percent1,
+                       percent2,
+                       percent1,
+                       false,
+                       256,
+                       Tracing::TraceRequestIdSampleDecisionPolicy::Default};
     ON_CALL(config_, tracingConfig()).WillByDefault(Return(&tracing_config_));
     ON_CALL(config_, localReply()).WillByDefault(ReturnRef(*local_reply_));
 
@@ -1229,6 +1235,16 @@ TEST_F(ConnectionManagerUtilityTest, SamplingWithoutRouteOverride) {
   EXPECT_EQ(Tracing::Reason::Sampling, request_id_extension_->getTraceReason(request_headers));
 }
 
+TEST_F(ConnectionManagerUtilityTest, CheckSamplingDecisionWithBypassSamplingWithRequestId) {
+  tracing_config_.trace_request_id_sample_decision_policy_ =
+      Tracing::TraceRequestIdSampleDecisionPolicy::ByPass;
+
+  Http::TestRequestHeaderMapImpl request_headers{
+      {"x-request-id", "125a4afb-6f55-44ba-ad80-413f09f48a28"}};
+  const auto ret = callMutateRequestHeaders(request_headers, Protocol::Http2);
+  EXPECT_EQ(Tracing::Reason::Sampling, ret.trace_reason_);
+}
+
 TEST_F(ConnectionManagerUtilityTest, SamplingWithRouteOverride) {
   EXPECT_CALL(
       runtime_.snapshot_,

test/mocks/tracing/mocks.cc

@@ -16,6 +16,7 @@ MockConfig::MockConfig() {
   ON_CALL(*this, customTags()).WillByDefault(Return(&custom_tags_));
   ON_CALL(*this, verbose()).WillByDefault(Return(verbose_));
   ON_CALL(*this, maxPathTagLength()).WillByDefault(Return(uint32_t(256)));
+  ON_CALL(*this, traceRequestIdSampleDecisionPolicy()).WillByDefault(Return(policy_));
 }
 MockConfig::~MockConfig() = default;
 

test/mocks/tracing/mocks.h

@@ -20,8 +20,10 @@ class MockConfig : public Config {
   MOCK_METHOD(const CustomTagMap*, customTags, (), (const));
   MOCK_METHOD(bool, verbose, (), (const));
   MOCK_METHOD(uint32_t, maxPathTagLength, (), (const));
+  MOCK_METHOD(TraceRequestIdSampleDecisionPolicy, traceRequestIdSampleDecisionPolicy, (), (const));
 
   OperationName operation_name_{OperationName::Ingress};
+  TraceRequestIdSampleDecisionPolicy policy_{TraceRequestIdSampleDecisionPolicy::Default};
   CustomTagMap custom_tags_;
   bool verbose_{false};
 };