Handle deprecated api

source/extensions/tracers/opentelemetry/opentelemetry_tracer_impl.cc

@@ -31,15 +31,20 @@ namespace {
 
 SamplerSharedPtr
 tryCreateSamper(const envoy::config::trace::v3::OpenTelemetryConfig& opentelemetry_config,
-                Server::Configuration::TracerFactoryContext& context) {
+                Server::Configuration::TracerFactoryContext& context, spdlog::logger& logger) {
   SamplerSharedPtr sampler;
   if (opentelemetry_config.has_sampler()) {
     auto& sampler_config = opentelemetry_config.sampler();
     auto* factory = Envoy::Config::Utility::getFactory<SamplerFactory>(sampler_config);
     if (!factory) {
       throw EnvoyException(fmt::format("Sampler factory not found: '{}'", sampler_config.name()));
     }
-    sampler = factory->createSampler(sampler_config.typed_config(), context);
+    TRY_ASSERT_MAIN_THREAD {
+      sampler = factory->createSampler(sampler_config.typed_config(), context);
+    }
+    END_TRY CATCH(const EnvoyException& ee, {
+      ENVOY_LOG_TO_LOGGER(logger, error, "Failed to create sampler: {}", ee.what());
+    });
   }
   return sampler;
 }
@@ -82,7 +87,7 @@ Driver::Driver(const envoy::config::trace::v3::OpenTelemetryConfig& opentelemetr
   }
 
   // Create the sampler if configured
-  SamplerSharedPtr sampler = tryCreateSamper(opentelemetry_config, context);
+  SamplerSharedPtr sampler = tryCreateSamper(opentelemetry_config, context, ENVOY_LOGGER());
 
   // Create the tracer in Thread Local Storage.
   tls_slot_ptr_->set([opentelemetry_config, &factory_context, this, resource_ptr,

source/extensions/tracers/opentelemetry/samplers/dynatrace/sampler_config_provider.cc

@@ -31,16 +31,43 @@ bool reEnableTimer(Http::Code response_code) {
 
 } // namespace
 
+void SamplerConfigProviderImpl::handleConfig(
+    const envoy::extensions::tracers::opentelemetry::samplers::v3::DynatraceSamplerConfig& config) {
+  // check possible errors
+  if (config.has_http_service()) {
+    if (config.has_http_uri()) {
+      throw EnvoyException("Dynatrace Sampler: `http_service` and `http_uri` are set.");
+    }
+    if (!config.token().empty()) {
+      throw EnvoyException("Dynatrace Sampler: `http_service` and `token` are set.");
+    }
+  } else { // no http_service
+    if (!config.has_http_uri() || config.token().empty()) {
+      throw EnvoyException("Dynatrace Sampler: `http_service` not set.");
+    }
+  }
+
+  // handle config
+  if (config.has_http_service()) {
+    for (const auto& header_value_option : config.http_service().request_headers_to_add()) {
+      parsed_headers_to_add_.push_back({Http::LowerCaseString(header_value_option.header().key()),
+                                        header_value_option.header().value()});
+    }
+    http_uri_ = config.http_service().http_uri();
+  } else {
+    parsed_headers_to_add_.push_back(
+        {Http::CustomHeaders::get().Authorization, absl::StrCat("Api-Token ", config.token())});
+    http_uri_ = config.http_uri();
+  }
+}
+
 SamplerConfigProviderImpl::SamplerConfigProviderImpl(
     Server::Configuration::TracerFactoryContext& context,
     const envoy::extensions::tracers::opentelemetry::samplers::v3::DynatraceSamplerConfig& config)
     : cluster_manager_(context.serverFactoryContext().clusterManager()),
-      http_uri_(config.http_service().http_uri()), sampler_config_(config.root_spans_per_minute()) {
+      sampler_config_(config.root_spans_per_minute()) {
 
-  for (const auto& header_value_option : config.http_service().request_headers_to_add()) {
-    parsed_headers_to_add_.push_back({Http::LowerCaseString(header_value_option.header().key()),
-                                      header_value_option.header().value()});
-  }
+  handleConfig(config);
 
   timer_ = context.serverFactoryContext().mainThreadDispatcher().createTimer([this]() -> void {
     const auto thread_local_cluster = cluster_manager_.getThreadLocalCluster(http_uri_.cluster());

source/extensions/tracers/opentelemetry/samplers/dynatrace/sampler_config_provider.h

@@ -70,6 +70,10 @@ class SamplerConfigProviderImpl : public SamplerConfigProvider,
   std::vector<std::pair<const Http::LowerCaseString, const std::string>> parsed_headers_to_add_;
   Http::AsyncClient::Request* active_request_{};
   SamplerConfig sampler_config_;
+
+  void handleConfig(
+      const envoy::extensions::tracers::opentelemetry::samplers::v3::DynatraceSamplerConfig&
+          config);
 };
 
 using SamplerConfigProviderPtr = std::unique_ptr<SamplerConfigProvider>;

test/extensions/tracers/opentelemetry/samplers/dynatrace/config_test.cc

@@ -25,6 +25,15 @@ TEST(DynatraceSamplerFactoryTest, Test) {
     name: envoy.tracers.opentelemetry.samplers.dynatrace
     typed_config:
         "@type": type.googleapis.com/envoy.extensions.tracers.opentelemetry.samplers.v3.DynatraceSamplerConfig
+        http_service:
+          http_uri:
+            cluster: "cluster_name"
+            uri: "https://testhost.com/api/v2/samplingConfiguration"
+            timeout: 10s
+          request_headers_to_add:
+          - header:
+              key: "authorization"
+              value: "Api-Token tokenval"
   )EOF";
   TestUtility::loadFromYaml(sampler_yaml, typed_config);
 

test/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler_integration_test.cc

@@ -22,8 +22,23 @@ const char* TRACEPARENT_VALUE_START = "00-0af7651916cd43dd8448eb211c80319c";
 class DynatraceSamplerIntegrationTest : public Envoy::HttpIntegrationTest,
                                         public testing::TestWithParam<Network::Address::IpVersion> {
 public:
-  DynatraceSamplerIntegrationTest() : HttpIntegrationTest(Http::CodecType::HTTP1, GetParam()) {
-    const std::string yaml_string = R"EOF(
+  DynatraceSamplerIntegrationTest() : HttpIntegrationTest(Http::CodecType::HTTP1, GetParam()) {}
+
+  void initializeRoute(const std::string& config_yaml) {
+    auto tracing_config =
+        std::make_unique<::envoy::extensions::filters::network::http_connection_manager::v3::
+                             HttpConnectionManager_Tracing>();
+    TestUtility::loadFromYaml(config_yaml, *tracing_config.get());
+
+    config_helper_.addConfigModifier(
+        [&](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager&
+                hcm) -> void { hcm.set_allocated_tracing(tracing_config.release()); });
+
+    initialize();
+  }
+};
+
+static const std::string yaml_string = R"EOF(
   provider:
     name: envoy.tracers.opentelemetry
     typed_config:
@@ -39,27 +54,26 @@ class DynatraceSamplerIntegrationTest : public Envoy::HttpIntegrationTest,
           "@type": type.googleapis.com/envoy.extensions.tracers.opentelemetry.samplers.v3.DynatraceSamplerConfig
           tenant: "abc12345"
           cluster_id: -1743916452
+          http_service:
+            http_uri:
+              cluster: "cluster_name"
+              uri: "https://testhost.com/api/v2/samplingConfiguration"
+              timeout: 10s
+            request_headers_to_add:
+            - header:
+                key: "authorization"
+                value: "Api-Token tokenval"
   )EOF";
 
-    auto tracing_config =
-        std::make_unique<::envoy::extensions::filters::network::http_connection_manager::v3::
-                             HttpConnectionManager_Tracing>();
-    TestUtility::loadFromYaml(yaml_string, *tracing_config.get());
-    config_helper_.addConfigModifier(
-        [&](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager&
-                hcm) -> void { hcm.set_allocated_tracing(tracing_config.release()); });
-
-    initialize();
-    codec_client_ = makeHttpConnection(lookupPort("http"));
-  }
-};
-
 INSTANTIATE_TEST_SUITE_P(IpVersions, DynatraceSamplerIntegrationTest,
                          testing::ValuesIn(TestEnvironment::getIpVersionsForTest()),
                          TestUtility::ipTestParamsToString);
 
 // Sends a request with traceparent and tracestate header.
 TEST_P(DynatraceSamplerIntegrationTest, TestWithTraceparentAndTracestate) {
+  initializeRoute(yaml_string);
+  codec_client_ = makeHttpConnection(lookupPort("http"));
+
   // tracestate does not contain a Dynatrace tag
   Http::TestRequestHeaderMapImpl request_headers{
       {":method", "GET"},     {":path", "/test/long/url"}, {":scheme", "http"},
@@ -92,6 +106,9 @@ TEST_P(DynatraceSamplerIntegrationTest, TestWithTraceparentAndTracestate) {
 
 // Sends a request with traceparent but no tracestate header.
 TEST_P(DynatraceSamplerIntegrationTest, TestWithTraceparentOnly) {
+  initializeRoute(yaml_string);
+  codec_client_ = makeHttpConnection(lookupPort("http"));
+
   Http::TestRequestHeaderMapImpl request_headers{{":method", "GET"},
                                                  {":path", "/test/long/url"},
                                                  {":scheme", "http"},
@@ -122,6 +139,9 @@ TEST_P(DynatraceSamplerIntegrationTest, TestWithTraceparentOnly) {
 
 // Sends a request without traceparent and tracestate header.
 TEST_P(DynatraceSamplerIntegrationTest, TestWithoutTraceparentAndTracestate) {
+  initializeRoute(yaml_string);
+  codec_client_ = makeHttpConnection(lookupPort("http"));
+
   Http::TestRequestHeaderMapImpl request_headers{
       {":method", "GET"}, {":path", "/test/long/url"}, {":scheme", "http"}, {":authority", "host"}};
 
@@ -144,6 +164,64 @@ TEST_P(DynatraceSamplerIntegrationTest, TestWithoutTraceparentAndTracestate) {
       << "Received tracestate: " << tracestate_value;
 }
 
+// Simulates a wrong configuration, where the sampler is not used by the Tracer.
+TEST_P(DynatraceSamplerIntegrationTest, TestWithInvalidDeprecatedConfiguration) {
+  std::string yaml_config = R"EOF(
+  provider:
+    name: envoy.tracers.opentelemetry
+    typed_config:
+      "@type": type.googleapis.com/envoy.config.trace.v3.OpenTelemetryConfig
+      grpc_service:
+        envoy_grpc:
+          cluster_name: opentelemetry_collector
+        timeout: 0.250s
+      service_name: "a_service_name"
+      sampler:
+        name: envoy.tracers.opentelemetry.samplers.dynatrace
+        typed_config:
+          "@type": type.googleapis.com/envoy.extensions.tracers.opentelemetry.samplers.v3.DynatraceSamplerConfig
+          tenant: "abc12345"
+          cluster_id: -1743916452
+          http_uri:
+              cluster: "cluster_name"
+              uri: "https://unused.com/notused"
+              timeout: 10s
+          http_service:
+            http_uri:
+              cluster: "cluster_name"
+              uri: "https://testhost.com/api/v2/samplingConfiguration"
+              timeout: 10s
+            request_headers_to_add:
+            - header:
+                key: "authorization"
+                value: "Api-Token tokenval"
+  )EOF";
+
+  initializeRoute(yaml_config);
+  codec_client_ = makeHttpConnection(lookupPort("http"));
+
+  Http::TestRequestHeaderMapImpl request_headers{
+      {":method", "GET"}, {":path", "/test/long/url"}, {":scheme", "http"}, {":authority", "host"}};
+
+  auto response = sendRequestAndWaitForResponse(request_headers, 0, default_response_headers_, 0);
+
+  ASSERT_TRUE(response->waitForEndStream());
+  EXPECT_TRUE(response->complete());
+  EXPECT_EQ(response->headers().getStatusValue(), "200");
+
+  // traceparent will be added, trace_id and span_id will be generated, so there is nothing we can
+  // assert
+  EXPECT_EQ(upstream_request_->headers().get(::Envoy::Http::LowerCaseString("traceparent")).size(),
+            1);
+  // Dynatrace tag should NOT be added to tracestate
+  absl::string_view tracestate_value = upstream_request_->headers()
+                                           .get(Http::LowerCaseString("tracestate"))[0]
+                                           ->value()
+                                           .getStringView();
+  EXPECT_FALSE(absl::StartsWith(tracestate_value, "5b3f9fed-980df25c@dt=fw4;0;0;0;0;0;0;"))
+      << "Received tracestate: " << tracestate_value;
+}
+
 } // namespace
 } // namespace OpenTelemetry
 } // namespace Tracers

test/extensions/tracers/opentelemetry/samplers/dynatrace/sampler_config_provider_test.cc

@@ -20,7 +20,6 @@ namespace Tracers {
 namespace OpenTelemetry {
 using testing::NiceMock;
 using testing::Return;
-using testing::ReturnRef;
 
 class SamplerConfigProviderTest : public testing::Test {
 public:
@@ -61,23 +60,31 @@ class SamplerConfigProviderTest : public testing::Test {
   Http::MockAsyncClientRequest request_;
 };
 
-MATCHER_P(MessageMatcher, unusedArg, "") {
+struct ExpectedHeaders {
+  std::string auth_header;
+  std::string path;
+  std::string host;
+};
+
+MATCHER_P(MessageMatcher, expected_headers, "") {
   return (arg->headers()
               .get(Http::CustomHeaders::get().Authorization)[0]
               ->value()
-              .getStringView() == "Api-Token tokenval") &&
+              .getStringView() == expected_headers.auth_header) &&
          (arg->headers().get(Http::Headers::get().Path)[0]->value().getStringView() ==
-          "/api/v2/samplingConfiguration") &&
+          expected_headers.path) &&
          (arg->headers().get(Http::Headers::get().Host)[0]->value().getStringView() ==
-          "testhost.com") &&
+          expected_headers.host) &&
          (arg->headers().get(Http::Headers::get().Method)[0]->value().getStringView() == "GET");
 }
 
 // Test that a request is sent if timer fires
 TEST_F(SamplerConfigProviderTest, TestRequestIsSent) {
+  ExpectedHeaders expected_headers{"Api-Token tokenval", "/api/v2/samplingConfiguration",
+                                   "testhost.com"};
   EXPECT_CALL(tracer_factory_context_.server_factory_context_.cluster_manager_.thread_local_cluster_
                   .async_client_,
-              send_(MessageMatcher("unused-arg"), _, _));
+              send_(MessageMatcher(expected_headers), _, _));
   SamplerConfigProviderImpl config_provider(tracer_factory_context_, proto_config_);
   timer_->invokeCallback();
 }
@@ -265,6 +272,72 @@ TEST_F(SamplerConfigProviderTest, TestValueZeroConfigured) {
             SamplerConfig::ROOT_SPANS_PER_MINUTE_DEFAULT);
 }
 
+// Test handling if http_service and http_uri are set
+TEST(SamplerConfigProviderTestNoParent, TestConfigHandlingServiceAndUriSet) {
+  const std::string yaml_string = R"EOF(
+          http_service:
+            http_uri:
+              cluster: "cluster_name"
+              uri: "https://testhost.com/api/v2/samplingConfiguration"
+              timeout: 10s
+            request_headers_to_add:
+            - header:
+                key: "authorization"
+                value: "Api-Token tokenval"
+          http_uri:
+              cluster: "cluster_name"
+              uri: "https://unused.com/notused"
+    )EOF";
+
+  envoy::extensions::tracers::opentelemetry::samplers::v3::DynatraceSamplerConfig proto_config;
+  NiceMock<Envoy::Server::Configuration::MockTracerFactoryContext> tracer_factory_context;
+  TestUtility::loadFromYaml(yaml_string, proto_config);
+  EXPECT_THROW(SamplerConfigProviderImpl(tracer_factory_context, proto_config), EnvoyException);
+}
+
+// Test handling if http_service and token are set
+TEST(SamplerConfigProviderTestNoParent, TestConfigHandlingServiceAndTokenSet) {
+  const std::string yaml_string = R"EOF(
+          http_service:
+            http_uri:
+              cluster: "cluster_name"
+              uri: "https://testhost.com/api/v2/samplingConfiguration"
+              timeout: 10s
+            request_headers_to_add:
+            - header:
+                key: "authorization"
+                value: "Api-Token tokenval"
+          token: "shouldn't_be_used"
+    )EOF";
+
+  envoy::extensions::tracers::opentelemetry::samplers::v3::DynatraceSamplerConfig proto_config;
+  NiceMock<Envoy::Server::Configuration::MockTracerFactoryContext> tracer_factory_context;
+  TestUtility::loadFromYaml(yaml_string, proto_config);
+  EXPECT_THROW(SamplerConfigProviderImpl(tracer_factory_context, proto_config), EnvoyException);
+}
+
+// Test handling if http_service is not set but http_uri and token are set
+TEST_F(SamplerConfigProviderTest, TestConfigHandlingDeprecatedAreSet) {
+  const std::string yaml_string = R"EOF(
+          token: "tokenval"
+          http_uri:
+              cluster: "cluster_name"
+              uri: "https://testhost.com/api/v2/samplingConfiguration"
+              timeout: 10s
+    )EOF";
+
+  envoy::extensions::tracers::opentelemetry::samplers::v3::DynatraceSamplerConfig proto_config;
+  TestUtility::loadFromYaml(yaml_string, proto_config);
+
+  ExpectedHeaders expected_headers{"Api-Token tokenval", "/api/v2/samplingConfiguration",
+                                   "testhost.com"};
+  EXPECT_CALL(tracer_factory_context_.server_factory_context_.cluster_manager_.thread_local_cluster_
+                  .async_client_,
+              send_(MessageMatcher(expected_headers), _, _));
+  SamplerConfigProviderImpl config_provider(tracer_factory_context_, proto_config);
+  timer_->invokeCallback();
+}
+
 } // namespace OpenTelemetry
 } // namespace Tracers
 } // namespace Extensions