From 63a4be20cc844359e80224984612f35949c521b8 Mon Sep 17 00:00:00 2001 From: Thomas Ebner <96168670+samohte@users.noreply.github.com> Date: Tue, 17 Oct 2023 08:49:36 +0200 Subject: [PATCH 01/21] Allow specifying samplers for the OpenTelemetry tracer via a new configuration. Add AlwaysOnSampler as reference implementation. Signed-off-by: thomas.ebner Signed-off-by: Thomas Ebner <96168670+samohte@users.noreply.github.com> Signed-off-by: thomas.ebner --- api/BUILD | 1 + api/envoy/config/trace/v3/opentelemetry.proto | 9 + .../tracers/opentelemetry/samplers/v3/BUILD | 9 + .../samplers/v3/always_on_sampler.proto | 23 +++ api/versioning/BUILD | 1 + changelogs/current.yaml | 3 + .../config/trace/opentelemetry/samplers.rst | 10 + docs/root/api-v3/config/trace/trace.rst | 1 + source/extensions/extensions_build_config.bzl | 6 + source/extensions/extensions_metadata.yaml | 7 + source/extensions/tracers/opentelemetry/BUILD | 1 + .../opentelemetry_tracer_impl.cc | 57 ++++-- .../tracers/opentelemetry/samplers/BUILD | 25 +++ .../opentelemetry/samplers/always_on/BUILD | 33 +++ .../samplers/always_on/always_on_sampler.cc | 34 +++ .../samplers/always_on/always_on_sampler.h | 38 ++++ .../samplers/always_on/config.cc | 27 +++ .../opentelemetry/samplers/always_on/config.h | 42 ++++ .../tracers/opentelemetry/samplers/sampler.h | 111 ++++++++++ .../tracers/opentelemetry/tracer.cc | 45 +++- .../extensions/tracers/opentelemetry/tracer.h | 9 +- .../tracers/opentelemetry/samplers/BUILD | 23 +++ .../opentelemetry/samplers/always_on/BUILD | 37 ++++ .../always_on/always_on_sampler_test.cc | 56 +++++ .../samplers/always_on/config_test.cc | 38 ++++ .../opentelemetry/samplers/sampler_test.cc | 193 ++++++++++++++++++ tools/extensions/extensions_schema.yaml | 1 + 27 files changed, 815 insertions(+), 25 deletions(-) create mode 100644 api/envoy/extensions/tracers/opentelemetry/samplers/v3/BUILD create mode 100644 api/envoy/extensions/tracers/opentelemetry/samplers/v3/always_on_sampler.proto create mode 100644 docs/root/api-v3/config/trace/opentelemetry/samplers.rst create mode 100644 source/extensions/tracers/opentelemetry/samplers/BUILD create mode 100644 source/extensions/tracers/opentelemetry/samplers/always_on/BUILD create mode 100644 source/extensions/tracers/opentelemetry/samplers/always_on/always_on_sampler.cc create mode 100644 source/extensions/tracers/opentelemetry/samplers/always_on/always_on_sampler.h create mode 100644 source/extensions/tracers/opentelemetry/samplers/always_on/config.cc create mode 100644 source/extensions/tracers/opentelemetry/samplers/always_on/config.h create mode 100644 source/extensions/tracers/opentelemetry/samplers/sampler.h create mode 100644 test/extensions/tracers/opentelemetry/samplers/BUILD create mode 100644 test/extensions/tracers/opentelemetry/samplers/always_on/BUILD create mode 100644 test/extensions/tracers/opentelemetry/samplers/always_on/always_on_sampler_test.cc create mode 100644 test/extensions/tracers/opentelemetry/samplers/always_on/config_test.cc create mode 100644 test/extensions/tracers/opentelemetry/samplers/sampler_test.cc diff --git a/api/BUILD b/api/BUILD index 554d3fde4a5a..b6f5cf0db3f3 100644 --- a/api/BUILD +++ b/api/BUILD @@ -305,6 +305,7 @@ proto_library( "//envoy/extensions/stat_sinks/graphite_statsd/v3:pkg", "//envoy/extensions/stat_sinks/open_telemetry/v3:pkg", "//envoy/extensions/stat_sinks/wasm/v3:pkg", + "//envoy/extensions/tracers/opentelemetry/samplers/v3:pkg", "//envoy/extensions/transport_sockets/alts/v3:pkg", "//envoy/extensions/transport_sockets/http_11_proxy/v3:pkg", "//envoy/extensions/transport_sockets/internal_upstream/v3:pkg", diff --git a/api/envoy/config/trace/v3/opentelemetry.proto b/api/envoy/config/trace/v3/opentelemetry.proto index e9c7430dcfdd..7af9840d18d4 100644 --- a/api/envoy/config/trace/v3/opentelemetry.proto +++ b/api/envoy/config/trace/v3/opentelemetry.proto @@ -2,6 +2,7 @@ syntax = "proto3"; package envoy.config.trace.v3; +import "envoy/config/core/v3/extension.proto"; import "envoy/config/core/v3/grpc_service.proto"; import "udpa/annotations/status.proto"; @@ -25,4 +26,12 @@ message OpenTelemetryConfig { // The name for the service. This will be populated in the ResourceSpan Resource attributes. // If it is not provided, it will default to "unknown_service:envoy". string service_name = 2; + + // Specifies the sampler to be used by the OpenTelemetry tracer. + // The configured sampler implements the Sampler interface defined by the OpenTelemetry specification. + // This field can be left empty. In this case, the default Envoy sampling decision is used. + // + // See: `OpenTelemetry sampler specification `_ + // [#extension-category: envoy.tracers.opentelemetry.samplers] + core.v3.TypedExtensionConfig sampler = 3; } diff --git a/api/envoy/extensions/tracers/opentelemetry/samplers/v3/BUILD b/api/envoy/extensions/tracers/opentelemetry/samplers/v3/BUILD new file mode 100644 index 000000000000..ee92fb652582 --- /dev/null +++ b/api/envoy/extensions/tracers/opentelemetry/samplers/v3/BUILD @@ -0,0 +1,9 @@ +# DO NOT EDIT. This file is generated by tools/proto_format/proto_sync.py. + +load("@envoy_api//bazel:api_build_system.bzl", "api_proto_package") + +licenses(["notice"]) # Apache 2 + +api_proto_package( + deps = ["@com_github_cncf_udpa//udpa/annotations:pkg"], +) diff --git a/api/envoy/extensions/tracers/opentelemetry/samplers/v3/always_on_sampler.proto b/api/envoy/extensions/tracers/opentelemetry/samplers/v3/always_on_sampler.proto new file mode 100644 index 000000000000..241dc06eb1fc --- /dev/null +++ b/api/envoy/extensions/tracers/opentelemetry/samplers/v3/always_on_sampler.proto @@ -0,0 +1,23 @@ +syntax = "proto3"; + +package envoy.extensions.tracers.opentelemetry.samplers.v3; + +import "udpa/annotations/status.proto"; + +option java_package = "io.envoyproxy.envoy.extensions.tracers.opentelemetry.samplers.v3"; +option java_outer_classname = "AlwaysOnSamplerProto"; +option java_multiple_files = true; +option go_package = "github.com/envoyproxy/go-control-plane/envoy/extensions/tracers/opentelemetry/samplers/v3;samplersv3"; +option (udpa.annotations.file_status).package_version_status = ACTIVE; + +// [#protodoc-title: Always On Sampler config] +// Configuration for the "AlwaysOn" Sampler extension. +// The sampler follows the "AlwaysOn" implementation from the OpenTelemetry +// SDK specification. +// +// See: +// `AlwaysOn sampler specification `_ +// [#extension: envoy.tracers.opentelemetry.samplers.always_on] + +message AlwaysOnSamplerConfig { +} diff --git a/api/versioning/BUILD b/api/versioning/BUILD index 43de328ff168..be86aa5802d5 100644 --- a/api/versioning/BUILD +++ b/api/versioning/BUILD @@ -244,6 +244,7 @@ proto_library( "//envoy/extensions/stat_sinks/graphite_statsd/v3:pkg", "//envoy/extensions/stat_sinks/open_telemetry/v3:pkg", "//envoy/extensions/stat_sinks/wasm/v3:pkg", + "//envoy/extensions/tracers/opentelemetry/samplers/v3:pkg", "//envoy/extensions/transport_sockets/alts/v3:pkg", "//envoy/extensions/transport_sockets/http_11_proxy/v3:pkg", "//envoy/extensions/transport_sockets/internal_upstream/v3:pkg", diff --git a/changelogs/current.yaml b/changelogs/current.yaml index dcc37e21c9da..19d9dcac35c2 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -322,6 +322,9 @@ new_features: change: | Added :ref:`custom_sink ` type to enable writing tap data out to a custom sink extension. +- area: tracing + change: | + Added support to configure a sampler for the OpenTelemetry tracer. - area: tls change: | Added :ref:`disable_stateful_session_resumption diff --git a/docs/root/api-v3/config/trace/opentelemetry/samplers.rst b/docs/root/api-v3/config/trace/opentelemetry/samplers.rst new file mode 100644 index 000000000000..705155f640b9 --- /dev/null +++ b/docs/root/api-v3/config/trace/opentelemetry/samplers.rst @@ -0,0 +1,10 @@ +OpenTelemetry Samplers +====================== + +Samplers that can be configured with the OpenTelemetry Tracer: + +.. toctree:: + :glob: + :maxdepth: 3 + + ../../../extensions/tracers/opentelemetry/samplers/v3/* diff --git a/docs/root/api-v3/config/trace/trace.rst b/docs/root/api-v3/config/trace/trace.rst index 8f8d039a18d8..adc3daceb6e3 100644 --- a/docs/root/api-v3/config/trace/trace.rst +++ b/docs/root/api-v3/config/trace/trace.rst @@ -12,3 +12,4 @@ HTTP tracers :maxdepth: 2 v3/* + opentelemetry/samplers diff --git a/source/extensions/extensions_build_config.bzl b/source/extensions/extensions_build_config.bzl index 433fdd0f355e..bad8b26929dc 100644 --- a/source/extensions/extensions_build_config.bzl +++ b/source/extensions/extensions_build_config.bzl @@ -264,6 +264,12 @@ EXTENSIONS = { "envoy.tracers.skywalking": "//source/extensions/tracers/skywalking:config", "envoy.tracers.opentelemetry": "//source/extensions/tracers/opentelemetry:config", + # + # OpenTelemetry tracer samplers + # + + "envoy.tracers.opentelemetry.samplers.always_on": "//source/extensions/tracers/opentelemetry/samplers/always_on:config", + # # Transport sockets # diff --git a/source/extensions/extensions_metadata.yaml b/source/extensions/extensions_metadata.yaml index 7c4b60b96f4a..39f38fc4e8f5 100644 --- a/source/extensions/extensions_metadata.yaml +++ b/source/extensions/extensions_metadata.yaml @@ -1141,6 +1141,13 @@ envoy.tracers.opentelemetry: status: wip type_urls: - envoy.config.trace.v3.OpenTelemetryConfig +envoy.tracers.opentelemetry.samplers.always_on: + categories: + - envoy.tracers.opentelemetry.samplers + security_posture: unknown + status: wip + type_urls: + - envoy.extensions.tracers.opentelemetry.samplers.v3.AlwaysOnSamplerConfig envoy.tracers.skywalking: categories: - envoy.tracers diff --git a/source/extensions/tracers/opentelemetry/BUILD b/source/extensions/tracers/opentelemetry/BUILD index 3a36a3f91fd5..5b05a2da875e 100644 --- a/source/extensions/tracers/opentelemetry/BUILD +++ b/source/extensions/tracers/opentelemetry/BUILD @@ -41,6 +41,7 @@ envoy_cc_library( "//source/common/config:utility_lib", "//source/common/tracing:http_tracer_lib", "//source/extensions/tracers/common:factory_base_lib", + "//source/extensions/tracers/opentelemetry/samplers:sampler_lib", "@envoy_api//envoy/config/trace/v3:pkg_cc_proto", "@opentelemetry_proto//:trace_cc_proto", ], diff --git a/source/extensions/tracers/opentelemetry/opentelemetry_tracer_impl.cc b/source/extensions/tracers/opentelemetry/opentelemetry_tracer_impl.cc index 4ff15757b468..9e3de89975b2 100644 --- a/source/extensions/tracers/opentelemetry/opentelemetry_tracer_impl.cc +++ b/source/extensions/tracers/opentelemetry/opentelemetry_tracer_impl.cc @@ -8,6 +8,7 @@ #include "source/common/common/logger.h" #include "source/common/config/utility.h" #include "source/common/tracing/http_tracer_impl.h" +#include "source/extensions/tracers/opentelemetry/samplers/sampler.h" #include "opentelemetry/proto/collector/trace/v1/trace_service.pb.h" #include "opentelemetry/proto/trace/v1/trace.pb.h" @@ -20,29 +21,54 @@ namespace Extensions { namespace Tracers { namespace OpenTelemetry { +namespace { + +SamplerSharedPtr +tryCreateSamper(const envoy::config::trace::v3::OpenTelemetryConfig& opentelemetry_config, + Server::Configuration::TracerFactoryContext& context) { + SamplerSharedPtr sampler; + if (opentelemetry_config.has_sampler()) { + auto& sampler_config = opentelemetry_config.sampler(); + auto* factory = Envoy::Config::Utility::getFactory(sampler_config); + if (!factory) { + throw EnvoyException(fmt::format("Sampler factory not found: '{}'", sampler_config.name())); + } + sampler = factory->createSampler(sampler_config.typed_config(), context); + } + return sampler; +} + +} // namespace + Driver::Driver(const envoy::config::trace::v3::OpenTelemetryConfig& opentelemetry_config, Server::Configuration::TracerFactoryContext& context) : tls_slot_ptr_(context.serverFactoryContext().threadLocal().allocateSlot()), tracing_stats_{OPENTELEMETRY_TRACER_STATS( POOL_COUNTER_PREFIX(context.serverFactoryContext().scope(), "tracing.opentelemetry"))} { auto& factory_context = context.serverFactoryContext(); + + // Create the sampler if configured + SamplerSharedPtr sampler = tryCreateSamper(opentelemetry_config, context); + // Create the tracer in Thread Local Storage. - tls_slot_ptr_->set([opentelemetry_config, &factory_context, this](Event::Dispatcher& dispatcher) { - OpenTelemetryGrpcTraceExporterPtr exporter; - if (opentelemetry_config.has_grpc_service()) { - Grpc::AsyncClientFactoryPtr&& factory = - factory_context.clusterManager().grpcAsyncClientManager().factoryForGrpcService( - opentelemetry_config.grpc_service(), factory_context.scope(), true); - const Grpc::RawAsyncClientSharedPtr& async_client_shared_ptr = - factory->createUncachedRawAsyncClient(); - exporter = std::make_unique(async_client_shared_ptr); - } - TracerPtr tracer = std::make_unique( - std::move(exporter), factory_context.timeSource(), factory_context.api().randomGenerator(), - factory_context.runtime(), dispatcher, tracing_stats_, opentelemetry_config.service_name()); + tls_slot_ptr_->set( + [opentelemetry_config, &factory_context, this, sampler](Event::Dispatcher& dispatcher) { + OpenTelemetryGrpcTraceExporterPtr exporter; + if (opentelemetry_config.has_grpc_service()) { + Grpc::AsyncClientFactoryPtr&& factory = + factory_context.clusterManager().grpcAsyncClientManager().factoryForGrpcService( + opentelemetry_config.grpc_service(), factory_context.scope(), true); + const Grpc::RawAsyncClientSharedPtr& async_client_shared_ptr = + factory->createUncachedRawAsyncClient(); + exporter = std::make_unique(async_client_shared_ptr); + } + TracerPtr tracer = std::make_unique( + std::move(exporter), factory_context.timeSource(), + factory_context.api().randomGenerator(), factory_context.runtime(), dispatcher, + tracing_stats_, opentelemetry_config.service_name(), sampler); - return std::make_shared(std::move(tracer)); - }); + return std::make_shared(std::move(tracer)); + }); } Tracing::SpanPtr Driver::startSpan(const Tracing::Config& config, @@ -57,7 +83,6 @@ Tracing::SpanPtr Driver::startSpan(const Tracing::Config& config, // No propagation header, so we can create a fresh span with the given decision. Tracing::SpanPtr new_open_telemetry_span = tracer.startSpan(config, operation_name, stream_info.startTime(), tracing_decision); - new_open_telemetry_span->setSampled(tracing_decision.traced); return new_open_telemetry_span; } else { // Try to extract the span context. If we can't, just return a null span. diff --git a/source/extensions/tracers/opentelemetry/samplers/BUILD b/source/extensions/tracers/opentelemetry/samplers/BUILD new file mode 100644 index 000000000000..32a1005b11e8 --- /dev/null +++ b/source/extensions/tracers/opentelemetry/samplers/BUILD @@ -0,0 +1,25 @@ +load( + "//bazel:envoy_build_system.bzl", + "envoy_cc_library", + "envoy_extension_package", +) + +licenses(["notice"]) # Apache 2 + +envoy_extension_package() + +envoy_cc_library( + name = "sampler_lib", + srcs = [ + ], + hdrs = [ + "sampler.h", + ], + deps = [ + "//envoy/config:typed_config_interface", + "//envoy/server:tracer_config_interface", + "//source/common/common:logger_lib", + "//source/common/config:utility_lib", + "@opentelemetry_proto//:trace_cc_proto", + ], +) diff --git a/source/extensions/tracers/opentelemetry/samplers/always_on/BUILD b/source/extensions/tracers/opentelemetry/samplers/always_on/BUILD new file mode 100644 index 000000000000..744607330d57 --- /dev/null +++ b/source/extensions/tracers/opentelemetry/samplers/always_on/BUILD @@ -0,0 +1,33 @@ +load( + "//bazel:envoy_build_system.bzl", + "envoy_cc_extension", + "envoy_cc_library", + "envoy_extension_package", +) + +licenses(["notice"]) # Apache 2 + +envoy_extension_package() + +envoy_cc_extension( + name = "config", + srcs = ["config.cc"], + hdrs = ["config.h"], + deps = [ + ":always_on_sampler_lib", + "//envoy/registry", + "//source/common/config:utility_lib", + "@envoy_api//envoy/extensions/tracers/opentelemetry/samplers/v3:pkg_cc_proto", + ], +) + +envoy_cc_library( + name = "always_on_sampler_lib", + srcs = ["always_on_sampler.cc"], + hdrs = ["always_on_sampler.h"], + deps = [ + "//source/common/config:datasource_lib", + "//source/extensions/tracers/opentelemetry:opentelemetry_tracer_lib", + "//source/extensions/tracers/opentelemetry/samplers:sampler_lib", + ], +) diff --git a/source/extensions/tracers/opentelemetry/samplers/always_on/always_on_sampler.cc b/source/extensions/tracers/opentelemetry/samplers/always_on/always_on_sampler.cc new file mode 100644 index 000000000000..3bc0aa87ab3d --- /dev/null +++ b/source/extensions/tracers/opentelemetry/samplers/always_on/always_on_sampler.cc @@ -0,0 +1,34 @@ +#include "source/extensions/tracers/opentelemetry/samplers/always_on/always_on_sampler.h" + +#include +#include +#include + +#include "source/common/config/datasource.h" +#include "source/extensions/tracers/opentelemetry/span_context.h" + +namespace Envoy { +namespace Extensions { +namespace Tracers { +namespace OpenTelemetry { + +SamplingResult +AlwaysOnSampler::shouldSample(const absl::optional parent_context, + const std::string& /*trace_id*/, const std::string& /*name*/, + ::opentelemetry::proto::trace::v1::Span::SpanKind /*kind*/, + const std::map& /*attributes*/, + const std::vector& /*links*/) { + SamplingResult result; + result.decision = Decision::RECORD_AND_SAMPLE; + if (parent_context.has_value()) { + result.tracestate = parent_context.value().tracestate(); + } + return result; +} + +std::string AlwaysOnSampler::getDescription() const { return "AlwaysOnSampler"; } + +} // namespace OpenTelemetry +} // namespace Tracers +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/tracers/opentelemetry/samplers/always_on/always_on_sampler.h b/source/extensions/tracers/opentelemetry/samplers/always_on/always_on_sampler.h new file mode 100644 index 000000000000..2d53a511fa29 --- /dev/null +++ b/source/extensions/tracers/opentelemetry/samplers/always_on/always_on_sampler.h @@ -0,0 +1,38 @@ +#pragma once + +#include "envoy/server/factory_context.h" + +#include "source/common/common/logger.h" +#include "source/common/config/datasource.h" +#include "source/extensions/tracers/opentelemetry/samplers/sampler.h" + +namespace Envoy { +namespace Extensions { +namespace Tracers { +namespace OpenTelemetry { + +/** + * @brief A sampler which samples every span. + * https://opentelemetry.io/docs/specs/otel/trace/sdk/#alwayson + * - Returns RECORD_AND_SAMPLE always. + * - Description MUST be AlwaysOnSampler. + * + */ +class AlwaysOnSampler : public Sampler, Logger::Loggable { +public: + explicit AlwaysOnSampler(const Protobuf::Message& /*config*/, + Server::Configuration::TracerFactoryContext& /*context*/) {} + SamplingResult shouldSample(const absl::optional parent_context, + const std::string& trace_id, const std::string& name, + ::opentelemetry::proto::trace::v1::Span::SpanKind spankind, + const std::map& attributes, + const std::vector& links) override; + std::string getDescription() const override; + +private: +}; + +} // namespace OpenTelemetry +} // namespace Tracers +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/tracers/opentelemetry/samplers/always_on/config.cc b/source/extensions/tracers/opentelemetry/samplers/always_on/config.cc new file mode 100644 index 000000000000..99288c4bf469 --- /dev/null +++ b/source/extensions/tracers/opentelemetry/samplers/always_on/config.cc @@ -0,0 +1,27 @@ +#include "source/extensions/tracers/opentelemetry/samplers/always_on/config.h" + +#include "envoy/server/tracer_config.h" + +#include "source/common/config/utility.h" +#include "source/extensions/tracers/opentelemetry/samplers/always_on/always_on_sampler.h" + +namespace Envoy { +namespace Extensions { +namespace Tracers { +namespace OpenTelemetry { + +SamplerSharedPtr +AlwaysOnSamplerFactory::createSampler(const Protobuf::Message& config, + Server::Configuration::TracerFactoryContext& context) { + return std::make_shared(config, context); +} + +/** + * Static registration for the Env sampler factory. @see RegisterFactory. + */ +REGISTER_FACTORY(AlwaysOnSamplerFactory, SamplerFactory); + +} // namespace OpenTelemetry +} // namespace Tracers +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/tracers/opentelemetry/samplers/always_on/config.h b/source/extensions/tracers/opentelemetry/samplers/always_on/config.h new file mode 100644 index 000000000000..5f93f43c0f1f --- /dev/null +++ b/source/extensions/tracers/opentelemetry/samplers/always_on/config.h @@ -0,0 +1,42 @@ +#pragma once + +#include + +#include "envoy/extensions/tracers/opentelemetry/samplers/v3/always_on_sampler.pb.h" +#include "envoy/registry/registry.h" + +#include "source/extensions/tracers/opentelemetry/samplers/sampler.h" + +namespace Envoy { +namespace Extensions { +namespace Tracers { +namespace OpenTelemetry { + +/** + * Config registration for the AlwaysOnSampler. @see SamplerFactory. + */ +class AlwaysOnSamplerFactory : public SamplerFactory { +public: + /** + * @brief Create a Sampler. @see AlwaysOnSampler + * + * @param config Protobuf config for the sampler. + * @param context A reference to the TracerFactoryContext. + * @return SamplerSharedPtr + */ + SamplerSharedPtr createSampler(const Protobuf::Message& config, + Server::Configuration::TracerFactoryContext& context) override; + + ProtobufTypes::MessagePtr createEmptyConfigProto() override { + return std::make_unique< + envoy::extensions::tracers::opentelemetry::samplers::v3::AlwaysOnSamplerConfig>(); + } + std::string name() const override { return "envoy.tracers.opentelemetry.samplers.always_on"; } +}; + +DECLARE_FACTORY(AlwaysOnSamplerFactory); + +} // namespace OpenTelemetry +} // namespace Tracers +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/tracers/opentelemetry/samplers/sampler.h b/source/extensions/tracers/opentelemetry/samplers/sampler.h new file mode 100644 index 000000000000..fd2be0bb647e --- /dev/null +++ b/source/extensions/tracers/opentelemetry/samplers/sampler.h @@ -0,0 +1,111 @@ +#pragma once + +#include +#include +#include +#include + +#include "envoy/config/typed_config.h" +#include "envoy/server/tracer_config.h" +#include "envoy/tracing/trace_context.h" + +#include "absl/types/optional.h" +#include "opentelemetry/proto/trace/v1/trace.pb.h" + +namespace Envoy { +namespace Extensions { +namespace Tracers { +namespace OpenTelemetry { + +class SpanContext; + +enum class Decision { + // IsRecording will be false, the Span will not be recorded and all events and attributes will be + // dropped. + DROP, + // IsRecording will be true, but the Sampled flag MUST NOT be set. + RECORD_ONLY, + // IsRecording will be true and the Sampled flag MUST be set. + RECORD_AND_SAMPLE +}; + +struct SamplingResult { + /// @see Decision + Decision decision; + // A set of span Attributes that will also be added to the Span. Can be nullptr. + std::unique_ptr> attributes; + // A Tracestate that will be associated with the Span. If the sampler + // returns an empty Tracestate here, the Tracestate will be cleared, so samplers SHOULD normally + // return the passed-in Tracestate if they do not intend to change it + std::string tracestate; + + inline bool isRecording() const { + return decision == Decision::RECORD_ONLY || decision == Decision::RECORD_AND_SAMPLE; + } + + inline bool isSampled() const { return decision == Decision::RECORD_AND_SAMPLE; } +}; + +/** + * @brief The base type for all samplers + * see https://opentelemetry.io/docs/specs/otel/trace/sdk/#sampler + * + */ +class Sampler { +public: + virtual ~Sampler() = default; + + /** + * @brief Decides if a trace should be sampled. + * + * @param parent_context Span context describing the parent span. The Span's SpanContext may be + * invalid to indicate a root span. + * @param trace_id Trace id of the Span to be created. If the parent SpanContext contains a valid + * TraceId, they MUST always match. + * @param name Name of the Span to be created. + * @param spankind Span kind of the Span to be created. + * @param attributes Initial set of Attributes of the Span to be created. + * @param links Collection of links that will be associated with the Span to be created. + * @return SamplingResult @see SamplingResult + */ + virtual SamplingResult shouldSample(const absl::optional parent_context, + const std::string& trace_id, const std::string& name, + ::opentelemetry::proto::trace::v1::Span::SpanKind spankind, + const std::map& attributes, + const std::vector& links) PURE; + + /** + * @brief Returns a sampler description or name. + * + * @return The sampler name or short description with the configuration. + */ + virtual std::string getDescription() const PURE; +}; + +using SamplerSharedPtr = std::shared_ptr; + +/* + * A factory for creating a sampler + */ +class SamplerFactory : public Envoy::Config::TypedFactory { +public: + ~SamplerFactory() override = default; + + /** + * @brief Creates a sampler + * @param config The sampler protobuf config. + * @param context The TracerFactoryContext. + * @return SamplerSharedPtr A sampler. + */ + virtual SamplerSharedPtr createSampler(const Protobuf::Message& config, + Server::Configuration::TracerFactoryContext& context) PURE; + + std::string category() const override { return "envoy.tracers.opentelemetry.samplers"; } +}; + +using SamplerFactoryPtr = std::unique_ptr; + +} // namespace OpenTelemetry +} // namespace Tracers +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/tracers/opentelemetry/tracer.cc b/source/extensions/tracers/opentelemetry/tracer.cc index ca52280ed359..12caf9dfd446 100644 --- a/source/extensions/tracers/opentelemetry/tracer.cc +++ b/source/extensions/tracers/opentelemetry/tracer.cc @@ -24,6 +24,27 @@ constexpr absl::string_view kDefaultServiceName = "unknown_service:envoy"; using opentelemetry::proto::collector::trace::v1::ExportTraceServiceRequest; +namespace { + +void callSampler(SamplerSharedPtr sampler, const absl::optional span_context, + Span& new_span, const std::string& operation_name) { + if (!sampler) { + return; + } + const auto sampling_result = sampler->shouldSample( + span_context, operation_name, new_span.getTraceIdAsHex(), new_span.spankind(), {}, {}); + new_span.setSampled(sampling_result.isSampled()); + + if (sampling_result.attributes) { + for (auto const& attribute : *sampling_result.attributes) { + new_span.setTag(attribute.first, attribute.second); + } + } + new_span.setTracestate(sampling_result.tracestate); +} + +} // namespace + Span::Span(const Tracing::Config& config, const std::string& name, SystemTime start_time, Envoy::TimeSource& time_source, Tracer& parent_tracer, bool downstream_span) : parent_tracer_(parent_tracer), time_source_(time_source) { @@ -110,9 +131,9 @@ void Span::setTag(absl::string_view name, absl::string_view value) { Tracer::Tracer(OpenTelemetryGrpcTraceExporterPtr exporter, Envoy::TimeSource& time_source, Random::RandomGenerator& random, Runtime::Loader& runtime, Event::Dispatcher& dispatcher, OpenTelemetryTracerStats tracing_stats, - const std::string& service_name) + const std::string& service_name, SamplerSharedPtr sampler) : exporter_(std::move(exporter)), time_source_(time_source), random_(random), runtime_(runtime), - tracing_stats_(tracing_stats), service_name_(service_name) { + tracing_stats_(tracing_stats), service_name_(service_name), sampler_(sampler) { if (service_name.empty()) { service_name_ = std::string{kDefaultServiceName}; } @@ -172,12 +193,16 @@ Tracing::SpanPtr Tracer::startSpan(const Tracing::Config& config, const std::str bool downstream_span) { // Create an Tracers::OpenTelemetry::Span class that will contain the OTel span. Span new_span(config, operation_name, start_time, time_source_, *this, downstream_span); - new_span.setSampled(tracing_decision.traced); uint64_t trace_id_high = random_.random(); uint64_t trace_id = random_.random(); new_span.setTraceId(absl::StrCat(Hex::uint64ToHex(trace_id_high), Hex::uint64ToHex(trace_id))); uint64_t span_id = random_.random(); new_span.setId(Hex::uint64ToHex(span_id)); + if (sampler_) { + callSampler(sampler_, absl::nullopt, new_span, operation_name); + } else { + new_span.setSampled(tracing_decision.traced); + } return std::make_unique(new_span); } @@ -186,7 +211,6 @@ Tracing::SpanPtr Tracer::startSpan(const Tracing::Config& config, const std::str bool downstream_span) { // Create a new span and populate details from the span context. Span new_span(config, operation_name, start_time, time_source_, *this, downstream_span); - new_span.setSampled(previous_span_context.sampled()); new_span.setTraceId(previous_span_context.traceId()); if (!previous_span_context.parentId().empty()) { new_span.setParentId(previous_span_context.parentId()); @@ -194,10 +218,15 @@ Tracing::SpanPtr Tracer::startSpan(const Tracing::Config& config, const std::str // Generate a new identifier for the span id. uint64_t span_id = random_.random(); new_span.setId(Hex::uint64ToHex(span_id)); - // Respect the previous span's sampled flag. - new_span.setSampled(previous_span_context.sampled()); - if (!previous_span_context.tracestate().empty()) { - new_span.setTracestate(std::string{previous_span_context.tracestate()}); + if (sampler_) { + // Sampler should make a sampling decision and set tracestate + callSampler(sampler_, previous_span_context, new_span, operation_name); + } else { + // Respect the previous span's sampled flag. + new_span.setSampled(previous_span_context.sampled()); + if (!previous_span_context.tracestate().empty()) { + new_span.setTracestate(std::string{previous_span_context.tracestate()}); + } } return std::make_unique(new_span); } diff --git a/source/extensions/tracers/opentelemetry/tracer.h b/source/extensions/tracers/opentelemetry/tracer.h index 7f0d547e356b..20695dda65a4 100644 --- a/source/extensions/tracers/opentelemetry/tracer.h +++ b/source/extensions/tracers/opentelemetry/tracer.h @@ -11,6 +11,7 @@ #include "source/common/common/logger.h" #include "source/extensions/tracers/common/factory_base.h" #include "source/extensions/tracers/opentelemetry/grpc_trace_exporter.h" +#include "source/extensions/tracers/opentelemetry/samplers/sampler.h" #include "absl/strings/escaping.h" #include "span_context.h" @@ -35,7 +36,8 @@ class Tracer : Logger::Loggable { public: Tracer(OpenTelemetryGrpcTraceExporterPtr exporter, Envoy::TimeSource& time_source, Random::RandomGenerator& random, Runtime::Loader& runtime, Event::Dispatcher& dispatcher, - OpenTelemetryTracerStats tracing_stats, const std::string& service_name); + OpenTelemetryTracerStats tracing_stats, const std::string& service_name, + SamplerSharedPtr sampler); void sendSpan(::opentelemetry::proto::trace::v1::Span& span); @@ -65,6 +67,7 @@ class Tracer : Logger::Loggable { Event::TimerPtr flush_timer_; OpenTelemetryTracerStats tracing_stats_; std::string service_name_; + SamplerSharedPtr sampler_; }; /** @@ -111,6 +114,10 @@ class Span : Logger::Loggable, public Tracing::Span { std::string getTraceIdAsHex() const override { return absl::BytesToHexString(span_.trace_id()); }; + ::opentelemetry::proto::trace::v1::Span::SpanKind spankind() const { return span_.kind(); } + + std::string tracestate() const { return span_.trace_state(); } + /** * Sets the span's id. */ diff --git a/test/extensions/tracers/opentelemetry/samplers/BUILD b/test/extensions/tracers/opentelemetry/samplers/BUILD new file mode 100644 index 000000000000..55414e7854c9 --- /dev/null +++ b/test/extensions/tracers/opentelemetry/samplers/BUILD @@ -0,0 +1,23 @@ +load( + "//bazel:envoy_build_system.bzl", + "envoy_cc_test", + "envoy_package", +) + +licenses(["notice"]) # Apache 2 + +envoy_package() + +envoy_cc_test( + name = "sampler_test", + srcs = ["sampler_test.cc"], + deps = [ + "//envoy/registry", + "//source/extensions/tracers/opentelemetry:opentelemetry_tracer_lib", + "//source/extensions/tracers/opentelemetry/samplers:sampler_lib", + "//test/mocks/server:tracer_factory_context_mocks", + "//test/test_common:registry_lib", + "//test/test_common:utility_lib", + "@envoy_api//envoy/config/trace/v3:pkg_cc_proto", + ], +) diff --git a/test/extensions/tracers/opentelemetry/samplers/always_on/BUILD b/test/extensions/tracers/opentelemetry/samplers/always_on/BUILD new file mode 100644 index 000000000000..342679cf14a0 --- /dev/null +++ b/test/extensions/tracers/opentelemetry/samplers/always_on/BUILD @@ -0,0 +1,37 @@ +load( + "//bazel:envoy_build_system.bzl", + "envoy_package", +) +load( + "//test/extensions:extensions_build_system.bzl", + "envoy_extension_cc_test", +) + +licenses(["notice"]) # Apache 2 + +envoy_package() + +envoy_extension_cc_test( + name = "config_test", + srcs = ["config_test.cc"], + extension_names = ["envoy.tracers.opentelemetry.samplers.always_on"], + deps = [ + "//envoy/registry", + "//source/extensions/tracers/opentelemetry/samplers/always_on:always_on_sampler_lib", + "//source/extensions/tracers/opentelemetry/samplers/always_on:config", + "//test/mocks/server:tracer_factory_context_mocks", + "//test/test_common:utility_lib", + ], +) + +envoy_extension_cc_test( + name = "always_on_sampler_test", + srcs = ["always_on_sampler_test.cc"], + extension_names = ["envoy.tracers.opentelemetry.samplers.always_on"], + deps = [ + "//source/extensions/tracers/opentelemetry/samplers/always_on:always_on_sampler_lib", + "//test/mocks/server:tracer_factory_context_mocks", + "//test/test_common:utility_lib", + "@envoy_api//envoy/extensions/tracers/opentelemetry/samplers/v3:pkg_cc_proto", + ], +) diff --git a/test/extensions/tracers/opentelemetry/samplers/always_on/always_on_sampler_test.cc b/test/extensions/tracers/opentelemetry/samplers/always_on/always_on_sampler_test.cc new file mode 100644 index 000000000000..79d37ca9bfe8 --- /dev/null +++ b/test/extensions/tracers/opentelemetry/samplers/always_on/always_on_sampler_test.cc @@ -0,0 +1,56 @@ +#include + +#include "envoy/extensions/tracers/opentelemetry/samplers/v3/always_on_sampler.pb.h" + +#include "source/extensions/tracers/opentelemetry/samplers/always_on/always_on_sampler.h" +#include "source/extensions/tracers/opentelemetry/span_context.h" + +#include "test/mocks/server/tracer_factory_context.h" + +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +namespace Envoy { +namespace Extensions { +namespace Tracers { +namespace OpenTelemetry { + +// Verify sampler being invoked with an invalid span context +TEST(AlwaysOnSamplerTest, TestWithInvalidParentContext) { + envoy::extensions::tracers::opentelemetry::samplers::v3::AlwaysOnSamplerConfig config; + NiceMock context; + auto sampler = std::make_shared(config, context); + EXPECT_STREQ(sampler->getDescription().c_str(), "AlwaysOnSampler"); + + auto sampling_result = + sampler->shouldSample(absl::nullopt, "operation_name", "12345", + ::opentelemetry::proto::trace::v1::Span::SPAN_KIND_SERVER, {}, {}); + EXPECT_EQ(sampling_result.decision, Decision::RECORD_AND_SAMPLE); + EXPECT_EQ(sampling_result.attributes, nullptr); + EXPECT_STREQ(sampling_result.tracestate.c_str(), ""); + EXPECT_TRUE(sampling_result.isRecording()); + EXPECT_TRUE(sampling_result.isSampled()); +} + +// Verify sampler being invoked with a valid span context +TEST(AlwaysOnSamplerTest, TestWithValidParentContext) { + envoy::extensions::tracers::opentelemetry::samplers::v3::AlwaysOnSamplerConfig config; + NiceMock context; + auto sampler = std::make_shared(config, context); + EXPECT_STREQ(sampler->getDescription().c_str(), "AlwaysOnSampler"); + + SpanContext span_context("0", "12345", "45678", false, "some_tracestate"); + auto sampling_result = + sampler->shouldSample(span_context, "operation_name", "12345", + ::opentelemetry::proto::trace::v1::Span::SPAN_KIND_SERVER, {}, {}); + EXPECT_EQ(sampling_result.decision, Decision::RECORD_AND_SAMPLE); + EXPECT_EQ(sampling_result.attributes, nullptr); + EXPECT_STREQ(sampling_result.tracestate.c_str(), "some_tracestate"); + EXPECT_TRUE(sampling_result.isRecording()); + EXPECT_TRUE(sampling_result.isSampled()); +} + +} // namespace OpenTelemetry +} // namespace Tracers +} // namespace Extensions +} // namespace Envoy diff --git a/test/extensions/tracers/opentelemetry/samplers/always_on/config_test.cc b/test/extensions/tracers/opentelemetry/samplers/always_on/config_test.cc new file mode 100644 index 000000000000..226cd58e34f3 --- /dev/null +++ b/test/extensions/tracers/opentelemetry/samplers/always_on/config_test.cc @@ -0,0 +1,38 @@ +#include "envoy/registry/registry.h" + +#include "source/extensions/tracers/opentelemetry/samplers/always_on/config.h" + +#include "test/mocks/server/tracer_factory_context.h" +#include "test/test_common/utility.h" + +#include "gtest/gtest.h" + +namespace Envoy { +namespace Extensions { +namespace Tracers { +namespace OpenTelemetry { + +// Test create sampler via factory +TEST(AlwaysOnSamplerFactoryTest, Test) { + auto* factory = Registry::FactoryRegistry::getFactory( + "envoy.tracers.opentelemetry.samplers.always_on"); + ASSERT_NE(factory, nullptr); + EXPECT_STREQ(factory->name().c_str(), "envoy.tracers.opentelemetry.samplers.always_on"); + EXPECT_NE(factory->createEmptyConfigProto(), nullptr); + + envoy::config::core::v3::TypedExtensionConfig typed_config; + const std::string yaml = R"EOF( + name: envoy.tracers.opentelemetry.samplers.always_on + typed_config: + "@type": type.googleapis.com/envoy.extensions.tracers.opentelemetry.samplers.v3.AlwaysOnSamplerConfig + )EOF"; + TestUtility::loadFromYaml(yaml, typed_config); + NiceMock context; + EXPECT_NE(factory->createSampler(typed_config.typed_config(), context), nullptr); + EXPECT_STREQ(factory->name().c_str(), "envoy.tracers.opentelemetry.samplers.always_on"); +} + +} // namespace OpenTelemetry +} // namespace Tracers +} // namespace Extensions +} // namespace Envoy diff --git a/test/extensions/tracers/opentelemetry/samplers/sampler_test.cc b/test/extensions/tracers/opentelemetry/samplers/sampler_test.cc new file mode 100644 index 000000000000..f80103b4345c --- /dev/null +++ b/test/extensions/tracers/opentelemetry/samplers/sampler_test.cc @@ -0,0 +1,193 @@ +#include + +#include "envoy/config/trace/v3/opentelemetry.pb.h" +#include "envoy/registry/registry.h" + +#include "source/common/tracing/http_tracer_impl.h" +#include "source/extensions/tracers/opentelemetry/opentelemetry_tracer_impl.h" +#include "source/extensions/tracers/opentelemetry/samplers/sampler.h" +#include "source/extensions/tracers/opentelemetry/span_context.h" + +#include "test/mocks/server/tracer_factory_context.h" +#include "test/test_common/registry.h" +#include "test/test_common/utility.h" + +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +namespace Envoy { +namespace Extensions { +namespace Tracers { +namespace OpenTelemetry { + +using ::testing::NiceMock; +using ::testing::StrictMock; + +class TestSampler : public Sampler { +public: + MOCK_METHOD(SamplingResult, shouldSample, + ((const absl::optional), (const std::string&), (const std::string&), + (::opentelemetry::proto::trace::v1::Span::SpanKind), + (const std::map&), (const std::vector&)), + (override)); + MOCK_METHOD(std::string, getDescription, (), (const, override)); +}; + +class TestSamplerFactory : public SamplerFactory { +public: + MOCK_METHOD(SamplerSharedPtr, createSampler, + (const Protobuf::Message& message, + Server::Configuration::TracerFactoryContext& context)); + + ProtobufTypes::MessagePtr createEmptyConfigProto() override { + return std::make_unique(); + } + + std::string name() const override { return "envoy.tracers.opentelemetry.samplers.testsampler"; } +}; + +class SamplerFactoryTest : public testing::Test { + +protected: + NiceMock config; + NiceMock stream_info; + Tracing::TestTraceContextImpl trace_context{}; + NiceMock context; +}; + +// Test OTLP tracer without a sampler +TEST_F(SamplerFactoryTest, TestWithoutSampler) { + // using StrictMock, calls to SamplerFactory would cause a test failure + auto test_sampler = std::make_shared>(); + StrictMock sampler_factory; + Registry::InjectFactory sampler_factory_registration(sampler_factory); + + // no sampler configured + const std::string yaml_string = R"EOF( + grpc_service: + envoy_grpc: + cluster_name: fake-cluster + timeout: 0.250s + service_name: my-service + )EOF"; + + envoy::config::trace::v3::OpenTelemetryConfig opentelemetry_config; + TestUtility::loadFromYaml(yaml_string, opentelemetry_config); + + auto driver = std::make_unique(opentelemetry_config, context); + + driver->startSpan(config, trace_context, stream_info, "operation_name", + {Tracing::Reason::Sampling, true}); +} + +// Test config containing an unknown sampler +TEST_F(SamplerFactoryTest, TestWithInvalidSampler) { + // using StrictMock, calls to SamplerFactory would cause a test failure + auto test_sampler = std::make_shared>(); + StrictMock sampler_factory; + Registry::InjectFactory sampler_factory_registration(sampler_factory); + + // invalid sampler configured + const std::string yaml_string = R"EOF( + grpc_service: + envoy_grpc: + cluster_name: fake-cluster + timeout: 0.250s + service_name: my-service + sampler: + name: envoy.tracers.opentelemetry.samplers.testsampler + typed_config: + "@type": type.googleapis.com/google.protobuf.Value + )EOF"; + + envoy::config::trace::v3::OpenTelemetryConfig opentelemetry_config; + TestUtility::loadFromYaml(yaml_string, opentelemetry_config); + + EXPECT_THROW(std::make_unique(opentelemetry_config, context), EnvoyException); +} + +// Test OTLP tracer with a sampler +TEST_F(SamplerFactoryTest, TestWithSampler) { + auto test_sampler = std::make_shared>(); + TestSamplerFactory sampler_factory; + Registry::InjectFactory sampler_factory_registration(sampler_factory); + + EXPECT_CALL(sampler_factory, createSampler(_, _)).WillOnce(Return(test_sampler)); + + const std::string yaml_string = R"EOF( + grpc_service: + envoy_grpc: + cluster_name: fake-cluster + timeout: 0.250s + service_name: my-service + sampler: + name: envoy.tracers.opentelemetry.samplers.testsampler + typed_config: + "@type": type.googleapis.com/google.protobuf.Struct + )EOF"; + + envoy::config::trace::v3::OpenTelemetryConfig opentelemetry_config; + TestUtility::loadFromYaml(yaml_string, opentelemetry_config); + + auto driver = std::make_unique(opentelemetry_config, context); + + // shouldSample returns a result without additional attributes and Decision::RECORD_AND_SAMPLE + EXPECT_CALL(*test_sampler, shouldSample(_, _, _, _, _, _)) + .WillOnce([](const absl::optional, const std::string&, const std::string&, + ::opentelemetry::proto::trace::v1::Span::SpanKind, + const std::map&, const std::vector&) { + SamplingResult res; + res.decision = Decision::RECORD_AND_SAMPLE; + res.tracestate = "this_is=tracesate"; + return res; + }); + + Tracing::SpanPtr tracing_span = driver->startSpan( + config, trace_context, stream_info, "operation_name", {Tracing::Reason::Sampling, true}); + // startSpan returns a Tracing::SpanPtr. Tracing::Span has no sampled() method. + // We know that the underlying span is Extensions::Tracers::OpenTelemetry::Span + // So the dynamic_cast should be safe. + std::unique_ptr span(dynamic_cast(tracing_span.release())); + EXPECT_TRUE(span->sampled()); + EXPECT_STREQ(span->tracestate().c_str(), "this_is=tracesate"); + + // shouldSamples return a result containing additional attributes and Decision::DROP + EXPECT_CALL(*test_sampler, shouldSample(_, _, _, _, _, _)) + .WillOnce([](const absl::optional, const std::string&, const std::string&, + ::opentelemetry::proto::trace::v1::Span::SpanKind, + const std::map&, const std::vector&) { + SamplingResult res; + res.decision = Decision::DROP; + std::map attributes; + attributes["key"] = "value"; + attributes["another_key"] = "another_value"; + res.attributes = + std::make_unique>(std::move(attributes)); + res.tracestate = "this_is=another_tracesate"; + return res; + }); + tracing_span = driver->startSpan(config, trace_context, stream_info, "operation_name", + {Tracing::Reason::Sampling, true}); + std::unique_ptr unsampled_span(dynamic_cast(tracing_span.release())); + EXPECT_FALSE(unsampled_span->sampled()); + EXPECT_STREQ(unsampled_span->tracestate().c_str(), "this_is=another_tracesate"); +} + +// Test sampling result decision +TEST(SamplingResultTest, TestSamplingResult) { + SamplingResult result; + result.decision = Decision::RECORD_AND_SAMPLE; + EXPECT_TRUE(result.isRecording()); + EXPECT_TRUE(result.isSampled()); + result.decision = Decision::RECORD_ONLY; + EXPECT_TRUE(result.isRecording()); + EXPECT_FALSE(result.isSampled()); + result.decision = Decision::DROP; + EXPECT_FALSE(result.isRecording()); + EXPECT_FALSE(result.isSampled()); +} + +} // namespace OpenTelemetry +} // namespace Tracers +} // namespace Extensions +} // namespace Envoy diff --git a/tools/extensions/extensions_schema.yaml b/tools/extensions/extensions_schema.yaml index 36181fb61786..3112b8797940 100644 --- a/tools/extensions/extensions_schema.yaml +++ b/tools/extensions/extensions_schema.yaml @@ -136,6 +136,7 @@ categories: - envoy.http.early_header_mutation - envoy.http.custom_response - envoy.router.cluster_specifier_plugin +- envoy.tracers.opentelemetry.samplers status_values: - name: stable From 3bf0fb6de4709cbfc221bdb864dc2153bb0b75d4 Mon Sep 17 00:00:00 2001 From: "thomas.ebner" Date: Tue, 24 Oct 2023 11:07:44 +0200 Subject: [PATCH 02/21] add integration test, remove redundant method, do not set empty tracestate Signed-off-by: thomas.ebner --- .../tracers/opentelemetry/tracer.cc | 4 +- .../extensions/tracers/opentelemetry/tracer.h | 4 +- .../opentelemetry/samplers/always_on/BUILD | 14 ++ .../always_on_sampler_integration_test.cc | 142 ++++++++++++++++++ 4 files changed, 160 insertions(+), 4 deletions(-) create mode 100644 test/extensions/tracers/opentelemetry/samplers/always_on/always_on_sampler_integration_test.cc diff --git a/source/extensions/tracers/opentelemetry/tracer.cc b/source/extensions/tracers/opentelemetry/tracer.cc index 8a4da9cd5a6c..d786cd1cea4a 100644 --- a/source/extensions/tracers/opentelemetry/tracer.cc +++ b/source/extensions/tracers/opentelemetry/tracer.cc @@ -40,7 +40,9 @@ void callSampler(SamplerSharedPtr sampler, const absl::optional spa new_span.setTag(attribute.first, attribute.second); } } - new_span.setTracestate(sampling_result.tracestate); + if (!sampling_result.tracestate.empty()) { + new_span.setTracestate(sampling_result.tracestate); + } } } // namespace diff --git a/source/extensions/tracers/opentelemetry/tracer.h b/source/extensions/tracers/opentelemetry/tracer.h index 6433d8299325..f5204bfdf05c 100644 --- a/source/extensions/tracers/opentelemetry/tracer.h +++ b/source/extensions/tracers/opentelemetry/tracer.h @@ -116,8 +116,6 @@ class Span : Logger::Loggable, public Tracing::Span { ::opentelemetry::proto::trace::v1::Span::SpanKind spankind() const { return span_.kind(); } - std::string tracestate() const { return span_.trace_state(); } - /** * Sets the span's id. */ @@ -134,7 +132,7 @@ class Span : Logger::Loggable, public Tracing::Span { span_.set_parent_span_id(absl::HexStringToBytes(parent_span_id_hex)); } - std::string tracestate() { return span_.trace_state(); } + std::string tracestate() const { return span_.trace_state(); } /** * Sets the span's tracestate. diff --git a/test/extensions/tracers/opentelemetry/samplers/always_on/BUILD b/test/extensions/tracers/opentelemetry/samplers/always_on/BUILD index 342679cf14a0..063cda9f0ec1 100644 --- a/test/extensions/tracers/opentelemetry/samplers/always_on/BUILD +++ b/test/extensions/tracers/opentelemetry/samplers/always_on/BUILD @@ -35,3 +35,17 @@ envoy_extension_cc_test( "@envoy_api//envoy/extensions/tracers/opentelemetry/samplers/v3:pkg_cc_proto", ], ) + +envoy_extension_cc_test( + name = "always_on_sampler_integration_test", + srcs = [ + "always_on_sampler_integration_test.cc", + ], + extension_names = ["envoy.tracers.opentelemetry.samplers.always_on"], + deps = [ + "//source/exe:main_common_lib", + "//test/integration:http_integration_lib", + "//test/test_common:utility_lib", + "@envoy_api//envoy/extensions/filters/network/http_connection_manager/v3:pkg_cc_proto", + ], +) diff --git a/test/extensions/tracers/opentelemetry/samplers/always_on/always_on_sampler_integration_test.cc b/test/extensions/tracers/opentelemetry/samplers/always_on/always_on_sampler_integration_test.cc new file mode 100644 index 000000000000..051a21b6846f --- /dev/null +++ b/test/extensions/tracers/opentelemetry/samplers/always_on/always_on_sampler_integration_test.cc @@ -0,0 +1,142 @@ +#include +#include + +#include "envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.pb.h" + +#include "test/integration/http_integration.h" +#include "test/test_common/utility.h" + +#include "absl/strings/match.h" +#include "absl/strings/string_view.h" +#include "gtest/gtest.h" + +namespace Envoy { +namespace Extensions { +namespace Tracers { +namespace OpenTelemetry { +namespace { + +const char* TRACEPARENT_VALUE = "00-0af7651916cd43dd8448eb211c80319c-b7ad6b7169203331-01"; +const char* TRACEPARENT_VALUE_START = "00-0af7651916cd43dd8448eb211c80319c"; + +class AlwaysOnSamplerIntegrationTest : public Envoy::HttpIntegrationTest, + public testing::TestWithParam { +public: + AlwaysOnSamplerIntegrationTest() : HttpIntegrationTest(Http::CodecType::HTTP1, GetParam()) { + + const std::string yaml_string = 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.AlwaysOnSamplerConfig + )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, AlwaysOnSamplerIntegrationTest, + testing::ValuesIn(TestEnvironment::getIpVersionsForTest()), + TestUtility::ipTestParamsToString); + +// Sends a request with traceparent and tracestate header. +TEST_P(AlwaysOnSamplerIntegrationTest, TestWithTraceparentAndTracestate) { + Http::TestRequestHeaderMapImpl request_headers{ + {":method", "GET"}, {":path", "/test/long/url"}, {":scheme", "http"}, + {":authority", "host"}, {"tracestate", "key=value"}, {"traceparent", TRACEPARENT_VALUE}}; + + 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 should be set: traceid should be re-used, span id should be different + absl::string_view traceparent_value = upstream_request_->headers() + .get(Http::LowerCaseString("traceparent"))[0] + ->value() + .getStringView(); + EXPECT_TRUE(absl::StartsWith(traceparent_value, TRACEPARENT_VALUE_START)); + EXPECT_NE(TRACEPARENT_VALUE, traceparent_value); + // tracestate should be forwarded + absl::string_view tracestate_value = upstream_request_->headers() + .get(Http::LowerCaseString("tracestate"))[0] + ->value() + .getStringView(); + EXPECT_EQ("key=value", tracestate_value); +} + +// Sends a request with traceparent but no tracestate header. +TEST_P(AlwaysOnSamplerIntegrationTest, TestWithTraceparentOnly) { + Http::TestRequestHeaderMapImpl request_headers{{":method", "GET"}, + {":path", "/test/long/url"}, + {":scheme", "http"}, + {":authority", "host"}, + {"traceparent", TRACEPARENT_VALUE}}; + 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 should be set: traceid should be re-used, span id should be different + absl::string_view traceparent_value = upstream_request_->headers() + .get(Http::LowerCaseString("traceparent"))[0] + ->value() + .getStringView(); + EXPECT_TRUE(absl::StartsWith(traceparent_value, TRACEPARENT_VALUE_START)); + EXPECT_NE(TRACEPARENT_VALUE, traceparent_value); + // OTLP tracer adds an empty tracestate + absl::string_view tracestate_value = upstream_request_->headers() + .get(Http::LowerCaseString("tracestate"))[0] + ->value() + .getStringView(); + EXPECT_EQ("", tracestate_value); +} + +// Sends a request without traceparent and tracestate header. +TEST_P(AlwaysOnSamplerIntegrationTest, TestWithoutTraceparentAndTracestate) { + 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); + // OTLP tracer adds an empty tracestate + absl::string_view tracestate_value = upstream_request_->headers() + .get(Http::LowerCaseString("tracestate"))[0] + ->value() + .getStringView(); + EXPECT_EQ("", tracestate_value); +} + +} // namespace +} // namespace OpenTelemetry +} // namespace Tracers +} // namespace Extensions +} // namespace Envoy From 2125d97c4301457a9da5dc85de96bd22955962e0 Mon Sep 17 00:00:00 2001 From: "thomas.ebner" Date: Thu, 19 Oct 2023 12:50:08 +0200 Subject: [PATCH 03/21] add dynatrace sampler Signed-off-by: thomas.ebner --- .../samplers/v3/dynatrace_sampler.proto | 20 +++ source/extensions/extensions_build_config.bzl | 1 + .../opentelemetry/samplers/dynatrace/BUILD | 41 ++++++ .../samplers/dynatrace/config.cc | 34 +++++ .../opentelemetry/samplers/dynatrace/config.h | 41 ++++++ .../samplers/dynatrace/dynatrace_sampler.cc | 85 +++++++++++ .../samplers/dynatrace/dynatrace_sampler.h | 47 +++++++ .../samplers/dynatrace/dynatrace_tracestate.h | 120 ++++++++++++++++ .../samplers/dynatrace/tracestate.cc | 45 ++++++ .../samplers/dynatrace/tracestate.h | 40 ++++++ .../opentelemetry/samplers/dynatrace/BUILD | 39 ++++++ .../samplers/dynatrace/config_test.cc | 37 +++++ .../dynatrace/dynatrace_tracestate_test.cc | 21 +++ .../samplers/dynatrace/tracestate_test.cc | 132 ++++++++++++++++++ 14 files changed, 703 insertions(+) create mode 100644 api/envoy/extensions/tracers/opentelemetry/samplers/v3/dynatrace_sampler.proto create mode 100644 source/extensions/tracers/opentelemetry/samplers/dynatrace/BUILD create mode 100644 source/extensions/tracers/opentelemetry/samplers/dynatrace/config.cc create mode 100644 source/extensions/tracers/opentelemetry/samplers/dynatrace/config.h create mode 100644 source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.cc create mode 100644 source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.h create mode 100644 source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_tracestate.h create mode 100644 source/extensions/tracers/opentelemetry/samplers/dynatrace/tracestate.cc create mode 100644 source/extensions/tracers/opentelemetry/samplers/dynatrace/tracestate.h create mode 100644 test/extensions/tracers/opentelemetry/samplers/dynatrace/BUILD create mode 100644 test/extensions/tracers/opentelemetry/samplers/dynatrace/config_test.cc create mode 100644 test/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_tracestate_test.cc create mode 100644 test/extensions/tracers/opentelemetry/samplers/dynatrace/tracestate_test.cc diff --git a/api/envoy/extensions/tracers/opentelemetry/samplers/v3/dynatrace_sampler.proto b/api/envoy/extensions/tracers/opentelemetry/samplers/v3/dynatrace_sampler.proto new file mode 100644 index 000000000000..d02072f1e5cc --- /dev/null +++ b/api/envoy/extensions/tracers/opentelemetry/samplers/v3/dynatrace_sampler.proto @@ -0,0 +1,20 @@ +syntax = "proto3"; + +package envoy.extensions.tracers.opentelemetry.samplers.v3; + +import "udpa/annotations/status.proto"; + +option java_package = "io.envoyproxy.envoy.extensions.tracers.opentelemetry.samplers.v3"; +option java_outer_classname = "DynatraceSamplerProto"; +option java_multiple_files = true; +option go_package = "github.com/envoyproxy/go-control-plane/envoy/extensions/tracers/opentelemetry/samplers/v3;samplersv3"; +option (udpa.annotations.file_status).package_version_status = ACTIVE; + +// [#protodoc-title: Always On Sampler config] +// [#extension: envoy.tracers.opentelemetry.samplers.dynatrace] + +message DynatraceSamplerConfig { + string tenant_id = 1; + + string cluster_id = 2; +} diff --git a/source/extensions/extensions_build_config.bzl b/source/extensions/extensions_build_config.bzl index a1faca176ed9..83e4600448cf 100644 --- a/source/extensions/extensions_build_config.bzl +++ b/source/extensions/extensions_build_config.bzl @@ -270,6 +270,7 @@ EXTENSIONS = { # "envoy.tracers.opentelemetry.samplers.always_on": "//source/extensions/tracers/opentelemetry/samplers/always_on:config", + "envoy.tracers.opentelemetry.samplers.dynatrace": "//source/extensions/tracers/opentelemetry/samplers/dynatrace:config", # # Transport sockets diff --git a/source/extensions/tracers/opentelemetry/samplers/dynatrace/BUILD b/source/extensions/tracers/opentelemetry/samplers/dynatrace/BUILD new file mode 100644 index 000000000000..a89eb440e79d --- /dev/null +++ b/source/extensions/tracers/opentelemetry/samplers/dynatrace/BUILD @@ -0,0 +1,41 @@ +load( + "//bazel:envoy_build_system.bzl", + "envoy_cc_extension", + "envoy_cc_library", + "envoy_extension_package", +) + +licenses(["notice"]) # Apache 2 + +envoy_extension_package() + +envoy_cc_extension( + name = "config", + srcs = ["config.cc"], + hdrs = ["config.h"], + deps = [ + ":dynatrace_sampler_lib", + "//envoy/registry", + "//source/common/config:utility_lib", + "@envoy_api//envoy/extensions/tracers/opentelemetry/samplers/v3:pkg_cc_proto", + ], +) + +envoy_cc_library( + name = "dynatrace_sampler_lib", + srcs = [ + "dynatrace_sampler.cc", + "tracestate.cc", + ], + hdrs = [ + "dynatrace_sampler.h", + "dynatrace_tracestate.h", + "tracestate.h", + ], + deps = [ + "//source/common/config:datasource_lib", + "//source/extensions/tracers/opentelemetry:opentelemetry_tracer_lib", + "//source/extensions/tracers/opentelemetry/samplers:sampler_lib", + "@envoy_api//envoy/extensions/tracers/opentelemetry/samplers/v3:pkg_cc_proto", + ], +) diff --git a/source/extensions/tracers/opentelemetry/samplers/dynatrace/config.cc b/source/extensions/tracers/opentelemetry/samplers/dynatrace/config.cc new file mode 100644 index 000000000000..22ace4bd3cfe --- /dev/null +++ b/source/extensions/tracers/opentelemetry/samplers/dynatrace/config.cc @@ -0,0 +1,34 @@ +#include "config.h" + +#include "envoy/extensions/tracers/opentelemetry/samplers/v3/dynatrace_sampler.pb.validate.h" + +#include "source/common/config/utility.h" +#include "source/common/protobuf/utility.h" + +#include "dynatrace_sampler.h" + +namespace Envoy { +namespace Extensions { +namespace Tracers { +namespace OpenTelemetry { + +SamplerSharedPtr +DynatraceSamplerFactory::createSampler(const Protobuf::Message& config, + Server::Configuration::TracerFactoryContext& context) { + auto mptr = Envoy::Config::Utility::translateAnyToFactoryConfig( + dynamic_cast(config), context.messageValidationVisitor(), *this); + return std::make_shared( + MessageUtil::downcastAndValidate< + const envoy::extensions::tracers::opentelemetry::samplers::v3::DynatraceSamplerConfig&>( + *mptr, context.messageValidationVisitor())); +} + +/** + * Static registration for the Env sampler factory. @see RegisterFactory. + */ +REGISTER_FACTORY(DynatraceSamplerFactory, SamplerFactory); + +} // namespace OpenTelemetry +} // namespace Tracers +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/tracers/opentelemetry/samplers/dynatrace/config.h b/source/extensions/tracers/opentelemetry/samplers/dynatrace/config.h new file mode 100644 index 000000000000..bc1baf6c3451 --- /dev/null +++ b/source/extensions/tracers/opentelemetry/samplers/dynatrace/config.h @@ -0,0 +1,41 @@ +#pragma once + +#include + +#include "envoy/extensions/tracers/opentelemetry/samplers/v3/dynatrace_sampler.pb.h" +#include "envoy/registry/registry.h" + +#include "source/extensions/tracers/opentelemetry/samplers/sampler.h" + +namespace Envoy { +namespace Extensions { +namespace Tracers { +namespace OpenTelemetry { + +/** + * Config registration for the DynatraceSampler. @see SamplerFactory. + */ +class DynatraceSamplerFactory : public SamplerFactory { +public: + /** + * @brief Create a Sampler which samples every span + * + * @param context + * @return SamplerSharedPtr + */ + SamplerSharedPtr createSampler(const Protobuf::Message& config, + Server::Configuration::TracerFactoryContext& context) override; + + ProtobufTypes::MessagePtr createEmptyConfigProto() override { + return std::make_unique< + envoy::extensions::tracers::opentelemetry::samplers::v3::DynatraceSamplerConfig>(); + } + std::string name() const override { return "envoy.tracers.opentelemetry.samplers.dynatrace"; } +}; + +DECLARE_FACTORY(DynatraceSamplerFactory); + +} // namespace OpenTelemetry +} // namespace Tracers +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.cc b/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.cc new file mode 100644 index 000000000000..0e1160c21684 --- /dev/null +++ b/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.cc @@ -0,0 +1,85 @@ +#include "dynatrace_sampler.h" + +#include +#include +#include + +#include "source/common/config/datasource.h" +#include "source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_tracestate.h" +#include "source/extensions/tracers/opentelemetry/samplers/dynatrace/tracestate.h" +#include "source/extensions/tracers/opentelemetry/samplers/sampler.h" +#include "source/extensions/tracers/opentelemetry/span_context.h" + +namespace Envoy { +namespace Extensions { +namespace Tracers { +namespace OpenTelemetry { + +static const char* SAMPLING_EXTRAPOLATION_SPAN_ATTRIBUTE_NAME = + "sampling_extrapolation_set_in_sampler"; + +DynatraceTracestate DynatraceSampler::createTracestate(bool is_recording, + std::string sampling_exponent) { + DynatraceTracestate tracestate; + tracestate.sampling_exponent = sampling_exponent; + tracestate.tenant_id = tenant_id_; + tracestate.cluster_id = cluster_id_; + tracestate.is_ignored = is_recording ? "0" : "1"; + return tracestate; +} + +FW4Tag DynatraceSampler::getFW4Tag(const Tracestate& tracestate) { + for (auto const& entry : tracestate.entries()) { + if (dynatrace_tracestate_.keyMatches( + entry.key)) { // found a tracestate entry with key matching our tenant/cluster + return FW4Tag::create(entry.value); + } + } + return FW4Tag::createInvalid(); +} + +SamplingResult +DynatraceSampler::shouldSample(const absl::optional parent_context, + const std::string& /*trace_id*/, const std::string& /*name*/, + ::opentelemetry::proto::trace::v1::Span::SpanKind /*kind*/, + const std::map& /*attributes*/, + const std::vector& /*links*/) { + + SamplingResult result; + std::map att; + + Tracestate tracestate; + tracestate.parse(parent_context.has_value() ? parent_context->tracestate() : ""); + + FW4Tag fw4_tag = getFW4Tag(tracestate); + + if (fw4_tag.isValid()) { // we found a trace decision in tracestate header + result.decision = fw4_tag.isIgnored() ? Decision::DROP : Decision::RECORD_AND_SAMPLE; + att[SAMPLING_EXTRAPOLATION_SPAN_ATTRIBUTE_NAME] = fw4_tag.getSamplingExponent(); + result.tracestate = parent_context->tracestate(); + + } else { // make a sampling decision + bool sample = true; // TODO + int sampling_exponent = 8; // TODO + att[SAMPLING_EXTRAPOLATION_SPAN_ATTRIBUTE_NAME] = sampling_exponent; + + result.decision = sample ? Decision::RECORD_AND_SAMPLE : Decision::DROP; + + FW4Tag new_tag = FW4Tag::create(!sample, sampling_exponent); + tracestate.add(dynatrace_tracestate_.getKey(), new_tag.createValue()); + result.tracestate = tracestate.asString(); + att[SAMPLING_EXTRAPOLATION_SPAN_ATTRIBUTE_NAME] = sampling_exponent; + } + + if (!att.empty()) { + result.attributes = std::make_unique>(std::move(att)); + } + return result; +} + +std::string DynatraceSampler::getDescription() const { return "DynatraceSampler"; } + +} // namespace OpenTelemetry +} // namespace Tracers +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.h b/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.h new file mode 100644 index 000000000000..5f554ce2a4ed --- /dev/null +++ b/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.h @@ -0,0 +1,47 @@ +#pragma once + +#include "envoy/extensions/tracers/opentelemetry/samplers/v3/dynatrace_sampler.pb.h" +#include "envoy/server/factory_context.h" + +#include "source/common/common/logger.h" +#include "source/common/config/datasource.h" +#include "source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_tracestate.h" +#include "source/extensions/tracers/opentelemetry/samplers/dynatrace/tracestate.h" +#include "source/extensions/tracers/opentelemetry/samplers/sampler.h" + +namespace Envoy { +namespace Extensions { +namespace Tracers { +namespace OpenTelemetry { + +/** + * @brief A Dynatrace specific sampler * + */ +class DynatraceSampler : public Sampler, Logger::Loggable { +public: + explicit DynatraceSampler( + const envoy::extensions::tracers::opentelemetry::samplers::v3::DynatraceSamplerConfig config) + : tenant_id_(config.tenant_id()), cluster_id_(config.cluster_id()), + dynatrace_tracestate_(tenant_id_, cluster_id_) {} + + SamplingResult shouldSample(const absl::optional parent_context, + const std::string& trace_id, const std::string& name, + ::opentelemetry::proto::trace::v1::Span::SpanKind spankind, + const std::map& attributes, + const std::vector& links) override; + + std::string getDescription() const override; + +private: + std::string tenant_id_; + std::string cluster_id_; + DynatraceTracestate dynatrace_tracestate_; + + FW4Tag getFW4Tag(const Tracestate& tracestate); + DynatraceTracestate createTracestate(bool is_recording, std::string sampling_exponent); +}; + +} // namespace OpenTelemetry +} // namespace Tracers +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_tracestate.h b/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_tracestate.h new file mode 100644 index 000000000000..e0f973f1652d --- /dev/null +++ b/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_tracestate.h @@ -0,0 +1,120 @@ +#pragma once + +#include +#include +#include +#include + +#include "absl/strings/match.h" +#include "absl/strings/str_cat.h" +#include "absl/strings/str_split.h" +#include "absl/strings/string_view.h" + +namespace Envoy { +namespace Extensions { +namespace Tracers { +namespace OpenTelemetry { + +class FW4Tag { +public: + static FW4Tag createInvalid() { return {false, false, 0}; } + + static FW4Tag create(bool ignored, int sampling_exponent) { + return {true, ignored, sampling_exponent}; + } + + static FW4Tag create(const std::string& value) { + std::vector tracestate_components = + absl::StrSplit(value, ';', absl::AllowEmpty()); + if (tracestate_components.size() < 7) { + return createInvalid(); + } + + if (tracestate_components[0] != "fw4") { + return createInvalid(); + } + bool ignored = tracestate_components[5] == "1"; + int sampling_exponent = std::stoi(std::string(tracestate_components[6])); + return {true, ignored, sampling_exponent}; + } + + FW4Tag(bool valid, bool ignored, int sampling_exponent) + : valid_(valid), ignored_(ignored), sampling_exponent_(sampling_exponent) {} + + bool isValid() const { return valid_; }; + bool isIgnored() const { return ignored_; }; + int getSamplingExponent() const { return sampling_exponent_; }; + + std::string createValue() const { + std::string ret = absl::StrCat("fw4;0;0;0;0;", ignored_ ? "1" : "0", ";", sampling_exponent_); + return ret; + } + +private: + bool valid_; + bool ignored_; + int sampling_exponent_; +}; + +// -@dt=fw4;0;0;0;0;;; +class DynatraceTracestate { +public: + DynatraceTracestate() = default; + + DynatraceTracestate(const std::string& tenant_id, const std::string& cluster_id) + : tenant_id_(tenant_id), cluster_id_(cluster_id) { + key_ = absl::StrCat(absl::string_view(tenant_id_), "-", absl::string_view(cluster_id), "@dt"); + } + + std::string getKey() const { return key_; }; + + bool keyMatches(const std::string& key) { return (key_.compare(key) == 0); } + + bool isIgnored(const std::string& value) { + std::vector tracestate_components = + absl::StrSplit(value, ';', absl::AllowEmpty()); + if (tracestate_components.size() > 7) { + return tracestate_components[5] == "1"; + } + return false; + } + + std::string tenant_id = ""; + std::string cluster_id = ""; + static constexpr absl::string_view at_dt_format = "@dt=fw4"; + static constexpr absl::string_view server_id = "0"; + static constexpr absl::string_view agent_d = "0"; + static constexpr absl::string_view tag_id = "0"; + static constexpr absl::string_view link_id = "0"; + std::string is_ignored = "0"; + std::string sampling_exponent = "0"; + std::string path_info = "0"; + std::string span_id = ""; + + std::string toString() { + std::string tracestate = absl::StrCat( + absl::string_view(tenant_id), "-", absl::string_view(cluster_id), at_dt_format, ";", + server_id, ";", agent_d, ";", tag_id, ";", link_id, ";", absl::string_view(is_ignored), ";", + absl::string_view(sampling_exponent), ";", absl::string_view(path_info)); + + // Which is: "abcdabcd-77@dt=fw4;8;66666666;111;99;0;0;66;;8cef;2h01;7h293e72b548735604" + if (!span_id.empty()) { + std::string tmp(tracestate); + // https://oaad.lab.dynatrace.org/agent/concepts/purepath/tagging/#forward-tags-fw + std::string extension = ";7h" + span_id; + std::string hash_as_hex = ""; + tracestate = absl::StrCat(absl::string_view(tmp), ";", absl::string_view(hash_as_hex), + absl::string_view(extension)); + } + return tracestate; + } + +private: + std::string tenant_id_; + std::string cluster_id_; + std::string key_; +}; +} // namespace OpenTelemetry +} // namespace Tracers +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/tracers/opentelemetry/samplers/dynatrace/tracestate.cc b/source/extensions/tracers/opentelemetry/samplers/dynatrace/tracestate.cc new file mode 100644 index 000000000000..688ce69898e2 --- /dev/null +++ b/source/extensions/tracers/opentelemetry/samplers/dynatrace/tracestate.cc @@ -0,0 +1,45 @@ +#include "source/extensions/tracers/opentelemetry/samplers/dynatrace/tracestate.h" + +#include "source/common/common/utility.h" + +#include "absl/strings/str_split.h" +#include "absl/strings/string_view.h" + +namespace Envoy { +namespace Extensions { +namespace Tracers { +namespace OpenTelemetry { + +void Tracestate::parse(const std::string& tracestate) { + clear(); + raw_trace_state_ = tracestate; + std::vector list_members = absl::StrSplit(tracestate, ',', absl::SkipEmpty()); + for (auto const& list_member : list_members) { + std::vector kv = absl::StrSplit(list_member, '=', absl::SkipEmpty()); + if (kv.size() != 2) { + // unexpected entry, ignore + continue; + } + absl::string_view key = StringUtil::ltrim(kv[0]); + absl::string_view val = StringUtil::rtrim(kv[1]); + TracestateEntry entry{{key.data(), key.size()}, {val.data(), val.size()}}; + entries_.push_back(std::move(entry)); + } +} + +void Tracestate::add(const std::string& key, const std::string& value) { + std::string tracestate = + absl::StrCat(absl::string_view(key), "=", absl::string_view(value), + raw_trace_state_.empty() > 0 ? "" : ",", absl::string_view(raw_trace_state_)); + parse(tracestate); +} + +void Tracestate::clear() { + entries_.clear(); + raw_trace_state_.clear(); +} + +} // namespace OpenTelemetry +} // namespace Tracers +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/tracers/opentelemetry/samplers/dynatrace/tracestate.h b/source/extensions/tracers/opentelemetry/samplers/dynatrace/tracestate.h new file mode 100644 index 000000000000..5eea07a80466 --- /dev/null +++ b/source/extensions/tracers/opentelemetry/samplers/dynatrace/tracestate.h @@ -0,0 +1,40 @@ +#pragma once + +#include +#include + +namespace Envoy { +namespace Extensions { +namespace Tracers { +namespace OpenTelemetry { + +class TracestateEntry { +public: + std::string key; + std::string value; +}; + +/** + * @brief parses and manipultes W3C tracestate header + * see https://www.w3.org/TR/trace-context/#tracestate-header + * + */ +class Tracestate { +public: + void parse(const std::string& tracestate); + + void add(const std::string& key, const std::string& value); + std::vector entries() const { return entries_; } + + std::string asString() const { return raw_trace_state_; } + +private: + std::vector entries_; + std::string raw_trace_state_; + + void clear(); +}; +} // namespace OpenTelemetry +} // namespace Tracers +} // namespace Extensions +} // namespace Envoy diff --git a/test/extensions/tracers/opentelemetry/samplers/dynatrace/BUILD b/test/extensions/tracers/opentelemetry/samplers/dynatrace/BUILD new file mode 100644 index 000000000000..7b52709e93de --- /dev/null +++ b/test/extensions/tracers/opentelemetry/samplers/dynatrace/BUILD @@ -0,0 +1,39 @@ +load( + "//bazel:envoy_build_system.bzl", + "envoy_package", +) +load( + "//test/extensions:extensions_build_system.bzl", + "envoy_extension_cc_test", +) + +licenses(["notice"]) # Apache 2 + +envoy_package() + +envoy_extension_cc_test( + name = "config_test", + srcs = ["config_test.cc"], + extension_names = ["envoy.tracers.opentelemetry.samplers.dynatrace"], + deps = [ + "//envoy/registry", + "//source/extensions/tracers/opentelemetry/samplers/dynatrace:config", + "//source/extensions/tracers/opentelemetry/samplers/dynatrace:dynatrace_sampler_lib", + "//test/mocks/server:tracer_factory_context_mocks", + "//test/test_common:utility_lib", + ], +) + +envoy_extension_cc_test( + name = "dynatrace_sampler_test", + srcs = [ + "dynatrace_tracestate_test.cc", + "tracestate_test.cc", + ], + extension_names = ["envoy.tracers.opentelemetry.samplers.dynatrace"], + deps = [ + "//source/extensions/tracers/opentelemetry/samplers/dynatrace:dynatrace_sampler_lib", + "//test/mocks/server:tracer_factory_context_mocks", + "//test/test_common:utility_lib", + ], +) diff --git a/test/extensions/tracers/opentelemetry/samplers/dynatrace/config_test.cc b/test/extensions/tracers/opentelemetry/samplers/dynatrace/config_test.cc new file mode 100644 index 000000000000..f4aa70929603 --- /dev/null +++ b/test/extensions/tracers/opentelemetry/samplers/dynatrace/config_test.cc @@ -0,0 +1,37 @@ +#include "envoy/registry/registry.h" + +#include "source/extensions/tracers/opentelemetry/samplers/dynatrace/config.h" + +#include "test/mocks/server/tracer_factory_context.h" +#include "test/test_common/utility.h" + +#include "gtest/gtest.h" + +namespace Envoy { +namespace Extensions { +namespace Tracers { +namespace OpenTelemetry { + +TEST(DynatraceSamplerFactoryTest, Test) { + auto* factory = Registry::FactoryRegistry::getFactory( + "envoy.tracers.opentelemetry.samplers.dynatrace"); + ASSERT_NE(factory, nullptr); + EXPECT_STREQ(factory->name().c_str(), "envoy.tracers.opentelemetry.samplers.dynatrace"); + EXPECT_NE(factory->createEmptyConfigProto(), nullptr); + + envoy::config::core::v3::TypedExtensionConfig typed_config; + const std::string yaml = R"EOF( + name: envoy.tracers.opentelemetry.samplers.dynatrace + typed_config: + "@type": type.googleapis.com/envoy.extensions.tracers.opentelemetry.samplers.v3.DynatraceSamplerConfig + )EOF"; + TestUtility::loadFromYaml(yaml, typed_config); + NiceMock context; + EXPECT_NE(factory->createSampler(typed_config.typed_config(), context), nullptr); + EXPECT_STREQ(factory->name().c_str(), "envoy.tracers.opentelemetry.samplers.dynatrace"); +} + +} // namespace OpenTelemetry +} // namespace Tracers +} // namespace Extensions +} // namespace Envoy diff --git a/test/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_tracestate_test.cc b/test/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_tracestate_test.cc new file mode 100644 index 000000000000..73d42af36a26 --- /dev/null +++ b/test/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_tracestate_test.cc @@ -0,0 +1,21 @@ +#include + +#include "source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_tracestate.h" + +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +namespace Envoy { +namespace Extensions { +namespace Tracers { +namespace OpenTelemetry { + +TEST(DynatraceTracestateTest, TestKey) { + DynatraceTracestate tracestate("98812ad49", "980df25c"); + EXPECT_STREQ(tracestate.getKey().c_str(), "98812ad49-980df25c@dt"); +} + +} // namespace OpenTelemetry +} // namespace Tracers +} // namespace Extensions +} // namespace Envoy diff --git a/test/extensions/tracers/opentelemetry/samplers/dynatrace/tracestate_test.cc b/test/extensions/tracers/opentelemetry/samplers/dynatrace/tracestate_test.cc new file mode 100644 index 000000000000..f8f72542281d --- /dev/null +++ b/test/extensions/tracers/opentelemetry/samplers/dynatrace/tracestate_test.cc @@ -0,0 +1,132 @@ +#include + +#include "source/extensions/tracers/opentelemetry/samplers/dynatrace/tracestate.h" + +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +namespace Envoy { +namespace Extensions { +namespace Tracers { +namespace OpenTelemetry { + +// Test parsing an empty string +TEST(TracestateTest, TestEmpty) { + Tracestate tracestate; + tracestate.parse(""); + EXPECT_EQ(0, tracestate.entries().size()); + EXPECT_STREQ(tracestate.asString().c_str(), ""); +} + +// Test parsing a tracestate string with one entry +TEST(TracestateTest, TestSingleEntry) { + Tracestate tracestate; + tracestate.parse("key0=value0"); + auto entries = tracestate.entries(); + EXPECT_EQ(1, entries.size()); + EXPECT_STREQ(entries[0].key.c_str(), "key0"); + EXPECT_STREQ(entries[0].value.c_str(), "value0"); +} + +// Test parsing a tracestate string an invalid entry +TEST(TracestateTest, TestInvalidEntry) { + Tracestate tracestate; + tracestate.parse("key0=value0,key1="); + auto entries = tracestate.entries(); + EXPECT_EQ(1, entries.size()); + EXPECT_STREQ(entries[0].key.c_str(), "key0"); + EXPECT_STREQ(entries[0].value.c_str(), "value0"); + + tracestate.parse("key0=value0,=value1"); + entries = tracestate.entries(); + EXPECT_EQ(1, entries.size()); + EXPECT_STREQ(entries[0].key.c_str(), "key0"); + EXPECT_STREQ(entries[0].value.c_str(), "value0"); + + tracestate.parse("key0=value0,something"); + entries = tracestate.entries(); + EXPECT_EQ(1, entries.size()); + EXPECT_STREQ(entries[0].key.c_str(), "key0"); + EXPECT_STREQ(entries[0].value.c_str(), "value0"); +} + +// Test parsing a tracestate string with multiple entries +TEST(TracestateTest, TestMultiEntries) { + Tracestate tracestate; + tracestate.parse("key0=value0,key1=value1,key2=value2,key3=value3"); + auto entries = tracestate.entries(); + EXPECT_EQ(4, entries.size()); + + EXPECT_STREQ(entries[0].key.c_str(), "key0"); + EXPECT_STREQ(entries[0].value.c_str(), "value0"); + + EXPECT_STREQ(entries[1].key.c_str(), "key1"); + EXPECT_STREQ(entries[1].value.c_str(), "value1"); + + EXPECT_STREQ(entries[2].key.c_str(), "key2"); + EXPECT_STREQ(entries[2].value.c_str(), "value2"); + + EXPECT_STREQ(entries[3].key.c_str(), "key3"); + EXPECT_STREQ(entries[3].value.c_str(), "value3"); +} + +// Test parsing a tracestate string with optional white spaces +TEST(TracestateTest, TestWithOWS) { + Tracestate tracestate; + // whitespace before and after ',' should be removed + // whitespace inside value is allowed + const char* c = + "key0=value0,key1=value1, key2=val ue2 , key3=value3 ,key4=value4 , key5=value5"; + tracestate.parse(c); + EXPECT_STREQ(tracestate.asString().c_str(), c); + auto entries = tracestate.entries(); + EXPECT_EQ(6, entries.size()); + + EXPECT_STREQ(entries[0].key.c_str(), "key0"); + EXPECT_STREQ(entries[0].value.c_str(), "value0"); + + EXPECT_STREQ(entries[1].key.c_str(), "key1"); + EXPECT_STREQ(entries[1].value.c_str(), "value1"); + + EXPECT_STREQ(entries[2].key.c_str(), "key2"); + EXPECT_STREQ(entries[2].value.c_str(), "val ue2"); + + EXPECT_STREQ(entries[3].key.c_str(), "key3"); + EXPECT_STREQ(entries[3].value.c_str(), "value3"); + + EXPECT_STREQ(entries[4].key.c_str(), "key4"); + EXPECT_STREQ(entries[4].value.c_str(), "value4"); + + EXPECT_STREQ(entries[5].key.c_str(), "key5"); + EXPECT_STREQ(entries[5].value.c_str(), "value5"); +} + +// Test appending to an empty tracestate +TEST(TracestateTest, TestAppendToEmpty) { + Tracestate tracestate; + tracestate.add("new_key", "new_value"); + auto entries = tracestate.entries(); + EXPECT_EQ(1, entries.size()); + EXPECT_STREQ(entries[0].key.c_str(), "new_key"); + EXPECT_STREQ(entries[0].value.c_str(), "new_value"); + EXPECT_STREQ(tracestate.asString().c_str(), "new_key=new_value"); +} + +// Test appending to an existing tracestate +TEST(TracestateTest, TestAppend) { + Tracestate tracestate; + tracestate.parse("key1=value1,key2=value2,key3=value3"); + tracestate.add("new_key", "new_value"); + + auto entries = tracestate.entries(); + EXPECT_EQ(4, entries.size()); + EXPECT_STREQ(tracestate.asString().c_str(), + "new_key=new_value,key1=value1,key2=value2,key3=value3"); + EXPECT_STREQ(entries[0].key.c_str(), "new_key"); + EXPECT_STREQ(entries[0].value.c_str(), "new_value"); +} + +} // namespace OpenTelemetry +} // namespace Tracers +} // namespace Extensions +} // namespace Envoy From 40bff9bc8310a647c159a9051ffc265ded6c6d0e Mon Sep 17 00:00:00 2001 From: "thomas.ebner" Date: Thu, 19 Oct 2023 13:00:08 +0200 Subject: [PATCH 04/21] add Dynatrace sampler to metadata yaml Signed-off-by: thomas.ebner --- source/extensions/extensions_metadata.yaml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/source/extensions/extensions_metadata.yaml b/source/extensions/extensions_metadata.yaml index 8e7faf7f16c2..22100b5b5025 100644 --- a/source/extensions/extensions_metadata.yaml +++ b/source/extensions/extensions_metadata.yaml @@ -1155,6 +1155,13 @@ envoy.tracers.opentelemetry.samplers.always_on: status: wip type_urls: - envoy.extensions.tracers.opentelemetry.samplers.v3.AlwaysOnSamplerConfig +envoy.tracers.opentelemetry.samplers.dynatrace: + categories: + - envoy.tracers.opentelemetry.samplers + security_posture: unknown + status: wip + type_urls: + - envoy.extensions.tracers.opentelemetry.samplers.v3.DynatraceSamplerConfig envoy.tracers.skywalking: categories: - envoy.tracers From 298a5c58082733a94b89546325fd482698a48d6b Mon Sep 17 00:00:00 2001 From: "thomas.ebner" Date: Thu, 19 Oct 2023 13:25:25 +0200 Subject: [PATCH 05/21] cleanup fix attribute Signed-off-by: thomas.ebner --- .../samplers/dynatrace/dynatrace_sampler.cc | 15 +------ .../samplers/dynatrace/dynatrace_tracestate.h | 45 ++----------------- 2 files changed, 5 insertions(+), 55 deletions(-) diff --git a/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.cc b/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.cc index 0e1160c21684..43e9ddb20fc8 100644 --- a/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.cc +++ b/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.cc @@ -18,16 +18,6 @@ namespace OpenTelemetry { static const char* SAMPLING_EXTRAPOLATION_SPAN_ATTRIBUTE_NAME = "sampling_extrapolation_set_in_sampler"; -DynatraceTracestate DynatraceSampler::createTracestate(bool is_recording, - std::string sampling_exponent) { - DynatraceTracestate tracestate; - tracestate.sampling_exponent = sampling_exponent; - tracestate.tenant_id = tenant_id_; - tracestate.cluster_id = cluster_id_; - tracestate.is_ignored = is_recording ? "0" : "1"; - return tracestate; -} - FW4Tag DynatraceSampler::getFW4Tag(const Tracestate& tracestate) { for (auto const& entry : tracestate.entries()) { if (dynatrace_tracestate_.keyMatches( @@ -55,20 +45,19 @@ DynatraceSampler::shouldSample(const absl::optional parent_context, if (fw4_tag.isValid()) { // we found a trace decision in tracestate header result.decision = fw4_tag.isIgnored() ? Decision::DROP : Decision::RECORD_AND_SAMPLE; - att[SAMPLING_EXTRAPOLATION_SPAN_ATTRIBUTE_NAME] = fw4_tag.getSamplingExponent(); + att[SAMPLING_EXTRAPOLATION_SPAN_ATTRIBUTE_NAME] = std::to_string(fw4_tag.getSamplingExponent()); result.tracestate = parent_context->tracestate(); } else { // make a sampling decision bool sample = true; // TODO int sampling_exponent = 8; // TODO - att[SAMPLING_EXTRAPOLATION_SPAN_ATTRIBUTE_NAME] = sampling_exponent; + att[SAMPLING_EXTRAPOLATION_SPAN_ATTRIBUTE_NAME] = std::to_string(sampling_exponent); result.decision = sample ? Decision::RECORD_AND_SAMPLE : Decision::DROP; FW4Tag new_tag = FW4Tag::create(!sample, sampling_exponent); tracestate.add(dynatrace_tracestate_.getKey(), new_tag.createValue()); result.tracestate = tracestate.asString(); - att[SAMPLING_EXTRAPOLATION_SPAN_ATTRIBUTE_NAME] = sampling_exponent; } if (!att.empty()) { diff --git a/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_tracestate.h b/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_tracestate.h index e0f973f1652d..760e6d547bc4 100644 --- a/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_tracestate.h +++ b/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_tracestate.h @@ -38,9 +38,6 @@ class FW4Tag { return {true, ignored, sampling_exponent}; } - FW4Tag(bool valid, bool ignored, int sampling_exponent) - : valid_(valid), ignored_(ignored), sampling_exponent_(sampling_exponent) {} - bool isValid() const { return valid_; }; bool isIgnored() const { return ignored_; }; int getSamplingExponent() const { return sampling_exponent_; }; @@ -51,6 +48,9 @@ class FW4Tag { } private: + FW4Tag(bool valid, bool ignored, int sampling_exponent) + : valid_(valid), ignored_(ignored), sampling_exponent_(sampling_exponent) {} + bool valid_; bool ignored_; int sampling_exponent_; @@ -70,45 +70,6 @@ class DynatraceTracestate { bool keyMatches(const std::string& key) { return (key_.compare(key) == 0); } - bool isIgnored(const std::string& value) { - std::vector tracestate_components = - absl::StrSplit(value, ';', absl::AllowEmpty()); - if (tracestate_components.size() > 7) { - return tracestate_components[5] == "1"; - } - return false; - } - - std::string tenant_id = ""; - std::string cluster_id = ""; - static constexpr absl::string_view at_dt_format = "@dt=fw4"; - static constexpr absl::string_view server_id = "0"; - static constexpr absl::string_view agent_d = "0"; - static constexpr absl::string_view tag_id = "0"; - static constexpr absl::string_view link_id = "0"; - std::string is_ignored = "0"; - std::string sampling_exponent = "0"; - std::string path_info = "0"; - std::string span_id = ""; - - std::string toString() { - std::string tracestate = absl::StrCat( - absl::string_view(tenant_id), "-", absl::string_view(cluster_id), at_dt_format, ";", - server_id, ";", agent_d, ";", tag_id, ";", link_id, ";", absl::string_view(is_ignored), ";", - absl::string_view(sampling_exponent), ";", absl::string_view(path_info)); - - // Which is: "abcdabcd-77@dt=fw4;8;66666666;111;99;0;0;66;;8cef;2h01;7h293e72b548735604" - if (!span_id.empty()) { - std::string tmp(tracestate); - // https://oaad.lab.dynatrace.org/agent/concepts/purepath/tagging/#forward-tags-fw - std::string extension = ";7h" + span_id; - std::string hash_as_hex = ""; - tracestate = absl::StrCat(absl::string_view(tmp), ";", absl::string_view(hash_as_hex), - absl::string_view(extension)); - } - return tracestate; - } - private: std::string tenant_id_; std::string cluster_id_; From 8b219014d2fb066ab4b6d3bae0bbb2f1968ef928 Mon Sep 17 00:00:00 2001 From: "thomas.ebner" Date: Thu, 19 Oct 2023 14:49:28 +0200 Subject: [PATCH 06/21] another cleanup Signed-off-by: thomas.ebner --- .../samplers/dynatrace/dynatrace_tracestate.h | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_tracestate.h b/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_tracestate.h index 760e6d547bc4..de964ccf980d 100644 --- a/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_tracestate.h +++ b/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_tracestate.h @@ -59,11 +59,8 @@ class FW4Tag { // -@dt=fw4;0;0;0;0;;; class DynatraceTracestate { public: - DynatraceTracestate() = default; - - DynatraceTracestate(const std::string& tenant_id, const std::string& cluster_id) - : tenant_id_(tenant_id), cluster_id_(cluster_id) { - key_ = absl::StrCat(absl::string_view(tenant_id_), "-", absl::string_view(cluster_id), "@dt"); + DynatraceTracestate(const std::string& tenant_id, const std::string& cluster_id) { + key_ = absl::StrCat(absl::string_view(tenant_id), "-", absl::string_view(cluster_id), "@dt"); } std::string getKey() const { return key_; }; @@ -71,8 +68,6 @@ class DynatraceTracestate { bool keyMatches(const std::string& key) { return (key_.compare(key) == 0); } private: - std::string tenant_id_; - std::string cluster_id_; std::string key_; }; } // namespace OpenTelemetry From 3ec14214dd44431160bb4c124921db364c490208 Mon Sep 17 00:00:00 2001 From: "thomas.ebner" Date: Wed, 25 Oct 2023 12:35:59 +0200 Subject: [PATCH 07/21] add integration test for dynatrace sampler Signed-off-by: thomas.ebner --- .../opentelemetry/samplers/dynatrace/BUILD | 14 ++ .../dynatrace_sampler_integration_test.cc | 144 ++++++++++++++++++ 2 files changed, 158 insertions(+) create mode 100644 test/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler_integration_test.cc diff --git a/test/extensions/tracers/opentelemetry/samplers/dynatrace/BUILD b/test/extensions/tracers/opentelemetry/samplers/dynatrace/BUILD index 7b52709e93de..cca7bc83839b 100644 --- a/test/extensions/tracers/opentelemetry/samplers/dynatrace/BUILD +++ b/test/extensions/tracers/opentelemetry/samplers/dynatrace/BUILD @@ -37,3 +37,17 @@ envoy_extension_cc_test( "//test/test_common:utility_lib", ], ) + +envoy_extension_cc_test( + name = "dynatrace_sampler_integration_test", + srcs = [ + "dynatrace_sampler_integration_test.cc", + ], + extension_names = ["envoy.tracers.opentelemetry.samplers.dynatrace"], + deps = [ + "//source/exe:main_common_lib", + "//test/integration:http_integration_lib", + "//test/test_common:utility_lib", + "@envoy_api//envoy/extensions/filters/network/http_connection_manager/v3:pkg_cc_proto", + ], +) 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 new file mode 100644 index 000000000000..f83c67a9720c --- /dev/null +++ b/test/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler_integration_test.cc @@ -0,0 +1,144 @@ +#include +#include + +#include "envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.pb.h" + +#include "test/integration/http_integration.h" +#include "test/test_common/utility.h" + +#include "absl/strings/match.h" +#include "absl/strings/string_view.h" +#include "gtest/gtest.h" + +namespace Envoy { +namespace Extensions { +namespace Tracers { +namespace OpenTelemetry { +namespace { + +const char* TRACEPARENT_VALUE = "00-0af7651916cd43dd8448eb211c80319c-b7ad6b7169203331-01"; +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( + 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_id: "9712ad40" + cluster_id: "980df25c" + )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) { + Http::TestRequestHeaderMapImpl request_headers{ + {":method", "GET"}, {":path", "/test/long/url"}, {":scheme", "http"}, + {":authority", "host"}, {"tracestate", "key=value"}, {"traceparent", TRACEPARENT_VALUE}}; + + 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 should be set: traceid should be re-used, span id should be different + absl::string_view traceparent_value = upstream_request_->headers() + .get(Http::LowerCaseString("traceparent"))[0] + ->value() + .getStringView(); + EXPECT_TRUE(absl::StartsWith(traceparent_value, TRACEPARENT_VALUE_START)); + EXPECT_NE(TRACEPARENT_VALUE, traceparent_value); + // tracestate should be forwarded + absl::string_view tracestate_value = upstream_request_->headers() + .get(Http::LowerCaseString("tracestate"))[0] + ->value() + .getStringView(); + EXPECT_EQ("9712ad40-980df25c@dt=fw4;0;0;0;0;0;8,key=value", tracestate_value); +} + +// Sends a request with traceparent but no tracestate header. +TEST_P(DynatraceSamplerIntegrationTest, TestWithTraceparentOnly) { + Http::TestRequestHeaderMapImpl request_headers{{":method", "GET"}, + {":path", "/test/long/url"}, + {":scheme", "http"}, + {":authority", "host"}, + {"traceparent", TRACEPARENT_VALUE}}; + 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 should be set: traceid should be re-used, span id should be different + absl::string_view traceparent_value = upstream_request_->headers() + .get(Http::LowerCaseString("traceparent"))[0] + ->value() + .getStringView(); + EXPECT_TRUE(absl::StartsWith(traceparent_value, TRACEPARENT_VALUE_START)); + EXPECT_NE(TRACEPARENT_VALUE, traceparent_value); + // OTLP tracer adds an empty tracestate + absl::string_view tracestate_value = upstream_request_->headers() + .get(Http::LowerCaseString("tracestate"))[0] + ->value() + .getStringView(); + EXPECT_EQ("9712ad40-980df25c@dt=fw4;0;0;0;0;0;8", tracestate_value); +} + +// Sends a request without traceparent and tracestate header. +TEST_P(DynatraceSamplerIntegrationTest, TestWithoutTraceparentAndTracestate) { + 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); + // OTLP tracer adds an empty tracestate + absl::string_view tracestate_value = upstream_request_->headers() + .get(Http::LowerCaseString("tracestate"))[0] + ->value() + .getStringView(); + EXPECT_EQ("9712ad40-980df25c@dt=fw4;0;0;0;0;0;8", tracestate_value); +} + +} // namespace +} // namespace OpenTelemetry +} // namespace Tracers +} // namespace Extensions +} // namespace Envoy From 13afa60744c1ea44f62c3af9690226ac8b1dee76 Mon Sep 17 00:00:00 2001 From: "thomas.ebner" Date: Wed, 25 Oct 2023 14:01:25 +0200 Subject: [PATCH 08/21] add dynatrace_sampler_test Signed-off-by: thomas.ebner --- .../samplers/dynatrace/config.cc | 3 +- .../samplers/dynatrace/dynatrace_sampler.h | 3 +- .../opentelemetry/samplers/dynatrace/BUILD | 2 + .../dynatrace/dynatrace_sampler_test.cc | 55 +++++++++++++++++++ 4 files changed, 61 insertions(+), 2 deletions(-) create mode 100644 test/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler_test.cc diff --git a/source/extensions/tracers/opentelemetry/samplers/dynatrace/config.cc b/source/extensions/tracers/opentelemetry/samplers/dynatrace/config.cc index 22ace4bd3cfe..03b6352238b9 100644 --- a/source/extensions/tracers/opentelemetry/samplers/dynatrace/config.cc +++ b/source/extensions/tracers/opentelemetry/samplers/dynatrace/config.cc @@ -20,7 +20,8 @@ DynatraceSamplerFactory::createSampler(const Protobuf::Message& config, return std::make_shared( MessageUtil::downcastAndValidate< const envoy::extensions::tracers::opentelemetry::samplers::v3::DynatraceSamplerConfig&>( - *mptr, context.messageValidationVisitor())); + *mptr, context.messageValidationVisitor()), + context); } /** diff --git a/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.h b/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.h index 5f554ce2a4ed..5a02420053dc 100644 --- a/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.h +++ b/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.h @@ -20,7 +20,8 @@ namespace OpenTelemetry { class DynatraceSampler : public Sampler, Logger::Loggable { public: explicit DynatraceSampler( - const envoy::extensions::tracers::opentelemetry::samplers::v3::DynatraceSamplerConfig config) + const envoy::extensions::tracers::opentelemetry::samplers::v3::DynatraceSamplerConfig config, + Server::Configuration::TracerFactoryContext& /*context*/) : tenant_id_(config.tenant_id()), cluster_id_(config.cluster_id()), dynatrace_tracestate_(tenant_id_, cluster_id_) {} diff --git a/test/extensions/tracers/opentelemetry/samplers/dynatrace/BUILD b/test/extensions/tracers/opentelemetry/samplers/dynatrace/BUILD index cca7bc83839b..e1d099cfc05d 100644 --- a/test/extensions/tracers/opentelemetry/samplers/dynatrace/BUILD +++ b/test/extensions/tracers/opentelemetry/samplers/dynatrace/BUILD @@ -27,6 +27,7 @@ envoy_extension_cc_test( envoy_extension_cc_test( name = "dynatrace_sampler_test", srcs = [ + "dynatrace_sampler_test.cc", "dynatrace_tracestate_test.cc", "tracestate_test.cc", ], @@ -35,6 +36,7 @@ envoy_extension_cc_test( "//source/extensions/tracers/opentelemetry/samplers/dynatrace:dynatrace_sampler_lib", "//test/mocks/server:tracer_factory_context_mocks", "//test/test_common:utility_lib", + "@envoy_api//envoy/extensions/tracers/opentelemetry/samplers/v3:pkg_cc_proto", ], ) diff --git a/test/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler_test.cc b/test/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler_test.cc new file mode 100644 index 000000000000..4d88caec3e34 --- /dev/null +++ b/test/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler_test.cc @@ -0,0 +1,55 @@ +#include +#include + +#include "envoy/extensions/tracers/opentelemetry/samplers/v3/dynatrace_sampler.pb.h" + +#include "source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.h" +#include "source/extensions/tracers/opentelemetry/span_context.h" + +#include "test/mocks/server/tracer_factory_context.h" + +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +namespace Envoy { +namespace Extensions { +namespace Tracers { +namespace OpenTelemetry { + +class DynatraceSamplerTest : public testing::Test { + + const std::string yaml_string = R"EOF( + tenant_id: "9712ad40" + cluster_id: "980df25c" + )EOF"; + +public: + DynatraceSamplerTest() { + TestUtility::loadFromYaml(yaml_string, config_); + NiceMock context; + sampler_ = std::make_shared(config_, context); + EXPECT_STREQ(sampler_->getDescription().c_str(), "DynatraceSampler"); + } + +protected: + envoy::extensions::tracers::opentelemetry::samplers::v3::DynatraceSamplerConfig config_; + std::shared_ptr sampler_; +}; + +// Verify sampler being invoked with an invalid span context +TEST_F(DynatraceSamplerTest, TestWithoutParentContext) { + + auto sampling_result = + sampler_->shouldSample(absl::nullopt, "operation_name", "12345", + ::opentelemetry::proto::trace::v1::Span::SPAN_KIND_SERVER, {}, {}); + EXPECT_EQ(sampling_result.decision, Decision::RECORD_AND_SAMPLE); + EXPECT_EQ(sampling_result.attributes->size(), 1); + EXPECT_STREQ(sampling_result.tracestate.c_str(), "9712ad40-980df25c@dt=fw4;0;0;0;0;0;8"); + EXPECT_TRUE(sampling_result.isRecording()); + EXPECT_TRUE(sampling_result.isSampled()); +} + +} // namespace OpenTelemetry +} // namespace Tracers +} // namespace Extensions +} // namespace Envoy From 8085c36c44f1dae7e8a793a6b540121ca63a74a7 Mon Sep 17 00:00:00 2001 From: "thomas.ebner" Date: Fri, 3 Nov 2023 13:15:06 +0100 Subject: [PATCH 09/21] use 0 as hardcoded exponent Signed-off-by: thomas.ebner --- .../opentelemetry/samplers/dynatrace/dynatrace_sampler.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.cc b/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.cc index 43e9ddb20fc8..8ab4fd9b6f41 100644 --- a/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.cc +++ b/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.cc @@ -50,7 +50,7 @@ DynatraceSampler::shouldSample(const absl::optional parent_context, } else { // make a sampling decision bool sample = true; // TODO - int sampling_exponent = 8; // TODO + int sampling_exponent = 0; // TODO att[SAMPLING_EXTRAPOLATION_SPAN_ATTRIBUTE_NAME] = std::to_string(sampling_exponent); result.decision = sample ? Decision::RECORD_AND_SAMPLE : Decision::DROP; From 2ba0b6a5be65b2e917b8f59083ef7faaa77e5f8c Mon Sep 17 00:00:00 2001 From: "thomas.ebner" Date: Tue, 7 Nov 2023 15:13:43 +0100 Subject: [PATCH 10/21] first approach to get inital attributes Signed-off-by: thomas.ebner --- .../opentelemetry_tracer_impl.cc | 32 +++++++++++++++++-- .../samplers/dynatrace/dynatrace_sampler.cc | 6 ++-- .../tracers/opentelemetry/samplers/sampler.h | 9 ++++-- .../tracers/opentelemetry/tracer.cc | 32 +++++++++++++------ .../extensions/tracers/opentelemetry/tracer.h | 8 ++--- 5 files changed, 66 insertions(+), 21 deletions(-) diff --git a/source/extensions/tracers/opentelemetry/opentelemetry_tracer_impl.cc b/source/extensions/tracers/opentelemetry/opentelemetry_tracer_impl.cc index 8d5786b8c232..c2807b5654e2 100644 --- a/source/extensions/tracers/opentelemetry/opentelemetry_tracer_impl.cc +++ b/source/extensions/tracers/opentelemetry/opentelemetry_tracer_impl.cc @@ -7,6 +7,7 @@ #include "source/common/common/empty_string.h" #include "source/common/common/logger.h" #include "source/common/config/utility.h" +#include "source/common/http/utility.h" #include "source/common/tracing/http_tracer_impl.h" #include "source/extensions/tracers/opentelemetry/grpc_trace_exporter.h" #include "source/extensions/tracers/opentelemetry/http_trace_exporter.h" @@ -97,18 +98,43 @@ Tracing::SpanPtr Driver::startSpan(const Tracing::Config& config, const std::string& operation_name, Tracing::Decision tracing_decision) { // Get tracer from TLS and start span. + auto protocol = trace_context.protocol(); + auto host = trace_context.host(); + auto path = trace_context.path(); + auto method = trace_context.method(); + ENVOY_LOG(info, "host: '{}' path: '{}' method: '{}' protocol: '{}'", std::string{host}, + std::string{path}, std::string{method}, std::string{protocol}); + + trace_context.forEach([](absl::string_view key, absl::string_view value) { + ENVOY_LOG(info, "headers: '{}' '{}'", std::string{key}, std::string{value}); + return true; + }); + + auto request_header_map = dynamic_cast(&trace_context); + ASSERT(request_header_map != nullptr); + auto utl = Http::Utility::buildOriginalUri(*request_header_map, config.maxPathTagLength()); + ENVOY_LOG(info, "original uri: {}", utl); + + // https://opentelemetry.io/docs/specs/semconv/http/http-spans/ + OTelSpanAttributes initial_attributes; + initial_attributes["http.X.Y"] = utl; + initial_attributes["http.request.method"] = trace_context.method(); + initial_attributes["url.path"] = + trace_context.path(); // TODO: trace_context.path() contains also "query", should be splitted + // by `?` character + auto& tracer = tls_slot_ptr_->getTyped().tracer(); SpanContextExtractor extractor(trace_context); if (!extractor.propagationHeaderPresent()) { // No propagation header, so we can create a fresh span with the given decision. - Tracing::SpanPtr new_open_telemetry_span = - tracer.startSpan(config, operation_name, stream_info.startTime(), tracing_decision); + Tracing::SpanPtr new_open_telemetry_span = tracer.startSpan( + config, operation_name, stream_info.startTime(), initial_attributes, tracing_decision); return new_open_telemetry_span; } else { // Try to extract the span context. If we can't, just return a null span. absl::StatusOr span_context = extractor.extractSpanContext(); if (span_context.ok()) { - return tracer.startSpan(config, operation_name, stream_info.startTime(), + return tracer.startSpan(config, operation_name, stream_info.startTime(), initial_attributes, span_context.value()); } else { ENVOY_LOG(trace, "Unable to extract span context: ", span_context.status()); diff --git a/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.cc b/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.cc index 8ab4fd9b6f41..412da041e525 100644 --- a/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.cc +++ b/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.cc @@ -32,12 +32,14 @@ SamplingResult DynatraceSampler::shouldSample(const absl::optional parent_context, const std::string& /*trace_id*/, const std::string& /*name*/, ::opentelemetry::proto::trace::v1::Span::SpanKind /*kind*/, - const std::map& /*attributes*/, + const std::map& attributes, const std::vector& /*links*/) { SamplingResult result; std::map att; - + if (attributes.size() > 10) { + return {}; + } Tracestate tracestate; tracestate.parse(parent_context.has_value() ? parent_context->tracestate() : ""); diff --git a/source/extensions/tracers/opentelemetry/samplers/sampler.h b/source/extensions/tracers/opentelemetry/samplers/sampler.h index fd2be0bb647e..cef0925a2fcb 100644 --- a/source/extensions/tracers/opentelemetry/samplers/sampler.h +++ b/source/extensions/tracers/opentelemetry/samplers/sampler.h @@ -29,11 +29,16 @@ enum class Decision { RECORD_AND_SAMPLE }; +/** + * @brief A string key-value map that stores the span attributes. + */ +using OTelSpanAttributes = std::map; + struct SamplingResult { /// @see Decision Decision decision; // A set of span Attributes that will also be added to the Span. Can be nullptr. - std::unique_ptr> attributes; + std::unique_ptr attributes; // A Tracestate that will be associated with the Span. If the sampler // returns an empty Tracestate here, the Tracestate will be cleared, so samplers SHOULD normally // return the passed-in Tracestate if they do not intend to change it @@ -71,7 +76,7 @@ class Sampler { virtual SamplingResult shouldSample(const absl::optional parent_context, const std::string& trace_id, const std::string& name, ::opentelemetry::proto::trace::v1::Span::SpanKind spankind, - const std::map& attributes, + const OTelSpanAttributes& attributes, const std::vector& links) PURE; /** diff --git a/source/extensions/tracers/opentelemetry/tracer.cc b/source/extensions/tracers/opentelemetry/tracer.cc index 73325ac7f317..a62590fb798f 100644 --- a/source/extensions/tracers/opentelemetry/tracer.cc +++ b/source/extensions/tracers/opentelemetry/tracer.cc @@ -25,12 +25,14 @@ using opentelemetry::proto::collector::trace::v1::ExportTraceServiceRequest; namespace { void callSampler(SamplerSharedPtr sampler, const absl::optional span_context, - Span& new_span, const std::string& operation_name) { + Span& new_span, const std::string& operation_name, + const OTelSpanAttributes& initial_attributes) { if (!sampler) { return; } - const auto sampling_result = sampler->shouldSample( - span_context, operation_name, new_span.getTraceIdAsHex(), new_span.spankind(), {}, {}); + const auto sampling_result = + sampler->shouldSample(span_context, operation_name, new_span.getTraceIdAsHex(), + new_span.spankind(), initial_attributes, {}); new_span.setSampled(sampling_result.isSampled()); if (sampling_result.attributes) { @@ -43,6 +45,12 @@ void callSampler(SamplerSharedPtr sampler, const absl::optional spa } } +void setInitialAttributes(Span& new_span, const OTelSpanAttributes& initial_attributes) { + for (auto const& attribute : initial_attributes) { + new_span.setTag(attribute.first, attribute.second); + } +} + } // namespace Span::Span(const Tracing::Config& config, const std::string& name, SystemTime start_time, @@ -77,7 +85,7 @@ Tracing::SpanPtr Span::spawnChild(const Tracing::Config& config, const std::stri SystemTime start_time) { // Build span_context from the current span, then generate the child span from that context. SpanContext span_context(kDefaultVersion, getTraceIdAsHex(), spanId(), sampled(), tracestate()); - return parent_tracer_.startSpan(config, name, start_time, span_context, + return parent_tracer_.startSpan(config, name, start_time, {}, span_context, /*downstream_span*/ false); } @@ -192,8 +200,9 @@ void Tracer::sendSpan(::opentelemetry::proto::trace::v1::Span& span) { } Tracing::SpanPtr Tracer::startSpan(const Tracing::Config& config, const std::string& operation_name, - SystemTime start_time, Tracing::Decision tracing_decision, - bool downstream_span) { + SystemTime start_time, + const OTelSpanAttributes& initial_attributes, + Tracing::Decision tracing_decision, bool downstream_span) { // Create an Tracers::OpenTelemetry::Span class that will contain the OTel span. Span new_span(config, operation_name, start_time, time_source_, *this, downstream_span); uint64_t trace_id_high = random_.random(); @@ -201,8 +210,9 @@ Tracing::SpanPtr Tracer::startSpan(const Tracing::Config& config, const std::str new_span.setTraceId(absl::StrCat(Hex::uint64ToHex(trace_id_high), Hex::uint64ToHex(trace_id))); uint64_t span_id = random_.random(); new_span.setId(Hex::uint64ToHex(span_id)); + setInitialAttributes(new_span, initial_attributes); if (sampler_) { - callSampler(sampler_, absl::nullopt, new_span, operation_name); + callSampler(sampler_, absl::nullopt, new_span, operation_name, initial_attributes); } else { new_span.setSampled(tracing_decision.traced); } @@ -210,8 +220,9 @@ Tracing::SpanPtr Tracer::startSpan(const Tracing::Config& config, const std::str } Tracing::SpanPtr Tracer::startSpan(const Tracing::Config& config, const std::string& operation_name, - SystemTime start_time, const SpanContext& previous_span_context, - bool downstream_span) { + SystemTime start_time, + const OTelSpanAttributes& initial_attributes, + const SpanContext& previous_span_context, bool downstream_span) { // Create a new span and populate details from the span context. Span new_span(config, operation_name, start_time, time_source_, *this, downstream_span); new_span.setTraceId(previous_span_context.traceId()); @@ -221,9 +232,10 @@ Tracing::SpanPtr Tracer::startSpan(const Tracing::Config& config, const std::str // Generate a new identifier for the span id. uint64_t span_id = random_.random(); new_span.setId(Hex::uint64ToHex(span_id)); + setInitialAttributes(new_span, initial_attributes); if (sampler_) { // Sampler should make a sampling decision and set tracestate - callSampler(sampler_, previous_span_context, new_span, operation_name); + callSampler(sampler_, previous_span_context, new_span, operation_name, initial_attributes); } else { // Respect the previous span's sampled flag. new_span.setSampled(previous_span_context.sampled()); diff --git a/source/extensions/tracers/opentelemetry/tracer.h b/source/extensions/tracers/opentelemetry/tracer.h index bea45d54f4cc..ba1fedd4d36a 100644 --- a/source/extensions/tracers/opentelemetry/tracer.h +++ b/source/extensions/tracers/opentelemetry/tracer.h @@ -43,12 +43,12 @@ class Tracer : Logger::Loggable { void sendSpan(::opentelemetry::proto::trace::v1::Span& span); Tracing::SpanPtr startSpan(const Tracing::Config& config, const std::string& operation_name, - SystemTime start_time, Tracing::Decision tracing_decision, - bool downstream_span = true); + SystemTime start_time, const OTelSpanAttributes& initial_attributes, + Tracing::Decision tracing_decision, bool downstream_span = true); Tracing::SpanPtr startSpan(const Tracing::Config& config, const std::string& operation_name, - SystemTime start_time, const SpanContext& previous_span_context, - bool downstream_span = true); + SystemTime start_time, const OTelSpanAttributes& initial_attributes, + const SpanContext& previous_span_context, bool downstream_span = true); private: /** From e873324f51d434c350d6c89edf986c10c037dd87 Mon Sep 17 00:00:00 2001 From: "thomas.ebner" Date: Tue, 7 Nov 2023 16:14:58 +0100 Subject: [PATCH 11/21] get initial attributes. Signed-off-by: thomas.ebner --- .../opentelemetry_tracer_impl.cc | 41 ++++++++----------- 1 file changed, 16 insertions(+), 25 deletions(-) diff --git a/source/extensions/tracers/opentelemetry/opentelemetry_tracer_impl.cc b/source/extensions/tracers/opentelemetry/opentelemetry_tracer_impl.cc index c2807b5654e2..07c7851fa69e 100644 --- a/source/extensions/tracers/opentelemetry/opentelemetry_tracer_impl.cc +++ b/source/extensions/tracers/opentelemetry/opentelemetry_tracer_impl.cc @@ -44,6 +44,20 @@ tryCreateSamper(const envoy::config::trace::v3::OpenTelemetryConfig& opentelemet return sampler; } +OTelSpanAttributes getInitialAttributes(const Tracing::TraceContext& trace_context) { + OTelSpanAttributes attributes; + std::vector address_port = absl::StrSplit(trace_context.host(), ":"); + attributes["server.address"] = address_port.size() > 0 ? address_port[0] : ""; + attributes["server.port"] = address_port.size() > 1 ? address_port[1] : ""; + std::vector path_query = absl::StrSplit(trace_context.path(), "?"); + attributes["url.path"] = path_query.size() > 0 ? path_query[0] : ""; + attributes["url.query"] = path_query.size() > 1 ? path_query[1] : ""; + auto scheme = trace_context.getByKey(":scheme"); + attributes["url.scheme"] = scheme.has_value() ? scheme.value() : ""; + attributes["http.request.method"] = trace_context.method(); + return attributes; +} + } // namespace Driver::Driver(const envoy::config::trace::v3::OpenTelemetryConfig& opentelemetry_config, @@ -97,34 +111,11 @@ Tracing::SpanPtr Driver::startSpan(const Tracing::Config& config, const StreamInfo::StreamInfo& stream_info, const std::string& operation_name, Tracing::Decision tracing_decision) { - // Get tracer from TLS and start span. - auto protocol = trace_context.protocol(); - auto host = trace_context.host(); - auto path = trace_context.path(); - auto method = trace_context.method(); - ENVOY_LOG(info, "host: '{}' path: '{}' method: '{}' protocol: '{}'", std::string{host}, - std::string{path}, std::string{method}, std::string{protocol}); - - trace_context.forEach([](absl::string_view key, absl::string_view value) { - ENVOY_LOG(info, "headers: '{}' '{}'", std::string{key}, std::string{value}); - return true; - }); - - auto request_header_map = dynamic_cast(&trace_context); - ASSERT(request_header_map != nullptr); - auto utl = Http::Utility::buildOriginalUri(*request_header_map, config.maxPathTagLength()); - ENVOY_LOG(info, "original uri: {}", utl); - - // https://opentelemetry.io/docs/specs/semconv/http/http-spans/ - OTelSpanAttributes initial_attributes; - initial_attributes["http.X.Y"] = utl; - initial_attributes["http.request.method"] = trace_context.method(); - initial_attributes["url.path"] = - trace_context.path(); // TODO: trace_context.path() contains also "query", should be splitted - // by `?` character + // Get tracer from TLS and start span. auto& tracer = tls_slot_ptr_->getTyped().tracer(); SpanContextExtractor extractor(trace_context); + auto initial_attributes = getInitialAttributes(trace_context); if (!extractor.propagationHeaderPresent()) { // No propagation header, so we can create a fresh span with the given decision. Tracing::SpanPtr new_open_telemetry_span = tracer.startSpan( From 0c9bf5fca3bab1f429474be29546fa6fd80b640f Mon Sep 17 00:00:00 2001 From: "thomas.ebner" Date: Wed, 8 Nov 2023 08:12:54 +0100 Subject: [PATCH 12/21] move span kind detection to startSpan(), adapt tests Signed-off-by: thomas.ebner --- .../opentelemetry_tracer_impl.cc | 52 ++++++++++++----- .../samplers/always_on/always_on_sampler.cc | 2 +- .../samplers/always_on/always_on_sampler.h | 2 +- .../samplers/dynatrace/dynatrace_sampler.cc | 11 ++-- .../samplers/dynatrace/dynatrace_sampler.h | 2 +- .../tracers/opentelemetry/samplers/sampler.h | 8 ++- .../tracers/opentelemetry/tracer.cc | 44 +++++---------- .../extensions/tracers/opentelemetry/tracer.h | 18 +++--- .../opentelemetry_tracer_impl_test.cc | 56 +++++++++++++++++++ .../dynatrace_sampler_integration_test.cc | 6 +- .../dynatrace/dynatrace_sampler_test.cc | 2 +- .../opentelemetry/samplers/sampler_test.cc | 12 ++-- 12 files changed, 140 insertions(+), 75 deletions(-) diff --git a/source/extensions/tracers/opentelemetry/opentelemetry_tracer_impl.cc b/source/extensions/tracers/opentelemetry/opentelemetry_tracer_impl.cc index 07c7851fa69e..49edb2af86d1 100644 --- a/source/extensions/tracers/opentelemetry/opentelemetry_tracer_impl.cc +++ b/source/extensions/tracers/opentelemetry/opentelemetry_tracer_impl.cc @@ -44,20 +44,43 @@ tryCreateSamper(const envoy::config::trace::v3::OpenTelemetryConfig& opentelemet return sampler; } -OTelSpanAttributes getInitialAttributes(const Tracing::TraceContext& trace_context) { +OTelSpanAttributes getInitialAttributes(const Tracing::TraceContext& trace_context, + OtelSpanKind span_kind) { OTelSpanAttributes attributes; - std::vector address_port = absl::StrSplit(trace_context.host(), ":"); - attributes["server.address"] = address_port.size() > 0 ? address_port[0] : ""; - attributes["server.port"] = address_port.size() > 1 ? address_port[1] : ""; - std::vector path_query = absl::StrSplit(trace_context.path(), "?"); - attributes["url.path"] = path_query.size() > 0 ? path_query[0] : ""; - attributes["url.query"] = path_query.size() > 1 ? path_query[1] : ""; - auto scheme = trace_context.getByKey(":scheme"); - attributes["url.scheme"] = scheme.has_value() ? scheme.value() : ""; - attributes["http.request.method"] = trace_context.method(); + + // required attributes for a server span are defined in + // https://opentelemetry.io/docs/specs/semconv/http/http-spans/#http-server-semantic-conventions + if (span_kind == ::opentelemetry::proto::trace::v1::Span::SPAN_KIND_SERVER) { + std::vector address_port = absl::StrSplit(trace_context.host(), ":"); + attributes["server.address"] = address_port.size() > 0 ? address_port[0] : ""; + attributes["server.port"] = address_port.size() > 1 ? address_port[1] : ""; + std::vector path_query = absl::StrSplit(trace_context.path(), "?"); + attributes["url.path"] = path_query.size() > 0 ? path_query[0] : ""; + attributes["url.query"] = path_query.size() > 1 ? path_query[1] : ""; + auto scheme = trace_context.getByKey(":scheme"); + attributes["url.scheme"] = scheme.has_value() ? scheme.value() : ""; + attributes["http.request.method"] = trace_context.method(); + } return attributes; } +OtelSpanKind getSpanKind(const Tracing::Config& config) { + OtelSpanKind span_kind; + // If this is downstream span that be created by 'startSpan' for downstream request, then + // set the span type based on the spawnUpstreamSpan flag and traffic direction: + // * If separate tracing span will be created for upstream request, then set span type to + // SERVER because the downstream span should be server span in trace chain. + // * If separate tracing span will not be created for upstream request, that means the + // Envoy will not be treated as independent hop in trace chain and then set span type + // based on the traffic direction. + span_kind = + (config.spawnUpstreamSpan() ? ::opentelemetry::proto::trace::v1::Span::SPAN_KIND_SERVER + : config.operationName() == Tracing::OperationName::Egress + ? ::opentelemetry::proto::trace::v1::Span::SPAN_KIND_CLIENT + : ::opentelemetry::proto::trace::v1::Span::SPAN_KIND_SERVER); + return span_kind; +} + } // namespace Driver::Driver(const envoy::config::trace::v3::OpenTelemetryConfig& opentelemetry_config, @@ -115,18 +138,19 @@ Tracing::SpanPtr Driver::startSpan(const Tracing::Config& config, // Get tracer from TLS and start span. auto& tracer = tls_slot_ptr_->getTyped().tracer(); SpanContextExtractor extractor(trace_context); - auto initial_attributes = getInitialAttributes(trace_context); + auto span_kind = getSpanKind(config); + auto initial_attributes = getInitialAttributes(trace_context, span_kind); if (!extractor.propagationHeaderPresent()) { // No propagation header, so we can create a fresh span with the given decision. Tracing::SpanPtr new_open_telemetry_span = tracer.startSpan( - config, operation_name, stream_info.startTime(), initial_attributes, tracing_decision); + operation_name, stream_info.startTime(), initial_attributes, tracing_decision, span_kind); return new_open_telemetry_span; } else { // Try to extract the span context. If we can't, just return a null span. absl::StatusOr span_context = extractor.extractSpanContext(); if (span_context.ok()) { - return tracer.startSpan(config, operation_name, stream_info.startTime(), initial_attributes, - span_context.value()); + return tracer.startSpan(operation_name, stream_info.startTime(), initial_attributes, + span_context.value(), span_kind); } else { ENVOY_LOG(trace, "Unable to extract span context: ", span_context.status()); return std::make_unique(); diff --git a/source/extensions/tracers/opentelemetry/samplers/always_on/always_on_sampler.cc b/source/extensions/tracers/opentelemetry/samplers/always_on/always_on_sampler.cc index 3bc0aa87ab3d..616f7f899d48 100644 --- a/source/extensions/tracers/opentelemetry/samplers/always_on/always_on_sampler.cc +++ b/source/extensions/tracers/opentelemetry/samplers/always_on/always_on_sampler.cc @@ -15,7 +15,7 @@ namespace OpenTelemetry { SamplingResult AlwaysOnSampler::shouldSample(const absl::optional parent_context, const std::string& /*trace_id*/, const std::string& /*name*/, - ::opentelemetry::proto::trace::v1::Span::SpanKind /*kind*/, + OtelSpanKind /*kind*/, const std::map& /*attributes*/, const std::vector& /*links*/) { SamplingResult result; diff --git a/source/extensions/tracers/opentelemetry/samplers/always_on/always_on_sampler.h b/source/extensions/tracers/opentelemetry/samplers/always_on/always_on_sampler.h index 2d53a511fa29..64d323fd59ce 100644 --- a/source/extensions/tracers/opentelemetry/samplers/always_on/always_on_sampler.h +++ b/source/extensions/tracers/opentelemetry/samplers/always_on/always_on_sampler.h @@ -24,7 +24,7 @@ class AlwaysOnSampler : public Sampler, Logger::Loggable { Server::Configuration::TracerFactoryContext& /*context*/) {} SamplingResult shouldSample(const absl::optional parent_context, const std::string& trace_id, const std::string& name, - ::opentelemetry::proto::trace::v1::Span::SpanKind spankind, + OtelSpanKind spankind, const std::map& attributes, const std::vector& links) override; std::string getDescription() const override; diff --git a/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.cc b/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.cc index 412da041e525..48bef3a30b66 100644 --- a/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.cc +++ b/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.cc @@ -28,12 +28,11 @@ FW4Tag DynatraceSampler::getFW4Tag(const Tracestate& tracestate) { return FW4Tag::createInvalid(); } -SamplingResult -DynatraceSampler::shouldSample(const absl::optional parent_context, - const std::string& /*trace_id*/, const std::string& /*name*/, - ::opentelemetry::proto::trace::v1::Span::SpanKind /*kind*/, - const std::map& attributes, - const std::vector& /*links*/) { +SamplingResult DynatraceSampler::shouldSample(const absl::optional parent_context, + const std::string& /*trace_id*/, + const std::string& /*name*/, OtelSpanKind /*kind*/, + const std::map& attributes, + const std::vector& /*links*/) { SamplingResult result; std::map att; diff --git a/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.h b/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.h index 5a02420053dc..d017e78ee7b6 100644 --- a/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.h +++ b/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.h @@ -27,7 +27,7 @@ class DynatraceSampler : public Sampler, Logger::Loggable { SamplingResult shouldSample(const absl::optional parent_context, const std::string& trace_id, const std::string& name, - ::opentelemetry::proto::trace::v1::Span::SpanKind spankind, + OtelSpanKind spankind, const std::map& attributes, const std::vector& links) override; diff --git a/source/extensions/tracers/opentelemetry/samplers/sampler.h b/source/extensions/tracers/opentelemetry/samplers/sampler.h index cef0925a2fcb..b01438a6c0c3 100644 --- a/source/extensions/tracers/opentelemetry/samplers/sampler.h +++ b/source/extensions/tracers/opentelemetry/samplers/sampler.h @@ -34,6 +34,11 @@ enum class Decision { */ using OTelSpanAttributes = std::map; +/** + * @brief The type of the span. + */ +using OtelSpanKind = ::opentelemetry::proto::trace::v1::Span::SpanKind; + struct SamplingResult { /// @see Decision Decision decision; @@ -75,8 +80,7 @@ class Sampler { */ virtual SamplingResult shouldSample(const absl::optional parent_context, const std::string& trace_id, const std::string& name, - ::opentelemetry::proto::trace::v1::Span::SpanKind spankind, - const OTelSpanAttributes& attributes, + OtelSpanKind spankind, const OTelSpanAttributes& attributes, const std::vector& links) PURE; /** diff --git a/source/extensions/tracers/opentelemetry/tracer.cc b/source/extensions/tracers/opentelemetry/tracer.cc index a62590fb798f..9b935b7bdb79 100644 --- a/source/extensions/tracers/opentelemetry/tracer.cc +++ b/source/extensions/tracers/opentelemetry/tracer.cc @@ -53,40 +53,23 @@ void setInitialAttributes(Span& new_span, const OTelSpanAttributes& initial_attr } // namespace -Span::Span(const Tracing::Config& config, const std::string& name, SystemTime start_time, - Envoy::TimeSource& time_source, Tracer& parent_tracer, bool downstream_span) +Span::Span(const std::string& name, SystemTime start_time, Envoy::TimeSource& time_source, + Tracer& parent_tracer, OtelSpanKind span_kind) : parent_tracer_(parent_tracer), time_source_(time_source) { span_ = ::opentelemetry::proto::trace::v1::Span(); - if (downstream_span) { - // If this is downstream span that be created by 'startSpan' for downstream request, then - // set the span type based on the spawnUpstreamSpan flag and traffic direction: - // * If separate tracing span will be created for upstream request, then set span type to - // SERVER because the downstream span should be server span in trace chain. - // * If separate tracing span will not be created for upstream request, that means the - // Envoy will not be treated as independent hop in trace chain and then set span type - // based on the traffic direction. - span_.set_kind(config.spawnUpstreamSpan() - ? ::opentelemetry::proto::trace::v1::Span::SPAN_KIND_SERVER - : config.operationName() == Tracing::OperationName::Egress - ? ::opentelemetry::proto::trace::v1::Span::SPAN_KIND_CLIENT - : ::opentelemetry::proto::trace::v1::Span::SPAN_KIND_SERVER); - } else { - // If this is an non-downstream span that be created for upstream request or async HTTP/gRPC - // request, then set the span type to client always. - span_.set_kind(::opentelemetry::proto::trace::v1::Span::SPAN_KIND_CLIENT); - } + span_.set_kind(span_kind); span_.set_name(name); span_.set_start_time_unix_nano(std::chrono::nanoseconds(start_time.time_since_epoch()).count()); } -Tracing::SpanPtr Span::spawnChild(const Tracing::Config& config, const std::string& name, +Tracing::SpanPtr Span::spawnChild(const Tracing::Config&, const std::string& name, SystemTime start_time) { // Build span_context from the current span, then generate the child span from that context. SpanContext span_context(kDefaultVersion, getTraceIdAsHex(), spanId(), sampled(), tracestate()); - return parent_tracer_.startSpan(config, name, start_time, {}, span_context, - /*downstream_span*/ false); + return parent_tracer_.startSpan(name, start_time, {}, span_context, + ::opentelemetry::proto::trace::v1::Span::SPAN_KIND_CLIENT); } void Span::finishSpan() { @@ -199,12 +182,11 @@ void Tracer::sendSpan(::opentelemetry::proto::trace::v1::Span& span) { } } -Tracing::SpanPtr Tracer::startSpan(const Tracing::Config& config, const std::string& operation_name, - SystemTime start_time, +Tracing::SpanPtr Tracer::startSpan(const std::string& operation_name, SystemTime start_time, const OTelSpanAttributes& initial_attributes, - Tracing::Decision tracing_decision, bool downstream_span) { + Tracing::Decision tracing_decision, OtelSpanKind span_kind) { // Create an Tracers::OpenTelemetry::Span class that will contain the OTel span. - Span new_span(config, operation_name, start_time, time_source_, *this, downstream_span); + Span new_span(operation_name, start_time, time_source_, *this, span_kind); uint64_t trace_id_high = random_.random(); uint64_t trace_id = random_.random(); new_span.setTraceId(absl::StrCat(Hex::uint64ToHex(trace_id_high), Hex::uint64ToHex(trace_id))); @@ -219,12 +201,12 @@ Tracing::SpanPtr Tracer::startSpan(const Tracing::Config& config, const std::str return std::make_unique(new_span); } -Tracing::SpanPtr Tracer::startSpan(const Tracing::Config& config, const std::string& operation_name, - SystemTime start_time, +Tracing::SpanPtr Tracer::startSpan(const std::string& operation_name, SystemTime start_time, const OTelSpanAttributes& initial_attributes, - const SpanContext& previous_span_context, bool downstream_span) { + const SpanContext& previous_span_context, + OtelSpanKind span_kind) { // Create a new span and populate details from the span context. - Span new_span(config, operation_name, start_time, time_source_, *this, downstream_span); + Span new_span(operation_name, start_time, time_source_, *this, span_kind); new_span.setTraceId(previous_span_context.traceId()); if (!previous_span_context.parentId().empty()) { new_span.setParentId(previous_span_context.parentId()); diff --git a/source/extensions/tracers/opentelemetry/tracer.h b/source/extensions/tracers/opentelemetry/tracer.h index ba1fedd4d36a..abfadbe50aa2 100644 --- a/source/extensions/tracers/opentelemetry/tracer.h +++ b/source/extensions/tracers/opentelemetry/tracer.h @@ -42,13 +42,13 @@ class Tracer : Logger::Loggable { void sendSpan(::opentelemetry::proto::trace::v1::Span& span); - Tracing::SpanPtr startSpan(const Tracing::Config& config, const std::string& operation_name, - SystemTime start_time, const OTelSpanAttributes& initial_attributes, - Tracing::Decision tracing_decision, bool downstream_span = true); + Tracing::SpanPtr startSpan(const std::string& operation_name, SystemTime start_time, + const OTelSpanAttributes& initial_attributes, + Tracing::Decision tracing_decision, OtelSpanKind span_kind); - Tracing::SpanPtr startSpan(const Tracing::Config& config, const std::string& operation_name, - SystemTime start_time, const OTelSpanAttributes& initial_attributes, - const SpanContext& previous_span_context, bool downstream_span = true); + Tracing::SpanPtr startSpan(const std::string& operation_name, SystemTime start_time, + const OTelSpanAttributes& initial_attributes, + const SpanContext& previous_span_context, OtelSpanKind span_kind); private: /** @@ -77,8 +77,8 @@ class Tracer : Logger::Loggable { */ class Span : Logger::Loggable, public Tracing::Span { public: - Span(const Tracing::Config& config, const std::string& name, SystemTime start_time, - Envoy::TimeSource& time_source, Tracer& parent_tracer, bool downstream_span); + Span(const std::string& name, SystemTime start_time, Envoy::TimeSource& time_source, + Tracer& parent_tracer, OtelSpanKind span_kind); // Tracing::Span functions void setOperation(absl::string_view /*operation*/) override{}; @@ -115,7 +115,7 @@ class Span : Logger::Loggable, public Tracing::Span { std::string getTraceIdAsHex() const override { return absl::BytesToHexString(span_.trace_id()); }; - ::opentelemetry::proto::trace::v1::Span::SpanKind spankind() const { return span_.kind(); } + OtelSpanKind spankind() const { return span_.kind(); } /** * Sets the span's id. diff --git a/test/extensions/tracers/opentelemetry/opentelemetry_tracer_impl_test.cc b/test/extensions/tracers/opentelemetry/opentelemetry_tracer_impl_test.cc index 300e92cf8d5a..2e22ee8b4d05 100644 --- a/test/extensions/tracers/opentelemetry/opentelemetry_tracer_impl_test.cc +++ b/test/extensions/tracers/opentelemetry/opentelemetry_tracer_impl_test.cc @@ -209,6 +209,25 @@ TEST_F(OpenTelemetryDriverTest, ParseSpanContextFromHeadersTest) { start_time_unix_nano: {} end_time_unix_nano: {} trace_state: "test=foo" + attributes: + - key: "http.request.method" + value: + string_value: "GET" + - key: "server.address" + value: + string_value: "test.com" + - key: "server.port" + value: + string_value: "" + - key: "url.path" + value: + string_value: "/" + - key: "url.query" + value: + string_value: "" + - key: "url.scheme" + value: + string_value: "" )"; opentelemetry::proto::collector::trace::v1::ExportTraceServiceRequest request_proto; SystemTime timestamp = time_system_.systemTime(); @@ -579,6 +598,24 @@ TEST_F(OpenTelemetryDriverTest, ExportOTLPSpanWithAttributes) { start_time_unix_nano: {} end_time_unix_nano: {} attributes: + - key: "http.request.method" + value: + string_value: "GET" + - key: "server.address" + value: + string_value: "test.com" + - key: "server.port" + value: + string_value: "" + - key: "url.path" + value: + string_value: "/" + - key: "url.query" + value: + string_value: "" + - key: "url.scheme" + value: + string_value: "" - key: "first_tag_name" value: string_value: "first_tag_new_value" @@ -690,6 +727,25 @@ TEST_F(OpenTelemetryDriverTest, ExportSpanWithCustomServiceName) { kind: SPAN_KIND_SERVER start_time_unix_nano: {} end_time_unix_nano: {} + attributes: + - key: "http.request.method" + value: + string_value: "GET" + - key: "server.address" + value: + string_value: "test.com" + - key: "server.port" + value: + string_value: "" + - key: "url.path" + value: + string_value: "/" + - key: "url.query" + value: + string_value: "" + - key: "url.scheme" + value: + string_value: "" )"; opentelemetry::proto::collector::trace::v1::ExportTraceServiceRequest request_proto; int64_t timestamp_ns = std::chrono::nanoseconds(timestamp.time_since_epoch()).count(); 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 f83c67a9720c..3721103b42da 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 @@ -83,7 +83,7 @@ TEST_P(DynatraceSamplerIntegrationTest, TestWithTraceparentAndTracestate) { .get(Http::LowerCaseString("tracestate"))[0] ->value() .getStringView(); - EXPECT_EQ("9712ad40-980df25c@dt=fw4;0;0;0;0;0;8,key=value", tracestate_value); + EXPECT_EQ("9712ad40-980df25c@dt=fw4;0;0;0;0;0;0,key=value", tracestate_value); } // Sends a request with traceparent but no tracestate header. @@ -111,7 +111,7 @@ TEST_P(DynatraceSamplerIntegrationTest, TestWithTraceparentOnly) { .get(Http::LowerCaseString("tracestate"))[0] ->value() .getStringView(); - EXPECT_EQ("9712ad40-980df25c@dt=fw4;0;0;0;0;0;8", tracestate_value); + EXPECT_EQ("9712ad40-980df25c@dt=fw4;0;0;0;0;0;0", tracestate_value); } // Sends a request without traceparent and tracestate header. @@ -134,7 +134,7 @@ TEST_P(DynatraceSamplerIntegrationTest, TestWithoutTraceparentAndTracestate) { .get(Http::LowerCaseString("tracestate"))[0] ->value() .getStringView(); - EXPECT_EQ("9712ad40-980df25c@dt=fw4;0;0;0;0;0;8", tracestate_value); + EXPECT_EQ("9712ad40-980df25c@dt=fw4;0;0;0;0;0;0", tracestate_value); } } // namespace diff --git a/test/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler_test.cc b/test/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler_test.cc index 4d88caec3e34..2223db8b3a4e 100644 --- a/test/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler_test.cc +++ b/test/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler_test.cc @@ -44,7 +44,7 @@ TEST_F(DynatraceSamplerTest, TestWithoutParentContext) { ::opentelemetry::proto::trace::v1::Span::SPAN_KIND_SERVER, {}, {}); EXPECT_EQ(sampling_result.decision, Decision::RECORD_AND_SAMPLE); EXPECT_EQ(sampling_result.attributes->size(), 1); - EXPECT_STREQ(sampling_result.tracestate.c_str(), "9712ad40-980df25c@dt=fw4;0;0;0;0;0;8"); + EXPECT_STREQ(sampling_result.tracestate.c_str(), "9712ad40-980df25c@dt=fw4;0;0;0;0;0;0"); EXPECT_TRUE(sampling_result.isRecording()); EXPECT_TRUE(sampling_result.isSampled()); } diff --git a/test/extensions/tracers/opentelemetry/samplers/sampler_test.cc b/test/extensions/tracers/opentelemetry/samplers/sampler_test.cc index f80103b4345c..8ca95e7bbd2b 100644 --- a/test/extensions/tracers/opentelemetry/samplers/sampler_test.cc +++ b/test/extensions/tracers/opentelemetry/samplers/sampler_test.cc @@ -27,8 +27,8 @@ class TestSampler : public Sampler { public: MOCK_METHOD(SamplingResult, shouldSample, ((const absl::optional), (const std::string&), (const std::string&), - (::opentelemetry::proto::trace::v1::Span::SpanKind), - (const std::map&), (const std::vector&)), + (OtelSpanKind), (const std::map&), + (const std::vector&)), (override)); MOCK_METHOD(std::string, getDescription, (), (const, override)); }; @@ -134,8 +134,8 @@ TEST_F(SamplerFactoryTest, TestWithSampler) { // shouldSample returns a result without additional attributes and Decision::RECORD_AND_SAMPLE EXPECT_CALL(*test_sampler, shouldSample(_, _, _, _, _, _)) .WillOnce([](const absl::optional, const std::string&, const std::string&, - ::opentelemetry::proto::trace::v1::Span::SpanKind, - const std::map&, const std::vector&) { + OtelSpanKind, const std::map&, + const std::vector&) { SamplingResult res; res.decision = Decision::RECORD_AND_SAMPLE; res.tracestate = "this_is=tracesate"; @@ -154,8 +154,8 @@ TEST_F(SamplerFactoryTest, TestWithSampler) { // shouldSamples return a result containing additional attributes and Decision::DROP EXPECT_CALL(*test_sampler, shouldSample(_, _, _, _, _, _)) .WillOnce([](const absl::optional, const std::string&, const std::string&, - ::opentelemetry::proto::trace::v1::Span::SpanKind, - const std::map&, const std::vector&) { + OtelSpanKind, const std::map&, + const std::vector&) { SamplingResult res; res.decision = Decision::DROP; std::map attributes; From edfcb036be8318f310f62fc8f2f6776b284c5bfa Mon Sep 17 00:00:00 2001 From: "thomas.ebner" Date: Fri, 17 Nov 2023 08:49:31 +0100 Subject: [PATCH 13/21] dummy commit Signed-off-by: thomas.ebner --- .../samplers/dynatrace/dynatrace_sampler_integration_test.cc | 1 - 1 file changed, 1 deletion(-) 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 3721103b42da..ee7ccb68678b 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 @@ -23,7 +23,6 @@ class DynatraceSamplerIntegrationTest : public Envoy::HttpIntegrationTest, public testing::TestWithParam { public: DynatraceSamplerIntegrationTest() : HttpIntegrationTest(Http::CodecType::HTTP1, GetParam()) { - const std::string yaml_string = R"EOF( provider: name: envoy.tracers.opentelemetry From 83483031244606c368f888ff063c7480d1a6430b Mon Sep 17 00:00:00 2001 From: "thomas.ebner" Date: Fri, 17 Nov 2023 12:29:48 +0100 Subject: [PATCH 14/21] fix partial merge Signed-off-by: thomas.ebner --- .../samplers/dynatrace/dynatrace_sampler.cc | 44 ++++++++++++++++--- .../samplers/dynatrace/dynatrace_sampler.h | 4 +- .../opentelemetry/samplers/sampler_test.cc | 8 ---- 3 files changed, 41 insertions(+), 15 deletions(-) diff --git a/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.cc b/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.cc index 48bef3a30b66..fade8813f6fc 100644 --- a/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.cc +++ b/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.cc @@ -28,17 +28,51 @@ FW4Tag DynatraceSampler::getFW4Tag(const Tracestate& tracestate) { return FW4Tag::createInvalid(); } +/* +OTelSpanAttributes getInitialAttributes(const Tracing::TraceContext& trace_context, + OTelSpanKind span_kind) { + OTelSpanAttributes attributes; + + // required attributes for a server span are defined in + // https://github.com/open-telemetry/semantic-conventions/blob/main/docs/http/http-spans.md + if (span_kind == ::opentelemetry::proto::trace::v1::Span::SPAN_KIND_SERVER) { + std::vector address_port = + absl::StrSplit(trace_context.host(), ":", absl::SkipEmpty()); + if (address_port.size() > 0) { + attributes["server.address"] = address_port[0]; + } + if (address_port.size() > 1) { + attributes["server.port"] = address_port[1]; + } + std::vector path_query = + absl::StrSplit(trace_context.path(), "?", absl::SkipEmpty()); + if (path_query.size() > 0) { + attributes["url.path"] = path_query[0]; + } + if (path_query.size() > 1) { + attributes["url.query"] = path_query[1]; + } + auto scheme = trace_context.getByKey(":scheme"); + if (scheme.has_value()) { + attributes["url.scheme"] = scheme.value(); + } + if (!trace_context.method().empty()) { + attributes["http.request.method"] = trace_context.method(); + } + } + return attributes; +} +*/ + SamplingResult DynatraceSampler::shouldSample(const absl::optional parent_context, const std::string& /*trace_id*/, - const std::string& /*name*/, OtelSpanKind /*kind*/, - const std::map& attributes, + const std::string& /*name*/, OTelSpanKind /*kind*/, + OptRef /*trace_context*/, const std::vector& /*links*/) { SamplingResult result; std::map att; - if (attributes.size() > 10) { - return {}; - } + Tracestate tracestate; tracestate.parse(parent_context.has_value() ? parent_context->tracestate() : ""); diff --git a/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.h b/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.h index d017e78ee7b6..35fc9ed856ac 100644 --- a/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.h +++ b/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.h @@ -27,8 +27,8 @@ class DynatraceSampler : public Sampler, Logger::Loggable { SamplingResult shouldSample(const absl::optional parent_context, const std::string& trace_id, const std::string& name, - OtelSpanKind spankind, - const std::map& attributes, + OTelSpanKind spankind, + OptRef trace_context, const std::vector& links) override; std::string getDescription() const override; diff --git a/test/extensions/tracers/opentelemetry/samplers/sampler_test.cc b/test/extensions/tracers/opentelemetry/samplers/sampler_test.cc index bad7e38eb058..a8ff62c71d85 100644 --- a/test/extensions/tracers/opentelemetry/samplers/sampler_test.cc +++ b/test/extensions/tracers/opentelemetry/samplers/sampler_test.cc @@ -28,11 +28,7 @@ class TestSampler : public Sampler { public: MOCK_METHOD(SamplingResult, shouldSample, ((const absl::optional), (const std::string&), (const std::string&), -<<<<<<< HEAD - (OtelSpanKind), (const std::map&), -======= (OTelSpanKind), (OptRef), ->>>>>>> upstream/main (const std::vector&)), (override)); MOCK_METHOD(std::string, getDescription, (), (const, override)); @@ -139,11 +135,7 @@ TEST_F(SamplerFactoryTest, TestWithSampler) { // shouldSample returns a result without additional attributes and Decision::RECORD_AND_SAMPLE EXPECT_CALL(*test_sampler, shouldSample(_, _, _, _, _, _)) .WillOnce([](const absl::optional, const std::string&, const std::string&, -<<<<<<< HEAD - OtelSpanKind, const std::map&, -======= OTelSpanKind, OptRef, ->>>>>>> upstream/main const std::vector&) { SamplingResult res; res.decision = Decision::RECORD_AND_SAMPLE; From 258f4c4bbb1dd21d7c0878b4fb6112d914a32650 Mon Sep 17 00:00:00 2001 From: "thomas.ebner" Date: Mon, 20 Nov 2023 14:20:01 +0100 Subject: [PATCH 15/21] * dummy usage of trace_context in dynatrace sampler * start a timer (just for testing, might be used to query config) Signed-off-by: thomas.ebner --- .../samplers/dynatrace/dynatrace_sampler.cc | 15 ++++++++++++++- .../samplers/dynatrace/dynatrace_sampler.h | 13 +++++++++++-- 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.cc b/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.cc index fade8813f6fc..669f75861536 100644 --- a/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.cc +++ b/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.cc @@ -67,12 +67,25 @@ OTelSpanAttributes getInitialAttributes(const Tracing::TraceContext& trace_conte SamplingResult DynatraceSampler::shouldSample(const absl::optional parent_context, const std::string& /*trace_id*/, const std::string& /*name*/, OTelSpanKind /*kind*/, - OptRef /*trace_context*/, + OptRef trace_context, const std::vector& /*links*/) { SamplingResult result; std::map att; + if (trace_context.has_value()) { + ENVOY_LOG(info, "trace_context->path:\t{}", trace_context->path()); + ENVOY_LOG(info, "trace_context->method():\t{}", trace_context->method()); + ENVOY_LOG(info, "trace_context->host():\t{}", trace_context->host()); + } + auto current = tracer_factory_context_.serverFactoryContext() + .timeSource() + .monotonicTime() + .time_since_epoch(); + + auto current_ms = std::chrono::duration_cast(current); + ENVOY_LOG(info, "monotonicTime:\t{}", current_ms.count()); + Tracestate tracestate; tracestate.parse(parent_context.has_value() ? parent_context->tracestate() : ""); diff --git a/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.h b/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.h index 35fc9ed856ac..44a132413a80 100644 --- a/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.h +++ b/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.h @@ -21,9 +21,16 @@ class DynatraceSampler : public Sampler, Logger::Loggable { public: explicit DynatraceSampler( const envoy::extensions::tracers::opentelemetry::samplers::v3::DynatraceSamplerConfig config, - Server::Configuration::TracerFactoryContext& /*context*/) + Server::Configuration::TracerFactoryContext& context) : tenant_id_(config.tenant_id()), cluster_id_(config.cluster_id()), - dynatrace_tracestate_(tenant_id_, cluster_id_) {} + dynatrace_tracestate_(tenant_id_, cluster_id_), tracer_factory_context_(context) { + timer_ = tracer_factory_context_.serverFactoryContext().mainThreadDispatcher().createTimer( + [this]() -> void { + ENVOY_LOG(info, "HELLO FROM TIMER"); + timer_->enableTimer(std::chrono::seconds(60)); + }); + timer_->enableTimer(std::chrono::seconds(10)); + } SamplingResult shouldSample(const absl::optional parent_context, const std::string& trace_id, const std::string& name, @@ -37,6 +44,8 @@ class DynatraceSampler : public Sampler, Logger::Loggable { std::string tenant_id_; std::string cluster_id_; DynatraceTracestate dynatrace_tracestate_; + Server::Configuration::TracerFactoryContext& tracer_factory_context_; + Event::TimerPtr timer_; FW4Tag getFW4Tag(const Tracestate& tracestate); DynatraceTracestate createTracestate(bool is_recording, std::string sampling_exponent); From b6e08dd25a03dac07e8cbf7e2d9e854592b4473b Mon Sep 17 00:00:00 2001 From: "thomas.ebner" Date: Tue, 21 Nov 2023 08:36:56 +0100 Subject: [PATCH 16/21] cleanup Signed-off-by: thomas.ebner --- .../samplers/dynatrace/dynatrace_sampler.cc | 13 +++++++++++++ .../samplers/dynatrace/dynatrace_sampler.h | 14 ++------------ 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.cc b/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.cc index 669f75861536..006366d7597f 100644 --- a/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.cc +++ b/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.cc @@ -64,6 +64,19 @@ OTelSpanAttributes getInitialAttributes(const Tracing::TraceContext& trace_conte } */ +DynatraceSampler::DynatraceSampler( + const envoy::extensions::tracers::opentelemetry::samplers::v3::DynatraceSamplerConfig config, + Server::Configuration::TracerFactoryContext& context) + : tenant_id_(config.tenant_id()), cluster_id_(config.cluster_id()), + dynatrace_tracestate_(tenant_id_, cluster_id_), tracer_factory_context_(context) { + timer_ = tracer_factory_context_.serverFactoryContext().mainThreadDispatcher().createTimer( + [this]() -> void { + ENVOY_LOG(info, "HELLO FROM TIMER"); + timer_->enableTimer(std::chrono::seconds(60)); + }); + timer_->enableTimer(std::chrono::seconds(10)); +} + SamplingResult DynatraceSampler::shouldSample(const absl::optional parent_context, const std::string& /*trace_id*/, const std::string& /*name*/, OTelSpanKind /*kind*/, diff --git a/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.h b/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.h index 44a132413a80..51b4da40fd0a 100644 --- a/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.h +++ b/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.h @@ -19,18 +19,9 @@ namespace OpenTelemetry { */ class DynatraceSampler : public Sampler, Logger::Loggable { public: - explicit DynatraceSampler( + DynatraceSampler( const envoy::extensions::tracers::opentelemetry::samplers::v3::DynatraceSamplerConfig config, - Server::Configuration::TracerFactoryContext& context) - : tenant_id_(config.tenant_id()), cluster_id_(config.cluster_id()), - dynatrace_tracestate_(tenant_id_, cluster_id_), tracer_factory_context_(context) { - timer_ = tracer_factory_context_.serverFactoryContext().mainThreadDispatcher().createTimer( - [this]() -> void { - ENVOY_LOG(info, "HELLO FROM TIMER"); - timer_->enableTimer(std::chrono::seconds(60)); - }); - timer_->enableTimer(std::chrono::seconds(10)); - } + Server::Configuration::TracerFactoryContext& context); SamplingResult shouldSample(const absl::optional parent_context, const std::string& trace_id, const std::string& name, @@ -48,7 +39,6 @@ class DynatraceSampler : public Sampler, Logger::Loggable { Event::TimerPtr timer_; FW4Tag getFW4Tag(const Tracestate& tracestate); - DynatraceTracestate createTracestate(bool is_recording, std::string sampling_exponent); }; } // namespace OpenTelemetry From 6319086dd4ff8f907dbc2a20a91b6d50690b5eab Mon Sep 17 00:00:00 2001 From: "thomas.ebner" Date: Wed, 22 Nov 2023 07:37:42 +0100 Subject: [PATCH 17/21] change FW4 tag creation (pathInfo was missing), seems it is now accecpted by oneagent Signed-off-by: thomas.ebner --- .../opentelemetry/samplers/dynatrace/dynatrace_tracestate.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_tracestate.h b/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_tracestate.h index de964ccf980d..b100a73f494a 100644 --- a/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_tracestate.h +++ b/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_tracestate.h @@ -43,7 +43,8 @@ class FW4Tag { int getSamplingExponent() const { return sampling_exponent_; }; std::string createValue() const { - std::string ret = absl::StrCat("fw4;0;0;0;0;", ignored_ ? "1" : "0", ";", sampling_exponent_); + std::string ret = + absl::StrCat("fw4;0;0;0;0;", ignored_ ? "1" : "0", ";", sampling_exponent_, ";0"); return ret; } From a9ec6b08d4808e78ac9acc171972c39cc646e8b1 Mon Sep 17 00:00:00 2001 From: "thomas.ebner" Date: Wed, 22 Nov 2023 07:50:15 +0100 Subject: [PATCH 18/21] do sampling decision: sample every second path Signed-off-by: thomas.ebner --- .../samplers/dynatrace/dynatrace_sampler.cc | 19 +++++++++++++++---- .../samplers/dynatrace/dynatrace_sampler.h | 2 +- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.cc b/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.cc index 006366d7597f..3bda8d18daee 100644 --- a/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.cc +++ b/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.cc @@ -68,7 +68,8 @@ DynatraceSampler::DynatraceSampler( const envoy::extensions::tracers::opentelemetry::samplers::v3::DynatraceSamplerConfig config, Server::Configuration::TracerFactoryContext& context) : tenant_id_(config.tenant_id()), cluster_id_(config.cluster_id()), - dynatrace_tracestate_(tenant_id_, cluster_id_), tracer_factory_context_(context) { + dynatrace_tracestate_(tenant_id_, cluster_id_), tracer_factory_context_(context), + counter_(0) { timer_ = tracer_factory_context_.serverFactoryContext().mainThreadDispatcher().createTimer( [this]() -> void { ENVOY_LOG(info, "HELLO FROM TIMER"); @@ -109,9 +110,19 @@ SamplingResult DynatraceSampler::shouldSample(const absl::optional att[SAMPLING_EXTRAPOLATION_SPAN_ATTRIBUTE_NAME] = std::to_string(fw4_tag.getSamplingExponent()); result.tracestate = parent_context->tracestate(); - } else { // make a sampling decision - bool sample = true; // TODO - int sampling_exponent = 0; // TODO + } else { + // make a sampling decision + uint32_t current_counter = counter_++; + bool sample; + int sampling_exponent; + if (current_counter % 2) { + sample = true; + sampling_exponent = 1; + } else { + sample = false; + sampling_exponent = 0; + } + att[SAMPLING_EXTRAPOLATION_SPAN_ATTRIBUTE_NAME] = std::to_string(sampling_exponent); result.decision = sample ? Decision::RECORD_AND_SAMPLE : Decision::DROP; diff --git a/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.h b/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.h index 51b4da40fd0a..c65f85a6334e 100644 --- a/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.h +++ b/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.h @@ -37,7 +37,7 @@ class DynatraceSampler : public Sampler, Logger::Loggable { DynatraceTracestate dynatrace_tracestate_; Server::Configuration::TracerFactoryContext& tracer_factory_context_; Event::TimerPtr timer_; - + std::atomic counter_; FW4Tag getFW4Tag(const Tracestate& tracestate); }; From 34ee752b219868211dd19d9fcd322fde5c38ce7b Mon Sep 17 00:00:00 2001 From: "thomas.ebner" Date: Thu, 23 Nov 2023 14:20:33 +0100 Subject: [PATCH 19/21] remove dead code, minor cleanup Signed-off-by: thomas.ebner --- .../opentelemetry/samplers/dynatrace/BUILD | 1 + .../samplers/dynatrace/dynatrace_sampler.cc | 84 +++---------------- .../samplers/dynatrace/dynatrace_sampler.h | 6 +- .../dynatrace/dynatrace_tracestate.cc | 46 ++++++++++ .../samplers/dynatrace/dynatrace_tracestate.h | 36 ++------ .../samplers/dynatrace/tracestate.h | 2 + .../dynatrace_sampler_integration_test.cc | 6 +- .../dynatrace/dynatrace_sampler_test.cc | 6 +- .../dynatrace/dynatrace_tracestate_test.cc | 2 +- 9 files changed, 77 insertions(+), 112 deletions(-) create mode 100644 source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_tracestate.cc diff --git a/source/extensions/tracers/opentelemetry/samplers/dynatrace/BUILD b/source/extensions/tracers/opentelemetry/samplers/dynatrace/BUILD index a89eb440e79d..2d772e01cb9f 100644 --- a/source/extensions/tracers/opentelemetry/samplers/dynatrace/BUILD +++ b/source/extensions/tracers/opentelemetry/samplers/dynatrace/BUILD @@ -25,6 +25,7 @@ envoy_cc_library( name = "dynatrace_sampler_lib", srcs = [ "dynatrace_sampler.cc", + "dynatrace_tracestate.cc", "tracestate.cc", ], hdrs = [ diff --git a/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.cc b/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.cc index 3bda8d18daee..c5f34f548a4e 100644 --- a/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.cc +++ b/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.cc @@ -1,4 +1,4 @@ -#include "dynatrace_sampler.h" +#include "source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.h" #include #include @@ -20,7 +20,7 @@ static const char* SAMPLING_EXTRAPOLATION_SPAN_ATTRIBUTE_NAME = FW4Tag DynatraceSampler::getFW4Tag(const Tracestate& tracestate) { for (auto const& entry : tracestate.entries()) { - if (dynatrace_tracestate_.keyMatches( + if (dt_tracestate_entry_.keyMatches( entry.key)) { // found a tracestate entry with key matching our tenant/cluster return FW4Tag::create(entry.value); } @@ -28,91 +28,33 @@ FW4Tag DynatraceSampler::getFW4Tag(const Tracestate& tracestate) { return FW4Tag::createInvalid(); } -/* -OTelSpanAttributes getInitialAttributes(const Tracing::TraceContext& trace_context, - OTelSpanKind span_kind) { - OTelSpanAttributes attributes; - - // required attributes for a server span are defined in - // https://github.com/open-telemetry/semantic-conventions/blob/main/docs/http/http-spans.md - if (span_kind == ::opentelemetry::proto::trace::v1::Span::SPAN_KIND_SERVER) { - std::vector address_port = - absl::StrSplit(trace_context.host(), ":", absl::SkipEmpty()); - if (address_port.size() > 0) { - attributes["server.address"] = address_port[0]; - } - if (address_port.size() > 1) { - attributes["server.port"] = address_port[1]; - } - std::vector path_query = - absl::StrSplit(trace_context.path(), "?", absl::SkipEmpty()); - if (path_query.size() > 0) { - attributes["url.path"] = path_query[0]; - } - if (path_query.size() > 1) { - attributes["url.query"] = path_query[1]; - } - auto scheme = trace_context.getByKey(":scheme"); - if (scheme.has_value()) { - attributes["url.scheme"] = scheme.value(); - } - if (!trace_context.method().empty()) { - attributes["http.request.method"] = trace_context.method(); - } - } - return attributes; -} -*/ - DynatraceSampler::DynatraceSampler( const envoy::extensions::tracers::opentelemetry::samplers::v3::DynatraceSamplerConfig config, - Server::Configuration::TracerFactoryContext& context) + Server::Configuration::TracerFactoryContext& /*context*/) : tenant_id_(config.tenant_id()), cluster_id_(config.cluster_id()), - dynatrace_tracestate_(tenant_id_, cluster_id_), tracer_factory_context_(context), - counter_(0) { - timer_ = tracer_factory_context_.serverFactoryContext().mainThreadDispatcher().createTimer( - [this]() -> void { - ENVOY_LOG(info, "HELLO FROM TIMER"); - timer_->enableTimer(std::chrono::seconds(60)); - }); - timer_->enableTimer(std::chrono::seconds(10)); -} + dt_tracestate_entry_(tenant_id_, cluster_id_), counter_(0) {} SamplingResult DynatraceSampler::shouldSample(const absl::optional parent_context, const std::string& /*trace_id*/, const std::string& /*name*/, OTelSpanKind /*kind*/, - OptRef trace_context, + OptRef /*trace_context*/, const std::vector& /*links*/) { SamplingResult result; std::map att; - - if (trace_context.has_value()) { - ENVOY_LOG(info, "trace_context->path:\t{}", trace_context->path()); - ENVOY_LOG(info, "trace_context->method():\t{}", trace_context->method()); - ENVOY_LOG(info, "trace_context->host():\t{}", trace_context->host()); - } - auto current = tracer_factory_context_.serverFactoryContext() - .timeSource() - .monotonicTime() - .time_since_epoch(); - - auto current_ms = std::chrono::duration_cast(current); - ENVOY_LOG(info, "monotonicTime:\t{}", current_ms.count()); - + // search for an existing forward tag in the tracestate Tracestate tracestate; tracestate.parse(parent_context.has_value() ? parent_context->tracestate() : ""); - FW4Tag fw4_tag = getFW4Tag(tracestate); - - if (fw4_tag.isValid()) { // we found a trace decision in tracestate header + if (FW4Tag fw4_tag = getFW4Tag(tracestate); + fw4_tag.isValid()) { // we found a trace decision in tracestate header result.decision = fw4_tag.isIgnored() ? Decision::DROP : Decision::RECORD_AND_SAMPLE; att[SAMPLING_EXTRAPOLATION_SPAN_ATTRIBUTE_NAME] = std::to_string(fw4_tag.getSamplingExponent()); result.tracestate = parent_context->tracestate(); - } else { - // make a sampling decision - uint32_t current_counter = counter_++; + } else { // make a sampling decision + // this is just a demo, we sample every second request here + uint32_t current_counter = ++counter_; bool sample; int sampling_exponent; if (current_counter % 2) { @@ -126,9 +68,9 @@ SamplingResult DynatraceSampler::shouldSample(const absl::optional att[SAMPLING_EXTRAPOLATION_SPAN_ATTRIBUTE_NAME] = std::to_string(sampling_exponent); result.decision = sample ? Decision::RECORD_AND_SAMPLE : Decision::DROP; - + // create new forward tag and add it to tracestate FW4Tag new_tag = FW4Tag::create(!sample, sampling_exponent); - tracestate.add(dynatrace_tracestate_.getKey(), new_tag.createValue()); + tracestate.add(dt_tracestate_entry_.getKey(), new_tag.asString()); result.tracestate = tracestate.asString(); } diff --git a/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.h b/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.h index c65f85a6334e..360048612457 100644 --- a/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.h +++ b/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.h @@ -34,10 +34,8 @@ class DynatraceSampler : public Sampler, Logger::Loggable { private: std::string tenant_id_; std::string cluster_id_; - DynatraceTracestate dynatrace_tracestate_; - Server::Configuration::TracerFactoryContext& tracer_factory_context_; - Event::TimerPtr timer_; - std::atomic counter_; + DtTracestateEntry dt_tracestate_entry_; + std::atomic counter_; // request counter for dummy sampling FW4Tag getFW4Tag(const Tracestate& tracestate); }; diff --git a/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_tracestate.cc b/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_tracestate.cc new file mode 100644 index 000000000000..25c9db3c6a94 --- /dev/null +++ b/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_tracestate.cc @@ -0,0 +1,46 @@ +#include "source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_tracestate.h" + +#include +#include + +#include "absl/strings/match.h" +#include "absl/strings/str_cat.h" +#include "absl/strings/str_split.h" +#include "absl/strings/string_view.h" + +namespace Envoy { +namespace Extensions { +namespace Tracers { +namespace OpenTelemetry { + +FW4Tag FW4Tag::createInvalid() { return {false, false, 0}; } + +FW4Tag FW4Tag::create(bool ignored, int sampling_exponent) { + return {true, ignored, sampling_exponent}; +} + +FW4Tag FW4Tag::create(const std::string& value) { + std::vector tracestate_components = + absl::StrSplit(value, ';', absl::AllowEmpty()); + if (tracestate_components.size() < 7) { + return createInvalid(); + } + + if (tracestate_components[0] != "fw4") { + return createInvalid(); + } + bool ignored = tracestate_components[5] == "1"; + int sampling_exponent = std::stoi(std::string(tracestate_components[6])); + return {true, ignored, sampling_exponent}; +} + +std::string FW4Tag::asString() const { + std::string ret = + absl::StrCat("fw4;0;0;0;0;", ignored_ ? "1" : "0", ";", sampling_exponent_, ";0"); + return ret; +} + +} // namespace OpenTelemetry +} // namespace Tracers +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_tracestate.h b/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_tracestate.h index b100a73f494a..eb2935fc8166 100644 --- a/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_tracestate.h +++ b/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_tracestate.h @@ -1,13 +1,8 @@ #pragma once -#include -#include #include -#include -#include "absl/strings/match.h" #include "absl/strings/str_cat.h" -#include "absl/strings/str_split.h" #include "absl/strings/string_view.h" namespace Envoy { @@ -17,37 +12,18 @@ namespace OpenTelemetry { class FW4Tag { public: - static FW4Tag createInvalid() { return {false, false, 0}; } + static FW4Tag createInvalid(); - static FW4Tag create(bool ignored, int sampling_exponent) { - return {true, ignored, sampling_exponent}; - } + static FW4Tag create(bool ignored, int sampling_exponent); - static FW4Tag create(const std::string& value) { - std::vector tracestate_components = - absl::StrSplit(value, ';', absl::AllowEmpty()); - if (tracestate_components.size() < 7) { - return createInvalid(); - } + static FW4Tag create(const std::string& value); - if (tracestate_components[0] != "fw4") { - return createInvalid(); - } - bool ignored = tracestate_components[5] == "1"; - int sampling_exponent = std::stoi(std::string(tracestate_components[6])); - return {true, ignored, sampling_exponent}; - } + std::string asString() const; bool isValid() const { return valid_; }; bool isIgnored() const { return ignored_; }; int getSamplingExponent() const { return sampling_exponent_; }; - std::string createValue() const { - std::string ret = - absl::StrCat("fw4;0;0;0;0;", ignored_ ? "1" : "0", ";", sampling_exponent_, ";0"); - return ret; - } - private: FW4Tag(bool valid, bool ignored, int sampling_exponent) : valid_(valid), ignored_(ignored), sampling_exponent_(sampling_exponent) {} @@ -58,9 +34,9 @@ class FW4Tag { }; // -@dt=fw4;0;0;0;0;;; -class DynatraceTracestate { +class DtTracestateEntry { public: - DynatraceTracestate(const std::string& tenant_id, const std::string& cluster_id) { + DtTracestateEntry(const std::string& tenant_id, const std::string& cluster_id) { key_ = absl::StrCat(absl::string_view(tenant_id), "-", absl::string_view(cluster_id), "@dt"); } diff --git a/source/extensions/tracers/opentelemetry/samplers/dynatrace/tracestate.h b/source/extensions/tracers/opentelemetry/samplers/dynatrace/tracestate.h index 5eea07a80466..1e3e2468451e 100644 --- a/source/extensions/tracers/opentelemetry/samplers/dynatrace/tracestate.h +++ b/source/extensions/tracers/opentelemetry/samplers/dynatrace/tracestate.h @@ -9,6 +9,7 @@ namespace Tracers { namespace OpenTelemetry { class TracestateEntry { + // TODO: could be string_views to Tracestate::raw_trace_state_ public: std::string key; std::string value; @@ -24,6 +25,7 @@ class Tracestate { void parse(const std::string& tracestate); void add(const std::string& key, const std::string& value); + std::vector entries() const { return entries_; } std::string asString() const { return raw_trace_state_; } 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 ee7ccb68678b..998ce7fab4e5 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 @@ -82,7 +82,7 @@ TEST_P(DynatraceSamplerIntegrationTest, TestWithTraceparentAndTracestate) { .get(Http::LowerCaseString("tracestate"))[0] ->value() .getStringView(); - EXPECT_EQ("9712ad40-980df25c@dt=fw4;0;0;0;0;0;0,key=value", tracestate_value); + EXPECT_EQ("9712ad40-980df25c@dt=fw4;0;0;0;0;0;1;0,key=value", tracestate_value); } // Sends a request with traceparent but no tracestate header. @@ -110,7 +110,7 @@ TEST_P(DynatraceSamplerIntegrationTest, TestWithTraceparentOnly) { .get(Http::LowerCaseString("tracestate"))[0] ->value() .getStringView(); - EXPECT_EQ("9712ad40-980df25c@dt=fw4;0;0;0;0;0;0", tracestate_value); + EXPECT_EQ("9712ad40-980df25c@dt=fw4;0;0;0;0;0;1;0", tracestate_value); } // Sends a request without traceparent and tracestate header. @@ -133,7 +133,7 @@ TEST_P(DynatraceSamplerIntegrationTest, TestWithoutTraceparentAndTracestate) { .get(Http::LowerCaseString("tracestate"))[0] ->value() .getStringView(); - EXPECT_EQ("9712ad40-980df25c@dt=fw4;0;0;0;0;0;0", tracestate_value); + EXPECT_EQ("9712ad40-980df25c@dt=fw4;0;0;0;0;0;1;0", tracestate_value); } } // namespace diff --git a/test/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler_test.cc b/test/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler_test.cc index 2223db8b3a4e..d3e8422c773d 100644 --- a/test/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler_test.cc +++ b/test/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler_test.cc @@ -27,13 +27,13 @@ class DynatraceSamplerTest : public testing::Test { DynatraceSamplerTest() { TestUtility::loadFromYaml(yaml_string, config_); NiceMock context; - sampler_ = std::make_shared(config_, context); + sampler_ = std::make_unique(config_, context); EXPECT_STREQ(sampler_->getDescription().c_str(), "DynatraceSampler"); } protected: envoy::extensions::tracers::opentelemetry::samplers::v3::DynatraceSamplerConfig config_; - std::shared_ptr sampler_; + std::unique_ptr sampler_; }; // Verify sampler being invoked with an invalid span context @@ -44,7 +44,7 @@ TEST_F(DynatraceSamplerTest, TestWithoutParentContext) { ::opentelemetry::proto::trace::v1::Span::SPAN_KIND_SERVER, {}, {}); EXPECT_EQ(sampling_result.decision, Decision::RECORD_AND_SAMPLE); EXPECT_EQ(sampling_result.attributes->size(), 1); - EXPECT_STREQ(sampling_result.tracestate.c_str(), "9712ad40-980df25c@dt=fw4;0;0;0;0;0;0"); + EXPECT_STREQ(sampling_result.tracestate.c_str(), "9712ad40-980df25c@dt=fw4;0;0;0;0;0;1;0"); EXPECT_TRUE(sampling_result.isRecording()); EXPECT_TRUE(sampling_result.isSampled()); } diff --git a/test/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_tracestate_test.cc b/test/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_tracestate_test.cc index 73d42af36a26..928235e58416 100644 --- a/test/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_tracestate_test.cc +++ b/test/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_tracestate_test.cc @@ -11,7 +11,7 @@ namespace Tracers { namespace OpenTelemetry { TEST(DynatraceTracestateTest, TestKey) { - DynatraceTracestate tracestate("98812ad49", "980df25c"); + DtTracestateEntry tracestate("98812ad49", "980df25c"); EXPECT_STREQ(tracestate.getKey().c_str(), "98812ad49-980df25c@dt"); } From c19b0705c46b8536604489d96cf4877ef9f5ddcc Mon Sep 17 00:00:00 2001 From: "thomas.ebner" Date: Fri, 24 Nov 2023 11:31:11 +0100 Subject: [PATCH 20/21] make constructer param a reference Signed-off-by: thomas.ebner --- .../opentelemetry/samplers/dynatrace/dynatrace_sampler.cc | 2 +- .../opentelemetry/samplers/dynatrace/dynatrace_sampler.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.cc b/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.cc index c5f34f548a4e..a76a76810ecb 100644 --- a/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.cc +++ b/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.cc @@ -29,7 +29,7 @@ FW4Tag DynatraceSampler::getFW4Tag(const Tracestate& tracestate) { } DynatraceSampler::DynatraceSampler( - const envoy::extensions::tracers::opentelemetry::samplers::v3::DynatraceSamplerConfig config, + const envoy::extensions::tracers::opentelemetry::samplers::v3::DynatraceSamplerConfig& config, Server::Configuration::TracerFactoryContext& /*context*/) : tenant_id_(config.tenant_id()), cluster_id_(config.cluster_id()), dt_tracestate_entry_(tenant_id_, cluster_id_), counter_(0) {} diff --git a/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.h b/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.h index 360048612457..33ae48fc9c55 100644 --- a/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.h +++ b/source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.h @@ -20,7 +20,7 @@ namespace OpenTelemetry { class DynatraceSampler : public Sampler, Logger::Loggable { public: DynatraceSampler( - const envoy::extensions::tracers::opentelemetry::samplers::v3::DynatraceSamplerConfig config, + const envoy::extensions::tracers::opentelemetry::samplers::v3::DynatraceSamplerConfig& config, Server::Configuration::TracerFactoryContext& context); SamplingResult shouldSample(const absl::optional parent_context, From b26251005c28f75a9a9a4769388a4c06e8f58cd6 Mon Sep 17 00:00:00 2001 From: "thomas.ebner" Date: Mon, 27 Nov 2023 06:18:37 +0100 Subject: [PATCH 21/21] review feedback Signed-off-by: thomas.ebner --- .../tracers/opentelemetry/samplers/v3/dynatrace_sampler.proto | 2 +- .../tracers/opentelemetry/samplers/dynatrace/config.cc | 3 +-- .../tracers/opentelemetry/samplers/dynatrace/tracestate.h | 4 ++++ 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/api/envoy/extensions/tracers/opentelemetry/samplers/v3/dynatrace_sampler.proto b/api/envoy/extensions/tracers/opentelemetry/samplers/v3/dynatrace_sampler.proto index d02072f1e5cc..e0c37a7e51a4 100644 --- a/api/envoy/extensions/tracers/opentelemetry/samplers/v3/dynatrace_sampler.proto +++ b/api/envoy/extensions/tracers/opentelemetry/samplers/v3/dynatrace_sampler.proto @@ -10,7 +10,7 @@ option java_multiple_files = true; option go_package = "github.com/envoyproxy/go-control-plane/envoy/extensions/tracers/opentelemetry/samplers/v3;samplersv3"; option (udpa.annotations.file_status).package_version_status = ACTIVE; -// [#protodoc-title: Always On Sampler config] +// [#protodoc-title: Dynatrace Sampler config] // [#extension: envoy.tracers.opentelemetry.samplers.dynatrace] message DynatraceSamplerConfig { diff --git a/source/extensions/tracers/opentelemetry/samplers/dynatrace/config.cc b/source/extensions/tracers/opentelemetry/samplers/dynatrace/config.cc index 03b6352238b9..9e410e75ea0c 100644 --- a/source/extensions/tracers/opentelemetry/samplers/dynatrace/config.cc +++ b/source/extensions/tracers/opentelemetry/samplers/dynatrace/config.cc @@ -4,8 +4,7 @@ #include "source/common/config/utility.h" #include "source/common/protobuf/utility.h" - -#include "dynatrace_sampler.h" +#include "source/extensions/tracers/opentelemetry/samplers/dynatrace/dynatrace_sampler.h" namespace Envoy { namespace Extensions { diff --git a/source/extensions/tracers/opentelemetry/samplers/dynatrace/tracestate.h b/source/extensions/tracers/opentelemetry/samplers/dynatrace/tracestate.h index 1e3e2468451e..4106a088e10f 100644 --- a/source/extensions/tracers/opentelemetry/samplers/dynatrace/tracestate.h +++ b/source/extensions/tracers/opentelemetry/samplers/dynatrace/tracestate.h @@ -8,6 +8,10 @@ namespace Extensions { namespace Tracers { namespace OpenTelemetry { +// TODO: file should be moved outside "dynatrace" folder, can be useful for other implementations +// should be aligned to open telemetry implementation: +// https://github.com/open-telemetry/opentelemetry-cpp/blob/main/api/include/opentelemetry/trace/trace_state.h + class TracestateEntry { // TODO: could be string_views to Tracestate::raw_trace_state_ public: