diff --git a/source/extensions/tracers/opentelemetry/opentelemetry_tracer_impl.cc b/source/extensions/tracers/opentelemetry/opentelemetry_tracer_impl.cc index d3fe8624c65de..c7ad3d203d893 100644 --- a/source/extensions/tracers/opentelemetry/opentelemetry_tracer_impl.cc +++ b/source/extensions/tracers/opentelemetry/opentelemetry_tracer_impl.cc @@ -31,7 +31,7 @@ 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(); @@ -39,7 +39,12 @@ tryCreateSamper(const envoy::config::trace::v3::OpenTelemetryConfig& opentelemet 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, diff --git a/source/extensions/tracers/opentelemetry/samplers/dynatrace/sampler_config_provider.cc b/source/extensions/tracers/opentelemetry/samplers/dynatrace/sampler_config_provider.cc index 0734ccf315367..7b8f6b64f8f53 100644 --- a/source/extensions/tracers/opentelemetry/samplers/dynatrace/sampler_config_provider.cc +++ b/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()); diff --git a/source/extensions/tracers/opentelemetry/samplers/dynatrace/sampler_config_provider.h b/source/extensions/tracers/opentelemetry/samplers/dynatrace/sampler_config_provider.h index 5257912c72865..ea3c0a9667ae2 100644 --- a/source/extensions/tracers/opentelemetry/samplers/dynatrace/sampler_config_provider.h +++ b/source/extensions/tracers/opentelemetry/samplers/dynatrace/sampler_config_provider.h @@ -70,6 +70,10 @@ class SamplerConfigProviderImpl : public SamplerConfigProvider, std::vector> 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; diff --git a/test/extensions/tracers/opentelemetry/samplers/dynatrace/config_test.cc b/test/extensions/tracers/opentelemetry/samplers/dynatrace/config_test.cc index 6860df5c0ece6..1ade5aded670c 100644 --- a/test/extensions/tracers/opentelemetry/samplers/dynatrace/config_test.cc +++ b/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); diff --git a/test/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler_integration_test.cc b/test/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler_integration_test.cc index a3887eaf5ab4e..c128d93b7c5a8 100644 --- a/test/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler_integration_test.cc +++ b/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 { 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 diff --git a/test/extensions/tracers/opentelemetry/samplers/dynatrace/sampler_config_provider_test.cc b/test/extensions/tracers/opentelemetry/samplers/dynatrace/sampler_config_provider_test.cc index a2a7f573174f5..9a9ea4bed65fc 100644 --- a/test/extensions/tracers/opentelemetry/samplers/dynatrace/sampler_config_provider_test.cc +++ b/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 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 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