tracing: apply tracer provider configuration defined as part of `http…

…_connection_manager` filter

Signed-off-by: Yaroslav Skopets <yaroslav@tetrate.io>

api/envoy/config/filter/network/http_connection_manager/v2/http_connection_manager.proto

@@ -161,7 +161,18 @@ message HttpConnectionManager {
     // Configuration for an external tracing provider.
     // If not specified, Envoy will fall back to using tracing provider configuration
     // from the bootstrap config.
-    // [#not-implemented-hide:]
+    //
+    // .. attention::
+    //   Please be aware that *envoy.tracers.opencensus* provider behaves differently than the
+    //   others.
+    //   By design, OpenCensus only supports a single tracing configuration per entire process;
+    //   most parts of that configuration cannot be updated or reset.
+    //   If you try to give a unique *envoy.tracers.opencensus* configuration to multiple filters,
+    //   the end result will be different from what you naturally expect.
+    //   Having said that, if your intent is to have exactly the same OpenCensus configuration
+    //   throughout the entire lifecycle of Envoy process, you can still benefit from defining that
+    //   configuration in here and get it delivered via xDS rather than put it into bootstrap
+    //   config.
     trace.v2.Tracing.Http provider = 9;
   }
 

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

@@ -149,7 +149,18 @@ message HttpConnectionManager {
     // Configuration for an external tracing provider.
     // If not specified, Envoy will fall back to using tracing provider configuration
     // from the bootstrap config.
-    // [#not-implemented-hide:]
+    //
+    // .. attention::
+    //   Please be aware that *envoy.tracers.opencensus* provider behaves differently than the
+    //   others.
+    //   By design, OpenCensus only supports a single tracing configuration per entire process;
+    //   most parts of that configuration cannot be updated or reset.
+    //   If you try to give a unique *envoy.tracers.opencensus* configuration to multiple filters,
+    //   the end result will be different from what you naturally expect.
+    //   Having said that, if your intent is to have exactly the same OpenCensus configuration
+    //   throughout the entire lifecycle of Envoy process, you can still benefit from defining that
+    //   configuration in here and get it delivered via xDS rather than put it into bootstrap
+    //   config.
     config.trace.v3.Tracing.Http provider = 9;
   }
 

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

@@ -535,11 +535,15 @@ const Network::Address::Instance& HttpConnectionManagerConfig::localAddress() {
  * "envoy.filters.network.http_connection_manager" filter instance.
  */
 const envoy::config::trace::v3::Tracing_Http* HttpConnectionManagerConfig::getPerFilterTracerConfig(
-    const envoy::extensions::filters::network::http_connection_manager::v3::
-        HttpConnectionManager&) {
-  // At the moment, it is not yet possible to define tracing provider as part of
-  // "envoy.filters.network.http_connection_manager" config.
-  // Therefore, we always fallback to using the default server-wide tracing provider.
+    const envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager&
+        config) {
+  // Give precedence to tracing provider configuration defined as part of
+  // "envoy.filters.network.http_connection_manager" filter config.
+  if (config.tracing().has_provider()) {
+    return &config.tracing().provider();
+  }
+  // Otherwise, for the sake of backwards compatibility, fallback to using tracing provider
+  // configuration defined in the bootstrap config.
   if (context_.httpContext().defaultTracingConfig().has_http()) {
     return &context_.httpContext().defaultTracingConfig().http();
   }

test/extensions/filters/network/http_connection_manager/config_test.cc

@@ -178,41 +178,6 @@ stat_prefix: router
   EXPECT_EQ(5 * 60 * 1000, config.streamIdleTimeout().count());
 }
 
-TEST_F(HttpConnectionManagerConfigTest, TracingConfigurationWithInlinedTracerProvider) {
-  const std::string yaml_string = R"EOF(
-codec_type: http1
-server_name: foo
-stat_prefix: router
-route_config:
-  virtual_hosts:
-  - name: service
-    domains:
-    - "*"
-    routes:
-    - match:
-        prefix: "/"
-      route:
-        cluster: cluster
-tracing:
-  operation_name: ingress
-  max_path_tag_length: 128
-  provider:                # notice inlined tracing provider configuration
-    name: zipkin
-    typed_config:
-      "@type": type.googleapis.com/envoy.config.trace.v2.ZipkinConfig
-      collector_cluster: zipkin
-      collector_endpoint: "/api/v1/spans"
-      collector_endpoint_version: HTTP_JSON
-http_filters:
-- name: envoy.filters.http.router
-  )EOF";
-
-  auto config = parseHttpConnectionManagerFromV2Yaml(yaml_string);
-
-  // Verify that inlined tracing provider configuration is actually parsed.
-  EXPECT_EQ("zipkin", config.tracing().provider().name());
-}
-
 TEST_F(HttpConnectionManagerConfigTest, TracingNotEnabledAndNoTracingConfigInBootstrap) {
   const std::string yaml_string = R"EOF(
 codec_type: http1
@@ -357,6 +322,65 @@ tracing: {} # notice that tracing is enabled
   EXPECT_THAT(config.tracer(), Eq(http_tracer_));
 }
 
+TEST_F(HttpConnectionManagerConfigTest, TracingIsEnabledAndThereIsInlinedTracerProvider) {
+  const std::string yaml_string = R"EOF(
+codec_type: http1
+server_name: foo
+stat_prefix: router
+route_config:
+  virtual_hosts:
+  - name: service
+    domains:
+    - "*"
+    routes:
+    - match:
+        prefix: "/"
+      route:
+        cluster: cluster
+tracing:
+  operation_name: ingress
+  max_path_tag_length: 128
+  provider:                # notice inlined tracing provider configuration
+    name: zipkin
+    typed_config:
+      "@type": type.googleapis.com/envoy.config.trace.v3.ZipkinConfig
+      collector_cluster: zipkin
+      collector_endpoint: "/api/v1/spans"
+      collector_endpoint_version: HTTP_JSON
+http_filters:
+- name: envoy.filters.http.router
+  )EOF";
+
+  // Simulate tracer provider configuration in the bootstrap config.
+  envoy::config::trace::v3::Tracing bootstrap_tracing_config;
+  bootstrap_tracing_config.mutable_http()->set_name("opencensus");
+  bootstrap_tracing_config.mutable_http()->mutable_typed_config()->PackFrom(
+      envoy::config::trace::v3::OpenCensusConfig{});
+  context_.http_context_.setDefaultTracingConfig(bootstrap_tracing_config);
+
+  // Set up expected tracer provider configuration.
+  envoy::config::trace::v3::Tracing_Http inlined_tracing_config;
+  inlined_tracing_config.set_name("zipkin");
+  envoy::config::trace::v3::ZipkinConfig zipkin_config;
+  zipkin_config.set_collector_cluster("zipkin");
+  zipkin_config.set_collector_endpoint("/api/v1/spans");
+  zipkin_config.set_collector_endpoint_version(envoy::config::trace::v3::ZipkinConfig::HTTP_JSON);
+  inlined_tracing_config.mutable_typed_config()->PackFrom(zipkin_config);
+
+  // When tracing is enabled on a given "envoy.filters.network.http_connection_manager" filter,
+  // an actual HttpTracer must be obtained from the HttpTracerManager.
+  // Expect inlined tracer provider configuration to take precedence over bootstrap configuration.
+  EXPECT_CALL(http_tracer_manager_, getOrCreateHttpTracer(Pointee(ProtoEq(inlined_tracing_config))))
+      .WillOnce(Return(http_tracer_));
+
+  HttpConnectionManagerConfig config(parseHttpConnectionManagerFromV2Yaml(yaml_string), context_,
+                                     date_provider_, route_config_provider_manager_,
+                                     scoped_routes_config_provider_manager_, http_tracer_manager_);
+
+  // Actual HttpTracer must be obtained from the HttpTracerManager.
+  EXPECT_THAT(config.tracer(), Eq(http_tracer_));
+}
+
 TEST_F(HttpConnectionManagerConfigTest, TracingCustomTagsConfig) {
   const std::string yaml_string = R"EOF(
 stat_prefix: router