From 3010537e517b369d50a902ae0fc347a1c756d61b Mon Sep 17 00:00:00 2001 From: Gustavo Lopes Date: Tue, 3 Jun 2025 15:08:56 +0100 Subject: [PATCH 1/6] Implement DD_TRACE_ENABLED=false --- include/datadog/collector.h | 4 +- include/datadog/config.h | 1 + include/datadog/datadog_agent_config.h | 4 + include/datadog/environment.h | 1 + include/datadog/injection_options.h | 4 +- include/datadog/null_collector.h | 2 +- include/datadog/sampling_mechanism.h | 2 +- include/datadog/trace_segment.h | 8 +- include/datadog/tracer.h | 8 +- include/datadog/tracer_config.h | 12 ++ src/datadog/config_manager.cpp | 6 +- src/datadog/config_manager.h | 5 +- src/datadog/datadog_agent.cpp | 10 +- src/datadog/datadog_agent.h | 7 +- src/datadog/datadog_agent_config.cpp | 7 ++ src/datadog/extraction_util.cpp | 2 +- src/datadog/tags.cpp | 2 + src/datadog/tags.h | 2 + src/datadog/telemetry/telemetry_impl.cpp | 2 + src/datadog/trace_sampler.cpp | 91 ++++++++++++++ src/datadog/trace_sampler.h | 46 ++++++++ src/datadog/trace_segment.cpp | 55 ++++++++- src/datadog/tracer.cpp | 40 +++++-- src/datadog/tracer_config.cpp | 13 ++ src/datadog/tracer_telemetry.cpp | 0 test/mocks/collectors.h | 10 +- test/test_datadog_agent.cpp | 55 +++++++++ test/test_span.cpp | 144 +++++++++++++++++++++++ test/test_tracer.cpp | 134 +++++++++++++++++++++ test/test_tracer_config.cpp | 19 +++ 30 files changed, 658 insertions(+), 38 deletions(-) create mode 100644 src/datadog/tracer_telemetry.cpp diff --git a/include/datadog/collector.h b/include/datadog/collector.h index 96f9e64d..4c8dbe75 100644 --- a/include/datadog/collector.h +++ b/include/datadog/collector.h @@ -19,7 +19,7 @@ namespace datadog { namespace tracing { struct SpanData; -class TraceSampler; +class ErasedTraceSampler; class Collector { public: @@ -29,7 +29,7 @@ class Collector { // occurs. virtual Expected send( std::vector>&& spans, - const std::shared_ptr& response_handler) = 0; + const std::shared_ptr& response_handler) = 0; // Return a JSON representation of this object's configuration. The JSON // representation is an object with the following properties: diff --git a/include/datadog/config.h b/include/datadog/config.h index df017e54..3a26c78d 100644 --- a/include/datadog/config.h +++ b/include/datadog/config.h @@ -27,6 +27,7 @@ enum class ConfigName : char { SPAN_SAMPLING_RULES, TRACE_BAGGAGE_MAX_BYTES, TRACE_BAGGAGE_MAX_ITEMS, + APM_TRACING_ENABLED, }; // Represents metadata for configuration parameters diff --git a/include/datadog/datadog_agent_config.h b/include/datadog/datadog_agent_config.h index a8a0742b..827a2776 100644 --- a/include/datadog/datadog_agent_config.h +++ b/include/datadog/datadog_agent_config.h @@ -66,6 +66,9 @@ struct DatadogAgentConfig { // How often, in seconds, to query the Datadog Agent for remote configuration // updates. Optional remote_configuration_poll_interval_seconds; + // Whether APM tracing is enabled. This affects whether the + // "Datadog-Client-Computed-Stats: yes" header is sent with trace requests. + Optional apm_tracing_enabled; }; class FinalizedDatadogAgentConfig { @@ -87,6 +90,7 @@ class FinalizedDatadogAgentConfig { std::chrono::steady_clock::duration shutdown_timeout; std::chrono::steady_clock::duration remote_configuration_poll_interval; std::unordered_map metadata; + bool apm_tracing_enabled; // Origin detection Optional admission_controller_uid; diff --git a/include/datadog/environment.h b/include/datadog/environment.h index 65d445f2..680b89af 100644 --- a/include/datadog/environment.h +++ b/include/datadog/environment.h @@ -60,6 +60,7 @@ namespace environment { MACRO(DD_INSTRUMENTATION_INSTALL_ID) \ MACRO(DD_INSTRUMENTATION_INSTALL_TYPE) \ MACRO(DD_INSTRUMENTATION_INSTALL_TIME) \ + MACRO(DD_APM_TRACING_ENABLED) \ MACRO(DD_EXTERNAL_ENV) #define WITH_COMMA(ARG) ARG, diff --git a/include/datadog/injection_options.h b/include/datadog/injection_options.h index f6f24b2d..85812d3e 100644 --- a/include/datadog/injection_options.h +++ b/include/datadog/injection_options.h @@ -7,7 +7,9 @@ namespace datadog { namespace tracing { -struct InjectionOptions {}; +struct InjectionOptions { + bool has_appsec_matches{}; +}; } // namespace tracing } // namespace datadog diff --git a/include/datadog/null_collector.h b/include/datadog/null_collector.h index e541f3db..0a53b660 100644 --- a/include/datadog/null_collector.h +++ b/include/datadog/null_collector.h @@ -11,7 +11,7 @@ namespace tracing { class NullCollector : public Collector { public: Expected send(std::vector>&&, - const std::shared_ptr&) override { + const std::shared_ptr&) override { return {}; } diff --git a/include/datadog/sampling_mechanism.h b/include/datadog/sampling_mechanism.h index 71c99a99..07a65f1c 100644 --- a/include/datadog/sampling_mechanism.h +++ b/include/datadog/sampling_mechanism.h @@ -49,7 +49,7 @@ enum class SamplingMechanism { // The sampling decision was made explicitly by the user, who set a sampling // priority. MANUAL = 4, - // Reserved for future use. + // Trace was kept because of AppSec event. APP_SEC = 5, // Reserved for future use. REMOTE_RATE_USER_DEFINED = 6, diff --git a/include/datadog/trace_segment.h b/include/datadog/trace_segment.h index 4db61f85..1bad7127 100644 --- a/include/datadog/trace_segment.h +++ b/include/datadog/trace_segment.h @@ -53,7 +53,7 @@ class Logger; struct SpanData; struct SpanDefaults; class SpanSampler; -class TraceSampler; +class ErasedTraceSampler; class ConfigManager; class TraceSegment { @@ -61,7 +61,8 @@ class TraceSegment { std::shared_ptr logger_; std::shared_ptr collector_; - std::shared_ptr trace_sampler_; + std::shared_ptr trace_sampler_; + bool apm_tracing_enabled_; std::shared_ptr span_sampler_; std::shared_ptr defaults_; @@ -83,7 +84,8 @@ class TraceSegment { public: TraceSegment(const std::shared_ptr& logger, const std::shared_ptr& collector, - const std::shared_ptr& trace_sampler, + std::shared_ptr trace_sampler, + bool apm_tracing_enabled, const std::shared_ptr& span_sampler, const std::shared_ptr& defaults, const std::shared_ptr& config_manager, diff --git a/include/datadog/tracer.h b/include/datadog/tracer.h index 5c183795..45511c08 100644 --- a/include/datadog/tracer.h +++ b/include/datadog/tracer.h @@ -29,7 +29,7 @@ namespace tracing { class ConfigManager; class DictReader; struct SpanConfig; -class TraceSampler; +class ErasedTraceSampler; class SpanSampler; class IDGenerator; class InMemoryFile; @@ -39,6 +39,8 @@ class Tracer { RuntimeID runtime_id_; TracerSignature signature_; std::shared_ptr config_manager_; + std::shared_ptr + apm_tracing_disabled_sampler_; // empty if enabled std::shared_ptr collector_; std::shared_ptr span_sampler_; std::shared_ptr generator_; @@ -104,6 +106,10 @@ class Tracer { // same JSON object that was logged when this Tracer was created. std::string config() const; + bool is_apm_tracing_enabled() const { + return apm_tracing_disabled_sampler_ == nullptr; + } + private: void store_config(); }; diff --git a/include/datadog/tracer_config.h b/include/datadog/tracer_config.h index ab8608d0..6b50a2f7 100644 --- a/include/datadog/tracer_config.h +++ b/include/datadog/tracer_config.h @@ -28,6 +28,7 @@ class Collector; class Logger; class SpanSampler; class TraceSampler; +class ErasedTraceSampler; struct TracerConfig { // Set the service name. @@ -80,6 +81,16 @@ struct TracerConfig { // `telemetry/configuration.h` By default, the telemetry module is enabled. telemetry::Configuration telemetry; + // `apm_tracing_enabled` indicates whether APM traces and APM trace metrics + // are enabled. If `false`, APM-specific traces are dropped and APM trace + // metrics are not computed. This allows other products (e.g., AppSec) to + // operate independently. + // This is distinct from `report_traces`, which controls whether any traces + // are sent at all. + // `apm_tracing_enabled` is overridden by the `DD_APM_TRACING_ENABLED` + // environment variable. Defaults to `true`. + Optional apm_tracing_enabled; + // `trace_sampler` configures trace sampling. Trace sampling determines which // traces are sent to Datadog. See `trace_sampler_config.h`. TraceSamplerConfig trace_sampler; @@ -197,6 +208,7 @@ class FinalizedTracerConfig final { std::shared_ptr logger; bool log_on_startup; bool generate_128bit_trace_ids; + bool apm_tracing_enabled; Optional runtime_id; Clock clock; std::string integration_name; diff --git a/src/datadog/config_manager.cpp b/src/datadog/config_manager.cpp index 36b054e5..712ec935 100644 --- a/src/datadog/config_manager.cpp +++ b/src/datadog/config_manager.cpp @@ -130,6 +130,8 @@ ConfigManager::ConfigManager(const FinalizedTracerConfig& config) default_metadata_(config.metadata), trace_sampler_( std::make_shared(config.trace_sampler, clock_)), + erased_trace_sampler_( + std::make_shared(trace_sampler_)), rules_(config.trace_sampler.rules), span_defaults_(std::make_shared(config.defaults)), report_traces_(config.report_traces) {} @@ -163,9 +165,9 @@ void ConfigManager::on_revert(const Configuration&) { telemetry::capture_configuration_change(config_metadata); } -std::shared_ptr ConfigManager::trace_sampler() { +std::shared_ptr ConfigManager::trace_sampler() { std::lock_guard lock(mutex_); - return trace_sampler_; + return erased_trace_sampler_; } std::shared_ptr ConfigManager::span_defaults() { diff --git a/src/datadog/config_manager.h b/src/datadog/config_manager.h index a19824a4..e6a6025d 100644 --- a/src/datadog/config_manager.h +++ b/src/datadog/config_manager.h @@ -70,6 +70,9 @@ class ConfigManager : public remote_config::Listener { std::unordered_map default_metadata_; std::shared_ptr trace_sampler_; + // wraps trace_sampler_ and should be changed if trace_sampler_ is changed + // Could be created every time, but that would be a waste + std::shared_ptr erased_trace_sampler_; std::vector rules_; DynamicConfig> span_defaults_; @@ -93,7 +96,7 @@ class ConfigManager : public remote_config::Listener { void on_post_process() override{}; // Return the `TraceSampler` consistent with the most recent configuration. - std::shared_ptr trace_sampler(); + std::shared_ptr trace_sampler(); // Return the `SpanDefaults` consistent with the most recent configuration. std::shared_ptr span_defaults(); diff --git a/src/datadog/datadog_agent.cpp b/src/datadog/datadog_agent.cpp index 6e788ae7..4378010e 100644 --- a/src/datadog/datadog_agent.cpp +++ b/src/datadog/datadog_agent.cpp @@ -157,7 +157,8 @@ DatadogAgent::DatadogAgent( flush_interval_(config.flush_interval), request_timeout_(config.request_timeout), shutdown_timeout_(config.shutdown_timeout), - remote_config_(tracer_signature, rc_listeners, logger) { + remote_config_(tracer_signature, rc_listeners, logger), + apm_tracing_enabled_(config.apm_tracing_enabled) { assert(logger_); // Set HTTP headers @@ -211,7 +212,7 @@ DatadogAgent::~DatadogAgent() { Expected DatadogAgent::send( std::vector>&& spans, - const std::shared_ptr& response_handler) { + const std::shared_ptr& response_handler) { std::lock_guard lock(mutex_); trace_chunks_.push_back(TraceChunk{std::move(spans), response_handler}); return nullopt; @@ -272,7 +273,7 @@ void DatadogAgent::flush() { // One HTTP request to the Agent could possibly involve trace chunks from // multiple tracers, and thus multiple trace samplers might need to have // their rates updated. Unlikely, but possible. - std::unordered_set> response_handlers; + std::unordered_set> response_handlers; for (auto& chunk : trace_chunks) { response_handlers.insert(std::move(chunk.response_handler)); } @@ -284,6 +285,9 @@ void DatadogAgent::flush() { for (const auto& [key, value] : headers_) { writer.set(key, value); } + if (!apm_tracing_enabled_) { + writer.set("Datadog-Client-Computed-Stats", "yes"); + } }; // This is the callback for the HTTP response. It's invoked diff --git a/src/datadog/datadog_agent.h b/src/datadog/datadog_agent.h index fe016c46..701bf122 100644 --- a/src/datadog/datadog_agent.h +++ b/src/datadog/datadog_agent.h @@ -21,17 +21,17 @@ namespace datadog { namespace tracing { +class ErasedTraceSampler; class FinalizedDatadogAgentConfig; class Logger; struct SpanData; -class TraceSampler; struct TracerSignature; class DatadogAgent : public Collector { public: struct TraceChunk { std::vector> spans; - std::shared_ptr response_handler; + std::shared_ptr response_handler; }; private: @@ -51,6 +51,7 @@ class DatadogAgent : public Collector { remote_config::Manager remote_config_; std::unordered_map headers_; + bool apm_tracing_enabled_; void flush(); @@ -63,7 +64,7 @@ class DatadogAgent : public Collector { Expected send( std::vector>&& spans, - const std::shared_ptr& response_handler) override; + const std::shared_ptr& response_handler) override; void get_and_apply_remote_configuration_updates(); diff --git a/src/datadog/datadog_agent_config.cpp b/src/datadog/datadog_agent_config.cpp index 9ffe6217..f0134a1c 100644 --- a/src/datadog/datadog_agent_config.cpp +++ b/src/datadog/datadog_agent_config.cpp @@ -31,6 +31,10 @@ Expected load_datadog_agent_env_config() { env_config.remote_configuration_poll_interval_seconds = *res; } + if (auto apm_enabled_env = lookup(environment::DD_APM_TRACING_ENABLED)) { + env_config.apm_tracing_enabled = !falsy(*apm_enabled_env); + } + auto env_host = lookup(environment::DD_AGENT_HOST); auto env_port = lookup(environment::DD_TRACE_AGENT_PORT); @@ -135,6 +139,9 @@ Expected finalize_config( value_or(env_config->remote_configuration_enabled, user_config.remote_configuration_enabled, true); + result.apm_tracing_enabled = value_or(env_config->apm_tracing_enabled, + user_config.apm_tracing_enabled, true); + const auto [origin, url] = pick(env_config->url, user_config.url, "http://localhost:8126"); auto parsed_url = HTTPClient::URL::parse(url); diff --git a/src/datadog/extraction_util.cpp b/src/datadog/extraction_util.cpp index 0b4965fa..dfeed0bc 100644 --- a/src/datadog/extraction_util.cpp +++ b/src/datadog/extraction_util.cpp @@ -34,7 +34,7 @@ void handle_trace_tags(StringView trace_tags, ExtractedData& result, } for (auto& [key, value] : *maybe_trace_tags) { - if (!starts_with(key, "_dd.p.")) { + if (!starts_with(key, "_dd.p.") && key != "_dd.p.ts") { continue; } diff --git a/src/datadog/tags.cpp b/src/datadog/tags.cpp index 32e1d4d5..63d86f86 100644 --- a/src/datadog/tags.cpp +++ b/src/datadog/tags.cpp @@ -31,6 +31,8 @@ const std::string language = "language"; const std::string runtime_id = "runtime-id"; const std::string sampling_decider = "_dd.is_sampling_decider"; const std::string w3c_parent_id = "_dd.parent_id"; +const std::string trace_source = "_dd.p.ts"; +const std::string apm_enabled = "_dd.apm.enabled"; } // namespace internal diff --git a/src/datadog/tags.h b/src/datadog/tags.h index 5593b26f..52a1af34 100644 --- a/src/datadog/tags.h +++ b/src/datadog/tags.h @@ -39,6 +39,8 @@ extern const std::string language; extern const std::string runtime_id; extern const std::string sampling_decider; extern const std::string w3c_parent_id; +extern const std::string trace_source; // _dd.p.ts +extern const std::string apm_enabled; // _dd.apm.enabled } // namespace internal // Return whether the specified `tag_name` is reserved for use internal to this diff --git a/src/datadog/telemetry/telemetry_impl.cpp b/src/datadog/telemetry/telemetry_impl.cpp index 85a30ecc..3b484d3a 100644 --- a/src/datadog/telemetry/telemetry_impl.cpp +++ b/src/datadog/telemetry/telemetry_impl.cpp @@ -107,6 +107,8 @@ std::string to_string(datadog::tracing::ConfigName name) { return "trace_baggage_max_bytes"; case ConfigName::TRACE_BAGGAGE_MAX_ITEMS: return "trace_baggage_max_items"; + case ConfigName::APM_TRACING_ENABLED: + return "apm_tracing_enabled"; } std::abort(); diff --git a/src/datadog/trace_sampler.cpp b/src/datadog/trace_sampler.cpp index d6f2d8bd..95cb083e 100644 --- a/src/datadog/trace_sampler.cpp +++ b/src/datadog/trace_sampler.cpp @@ -12,6 +12,7 @@ #include "json_serializer.h" #include "sampling_util.h" #include "span_data.h" +#include "tags.h" namespace datadog { namespace tracing { @@ -124,5 +125,95 @@ nlohmann::json TraceSampler::config_json() const { }); } +SamplingDecision ApmDisabledTraceSampler::decide(const SpanData& span_data) { + SamplingDecision decision; + decision.origin = SamplingDecision::Origin::LOCAL; + + if (span_data.tags.find(tags::internal::trace_source) != + span_data.tags.end()) { + decision.mechanism = static_cast(SamplingMechanism::APP_SEC); + decision.priority = static_cast(SamplingPriority::USER_KEEP); + } else { + auto now = clock_(); + auto last_kept = last_kept_.load(std::memory_order_relaxed); + auto num_asked = num_asked_.fetch_add(1, std::memory_order_relaxed) + 1; + uint64_t num_allowed; + if (now.wall - last_kept >= INTERVAL) { + if (last_kept_.compare_exchange_strong(last_kept, now.wall)) { + decision.priority = static_cast(SamplingPriority::USER_KEEP); + num_allowed = num_allowed_.fetch_add(1, std::memory_order_relaxed) + 1; + } else { + // another thread got to it first + decision.priority = static_cast(SamplingPriority::USER_DROP); + num_allowed = num_allowed_.load(std::memory_order_relaxed); + } + } else { + decision.priority = static_cast(SamplingPriority::USER_DROP); + num_allowed = num_allowed_.load(std::memory_order_relaxed); + } + + decision.limiter_max_per_second = ALLOWED_PER_SECOND; + double effective_rate = static_cast(num_allowed) / num_asked; + if (effective_rate > 1.0) { + // can happen due to the relaxed atomic operations + effective_rate = 1.0; + } + decision.limiter_effective_rate = Rate::from(effective_rate).value(); + } + + return decision; +} + +void ApmDisabledTraceSampler::handle_collector_response( + const CollectorResponse&) { + // do nothing +} + +nlohmann::json ApmDisabledTraceSampler::config_json() const { + return nlohmann::json::object({ + {"max_per_second", ALLOWED_PER_SECOND}, + }); +} + +template +struct ErasedTraceSampler::Model : Concept { + Model(Ptr&& samplerImpl) : impl_(std::move(samplerImpl)) {} + + SamplingDecision decide(const SpanData& span_data) override { + return impl_->decide(span_data); + } + + void handle_collector_response(const CollectorResponse& response) override { + impl_->handle_collector_response(response); + } + + nlohmann::json config_json() const override { return impl_->config_json(); } + + private: + Ptr impl_; +}; + +template +ErasedTraceSampler::ErasedTraceSampler(Ptr samplerImpl) { + impl_ = std::make_unique>(std::move(samplerImpl)); +} + +template ErasedTraceSampler::ErasedTraceSampler(std::shared_ptr); +template ErasedTraceSampler::ErasedTraceSampler( + std::unique_ptr); + +SamplingDecision ErasedTraceSampler::decide(const SpanData& span_data) { + return impl_->decide(span_data); +} + +void ErasedTraceSampler::handle_collector_response( + const CollectorResponse& response) { + impl_->handle_collector_response(response); +} + +nlohmann::json ErasedTraceSampler::config_json() const { + return impl_->config_json(); +} + } // namespace tracing } // namespace datadog diff --git a/src/datadog/trace_sampler.h b/src/datadog/trace_sampler.h index 034e6f31..dc936471 100644 --- a/src/datadog/trace_sampler.h +++ b/src/datadog/trace_sampler.h @@ -88,6 +88,7 @@ #include #include +#include #include #include #include @@ -127,5 +128,50 @@ class TraceSampler { nlohmann::json config_json() const; }; +class ApmDisabledTraceSampler { + public: + ApmDisabledTraceSampler(const Clock& clock) : clock_(clock) {} + + SamplingDecision decide(const SpanData& span_data); + void handle_collector_response(const CollectorResponse& response); + nlohmann::json config_json() const; + + private: + static constexpr auto ALLOWED_PER_SECOND = 1.0 / 60.0; + // allow a bit more than the declared ALLOWED_PER_SECOND rate + static constexpr auto INTERVAL = std::chrono::seconds(50); + + // the Limiter is not used, it's difficult to test reliably + Clock clock_; + std::atomic last_kept_{}; + std::atomic num_allowed_{0}; + std::atomic num_asked_{0}; +}; + +/* Erases the actual type implementing the decide function */ +class ErasedTraceSampler { + public: + template + ErasedTraceSampler(Ptr samplerImpl); + + SamplingDecision decide(const SpanData& span_data); + void handle_collector_response(const CollectorResponse& response); + nlohmann::json config_json() const; + + private: + struct Concept { + virtual ~Concept() = default; + virtual SamplingDecision decide(const SpanData& span_data) = 0; + virtual void handle_collector_response( + const CollectorResponse& response) = 0; + virtual nlohmann::json config_json() const = 0; + }; + + template + struct Model; + + std::unique_ptr impl_; +}; + } // namespace tracing } // namespace datadog diff --git a/src/datadog/trace_segment.cpp b/src/datadog/trace_segment.cpp index b7f74c0f..a7504b20 100644 --- a/src/datadog/trace_segment.cpp +++ b/src/datadog/trace_segment.cpp @@ -5,11 +5,13 @@ #include #include #include +#include #include #include #include #include +#include #include #include #include @@ -83,7 +85,7 @@ void inject_trace_tags( TraceSegment::TraceSegment( const std::shared_ptr& logger, const std::shared_ptr& collector, - const std::shared_ptr& trace_sampler, + std::shared_ptr trace_sampler, bool apm_tracing_enabled, const std::shared_ptr& span_sampler, const std::shared_ptr& defaults, const std::shared_ptr& config_manager, @@ -98,7 +100,8 @@ TraceSegment::TraceSegment( std::unique_ptr local_root) : logger_(logger), collector_(collector), - trace_sampler_(trace_sampler), + trace_sampler_(std::move(trace_sampler)), + apm_tracing_enabled_(apm_tracing_enabled), span_sampler_(span_sampler), defaults_(defaults), runtime_id_(runtime_id), @@ -231,6 +234,13 @@ void TraceSegment::span_finished() { local_root.tags[tags::internal::sampling_decider] = "0"; } + // RFC seems to only mandate that this be set if the trace is kept. + // However, system-tests expect this to be always be set. + // Add it all the time; can't hurt + if (!apm_tracing_enabled_) { + local_root.numeric_tags[tags::internal::apm_enabled] = 0; + } + // Some tags are repeated on all spans. for (const auto& span_ptr : spans_) { SpanData& span = *span_ptr; @@ -279,7 +289,23 @@ void TraceSegment::make_sampling_decision_if_null() { return; } - const SpanData& local_root = *spans_.front(); + SpanData& local_root = *spans_.front(); + + // ApmDisabledTraceSampler needs to know the value of _dd.p.ts. Copy it here + // so that the value is accessible in the span passed to decide(). + // The value may have been set because it was propagated from upstream or from + // a WAF run we already did + if (!apm_tracing_enabled_) { + auto it = std::find_if( + trace_tags_.begin(), trace_tags_.end(), + [](const auto& entry) { return entry.first == "_dd.p.ts"; }); + if (it != trace_tags_.end()) { + local_root.tags.insert_or_assign(it->first, it->second); + // don't move the strings or erase the entry from trace_tags_: + // besides filling meta, it may still be in the tags propagation header + } + } + sampling_decision_ = trace_sampler_->decide(local_root); update_decision_maker_trace_tag(); @@ -317,13 +343,17 @@ bool TraceSegment::inject(DictWriter& writer, const SpanData& span) { } bool TraceSegment::inject(DictWriter& writer, const SpanData& span, - const InjectionOptions&) { + const InjectionOptions& inj_opts) { // If the only injection style is `NONE`, then don't do anything. if (injection_styles_.size() == 1 && injection_styles_[0] == PropagationStyle::NONE) { return false; } + if (inj_opts.has_appsec_matches) { + trace_tags_.emplace_back(tags::internal::trace_source, "02"); + } + // The sampling priority can change (it can be overridden on another thread), // and trace tags might change when that happens ("_dd.p.dm"). // So, we lock here, make a sampling decision if necessary, and then copy the @@ -338,6 +368,23 @@ bool TraceSegment::inject(DictWriter& writer, const SpanData& span, trace_tags = trace_tags_; } + // with APM tracing disabled, stop propagation unless we must keep the trace + if (!apm_tracing_enabled_) { + if (sampling_priority != 2) { + writer.set("x-datadog-trace-id", {}); + writer.set("x-datadog-parent-id", {}); + writer.set("x-datadog-sampling-priority", {}); + writer.set("x-datadog-origin", {}); + writer.set("x-datadog-tags", {}); + writer.set("x-b3-traceid", {}); + writer.set("x-b3-spanid", {}); + writer.set("x-b3-sampled", {}); + writer.set("traceparent", {}); + writer.set("tracestate", {}); + return true; + } + } + for (const auto style : injection_styles_) { switch (style) { case PropagationStyle::DATADOG: diff --git a/src/datadog/tracer.cpp b/src/datadog/tracer.cpp index 8849c082..d786deb3 100644 --- a/src/datadog/tracer.cpp +++ b/src/datadog/tracer.cpp @@ -48,6 +48,11 @@ Tracer::Tracer(const FinalizedTracerConfig& config, signature_{runtime_id_, config.defaults.service, config.defaults.environment}, config_manager_(std::make_shared(config)), + apm_tracing_disabled_sampler_( + config.apm_tracing_enabled + ? nullptr + : std::make_shared( + std::make_unique(config.clock))), collector_(/* see constructor body */), span_sampler_( std::make_shared(config.span_sampler, config.clock)), @@ -180,14 +185,22 @@ Span Tracer::create_span(const SpanConfig& config) { hex_padded(span_data->trace_id.high)); } + std::shared_ptr trace_sampler; + if (is_apm_tracing_enabled()) { + trace_sampler = config_manager_->trace_sampler(); + } else { + trace_sampler = apm_tracing_disabled_sampler_; + } + const auto span_data_ptr = span_data.get(); telemetry::counter::increment(metrics::tracer::trace_segments_created, {"new_continued:new"}); const auto segment = std::make_shared( - logger_, collector_, config_manager_->trace_sampler(), span_sampler_, - defaults, config_manager_, runtime_id_, injection_styles_, hostname_, - nullopt /* origin */, tags_header_max_size_, std::move(trace_tags), - nullopt /* sampling_decision */, nullopt /* additional_w3c_tracestate */, + logger_, collector_, std::move(trace_sampler), is_apm_tracing_enabled(), + span_sampler_, defaults, config_manager_, runtime_id_, injection_styles_, + hostname_, nullopt /* origin */, tags_header_max_size_, + std::move(trace_tags), nullopt /* sampling_decision */, + nullopt /* additional_w3c_tracestate */, nullopt /* additional_datadog_w3c_tracestate*/, std::move(span_data)); Span span{span_data_ptr, segment, [generator = generator_]() { return generator->span_id(); }, @@ -378,7 +391,7 @@ Expected Tracer::extract_span(const DictReader& reader, } Optional sampling_decision; - if (merged_context.sampling_priority) { + if (merged_context.sampling_priority && is_apm_tracing_enabled()) { SamplingDecision decision; decision.priority = *merged_context.sampling_priority; // `decision.mechanism` is null. We might be able to infer it once we @@ -388,15 +401,22 @@ Expected Tracer::extract_span(const DictReader& reader, sampling_decision = decision; } + std::shared_ptr trace_sampler; + if (is_apm_tracing_enabled()) { + trace_sampler = config_manager_->trace_sampler(); + } else { + trace_sampler = apm_tracing_disabled_sampler_; + } + const auto span_data_ptr = span_data.get(); telemetry::counter::increment(metrics::tracer::trace_segments_created, {"new_continued:continued"}); const auto segment = std::make_shared( - logger_, collector_, config_manager_->trace_sampler(), span_sampler_, - config_manager_->span_defaults(), config_manager_, runtime_id_, - injection_styles_, hostname_, std::move(merged_context.origin), - tags_header_max_size_, std::move(merged_context.trace_tags), - std::move(sampling_decision), + logger_, collector_, std::move(trace_sampler), is_apm_tracing_enabled(), + span_sampler_, config_manager_->span_defaults(), config_manager_, + runtime_id_, injection_styles_, hostname_, + std::move(merged_context.origin), tags_header_max_size_, + std::move(merged_context.trace_tags), std::move(sampling_decision), std::move(merged_context.additional_w3c_tracestate), std::move(merged_context.additional_datadog_w3c_tracestate), std::move(span_data)); diff --git a/src/datadog/tracer_config.cpp b/src/datadog/tracer_config.cpp index fb8ac247..e7456bf2 100644 --- a/src/datadog/tracer_config.cpp +++ b/src/datadog/tracer_config.cpp @@ -6,6 +6,7 @@ #include #include #include +#include #include #include "datadog_agent.h" @@ -14,6 +15,7 @@ #include "parse_util.h" #include "platform_util.h" #include "string_util.h" +#include "tags.h" #include "threaded_event_scheduler.h" namespace datadog { @@ -128,6 +130,10 @@ Expected load_tracer_env_config(Logger &logger) { env_cfg.generate_128bit_trace_ids = !falsy(*enabled_env); } + if (auto apm_enabled_env = lookup(environment::DD_APM_TRACING_ENABLED)) { + env_cfg.apm_tracing_enabled = !falsy(*apm_enabled_env); + } + // Baggage if (auto baggage_items_env = lookup(environment::DD_TRACE_BAGGAGE_MAX_ITEMS)) { @@ -345,6 +351,13 @@ Expected finalize_config(const TracerConfig &user_config, final_config.metadata[ConfigName::REPORT_TRACES] = ConfigMetadata( ConfigName::REPORT_TRACES, to_string(final_config.report_traces), origin); + // APM Tracing Enabled + std::tie(origin, final_config.apm_tracing_enabled) = pick( + env_config->apm_tracing_enabled, user_config.apm_tracing_enabled, true); + final_config.metadata[ConfigName::APM_TRACING_ENABLED] = + ConfigMetadata(ConfigName::APM_TRACING_ENABLED, + to_string(final_config.apm_tracing_enabled), origin); + // Report hostname final_config.report_hostname = value_or(env_config->report_hostname, user_config.report_hostname, false); diff --git a/src/datadog/tracer_telemetry.cpp b/src/datadog/tracer_telemetry.cpp new file mode 100644 index 00000000..e69de29b diff --git a/test/mocks/collectors.h b/test/mocks/collectors.h index c2f8768a..91043f62 100644 --- a/test/mocks/collectors.h +++ b/test/mocks/collectors.h @@ -22,7 +22,7 @@ struct MockCollector : public Collector { std::vector>> chunks; Expected send(std::vector>&& spans, - const std::shared_ptr&) override { + const std::shared_ptr&) override { chunks.emplace_back(std::move(spans)); return {}; } @@ -51,7 +51,7 @@ struct MockCollectorWithResponse : public MockCollector { Expected send( std::vector>&& spans, - const std::shared_ptr& response_handler) override { + const std::shared_ptr& response_handler) override { MockCollector::send(std::move(spans), response_handler); response_handler->handle_collector_response(response); return {}; @@ -64,7 +64,7 @@ struct PriorityCountingCollector : public Collector { std::map sampling_priority_count; Expected send(std::vector>&& spans, - const std::shared_ptr&) override { + const std::shared_ptr&) override { const SpanData& root = root_span(spans); const auto priority = root.numeric_tags.at(tags::internal::sampling_priority); @@ -127,7 +127,7 @@ struct PriorityCountingCollectorWithResponse Expected send( std::vector>&& spans, - const std::shared_ptr& response_handler) override { + const std::shared_ptr& response_handler) override { PriorityCountingCollector::send(std::move(spans), response_handler); REQUIRE(response_handler); response_handler->handle_collector_response(response); @@ -141,7 +141,7 @@ struct FailureCollector : public Collector { Error failure{Error::OTHER, "send(...) failed because I told it to."}; Expected send(std::vector>&&, - const std::shared_ptr&) override { + const std::shared_ptr&) override { return failure; } diff --git a/test/test_datadog_agent.cpp b/test/test_datadog_agent.cpp index d07fb0fb..4aa3fbaf 100644 --- a/test/test_datadog_agent.cpp +++ b/test/test_datadog_agent.cpp @@ -231,3 +231,58 @@ DATADOG_AGENT_TEST("Remote Configuration") { CHECK(logger->error_count() == 1); } } + +TEST_CASE("Datadog-Client-Computed-Stats header", "[datadog_agent]") { + const auto logger = + std::make_shared(std::cerr, MockLogger::ERRORS_ONLY); + const auto event_scheduler = std::make_shared(); + const auto http_client = std::make_shared(); + + TracerConfig config; + config.service = "testsvc"; + config.logger = logger; + config.agent.event_scheduler = event_scheduler; + config.agent.http_client = http_client; + config.telemetry.enabled = false; + + SECTION("header sent when apm_tracing_enabled is false") { + config.agent.apm_tracing_enabled = false; + auto finalized = finalize_config(config); + REQUIRE(finalized); + + { + http_client->response_status = 200; + http_client->response_body << "{}"; + Tracer tracer{*finalized}; + Span span = tracer.create_span(); + } + + http_client->drain(std::chrono::steady_clock::now()); + + // the header is present + auto header_it = http_client->request_headers.items.find( + "Datadog-Client-Computed-Stats"); + REQUIRE(header_it != http_client->request_headers.items.end()); + REQUIRE(header_it->second == "yes"); + } + + SECTION("header not sent when apm_tracing_enabled is true") { + config.agent.apm_tracing_enabled = true; + auto finalized = finalize_config(config); + REQUIRE(finalized); + + { + http_client->response_status = 200; + http_client->response_body << "{}"; + Tracer tracer{*finalized}; + Span span = tracer.create_span(); + } + + http_client->drain(std::chrono::steady_clock::now()); + + // verify trhat the header is not present + auto header_it = http_client->request_headers.items.find( + "Datadog-Client-Computed-Stats"); + REQUIRE(header_it == http_client->request_headers.items.end()); + } +} diff --git a/test/test_span.cpp b/test/test_span.cpp index e12af2a4..391b7806 100644 --- a/test/test_span.cpp +++ b/test/test_span.cpp @@ -860,3 +860,147 @@ TEST_CASE("128-bit trace ID injection") { REQUIRE(found != writer.items.end()); REQUIRE(found->second == "deadbeefdeadbeefcafebabecafebabe"); } + +TEST_CASE("injection with has_appsec_matches option") { + TracerConfig config; + config.service = "testsvc"; + config.collector = std::make_shared(); + config.logger = std::make_shared(); + config.injection_styles = {PropagationStyle::DATADOG}; + + auto finalized_config = finalize_config(config); + REQUIRE(finalized_config); + Tracer tracer{*finalized_config}; + + SECTION("has_appsec_matches = false (default)") { + auto span = tracer.create_span(); + InjectionOptions options; + options.has_appsec_matches = false; + + MockDictWriter writer; + span.inject(writer, options); + + const auto& headers = writer.items; + // When has_appsec_matches is false, there should be no x-datadog-tags + // header or if there is one, it should not contain _dd.p.ts + if (headers.count("x-datadog-tags") > 0) { + const auto decoded_tags = decode_tags(headers.at("x-datadog-tags")); + REQUIRE(decoded_tags); + auto found = + std::find_if(decoded_tags->begin(), decoded_tags->end(), + [](const auto& tag) { return tag.first == "_dd.p.ts"; }); + REQUIRE(found == decoded_tags->end()); + } + } + + SECTION("has_appsec_matches = true") { + auto span = tracer.create_span(); + InjectionOptions options; + options.has_appsec_matches = true; + + MockDictWriter writer; + span.inject(writer, options); + + const auto& headers = writer.items; + REQUIRE(headers.count("x-datadog-tags") == 1); + + const auto decoded_tags = decode_tags(headers.at("x-datadog-tags")); + REQUIRE(decoded_tags); + + // Check that _dd.p.ts=02 is present in the trace tags + bool found_trace_source = false; + for (const auto& [key, value] : *decoded_tags) { + if (key == "_dd.p.ts" && value == "02") { + found_trace_source = true; + break; + } + } + REQUIRE(found_trace_source); + } + + SECTION("has_appsec_matches with existing trace tags") { + // Extract a span with existing trace tags + const std::unordered_map headers{ + {"x-datadog-trace-id", "123"}, + {"x-datadog-parent-id", "456"}, + {"x-datadog-sampling-priority", "1"}, + {"x-datadog-tags", "_dd.p.existing=value,_dd.p.another=test"}}; + MockDictReader reader{headers}; + auto span = tracer.extract_span(reader); + REQUIRE(span); + + InjectionOptions options; + options.has_appsec_matches = true; + + MockDictWriter writer; + span->inject(writer, options); + + const auto& output_headers = writer.items; + REQUIRE(output_headers.count("x-datadog-tags") == 1); + + const auto decoded_tags = decode_tags(output_headers.at("x-datadog-tags")); + REQUIRE(decoded_tags); + + // Check that _dd.p.ts=02 is present along with existing tags + bool found_trace_source = false; + bool found_existing = false; + bool found_another = false; + + for (const auto& [key, value] : *decoded_tags) { + if (key == "_dd.p.ts" && value == "02") { + found_trace_source = true; + } else if (key == "_dd.p.existing" && value == "value") { + found_existing = true; + } else if (key == "_dd.p.another" && value == "test") { + found_another = true; + } + } + + REQUIRE(found_trace_source); + REQUIRE(found_existing); + REQUIRE(found_another); + } + + SECTION("has_appsec_matches with APM tracing disabled") { + // Test the scenario where APM tracing is disabled + TracerConfig apm_disabled_config; + apm_disabled_config.service = "testsvc"; + apm_disabled_config.collector = std::make_shared(); + apm_disabled_config.logger = std::make_shared(); + apm_disabled_config.injection_styles = { + PropagationStyle::DATADOG, PropagationStyle::B3, PropagationStyle::W3C}; + // Disable APM tracing + apm_disabled_config.apm_tracing_enabled = false; + + auto apm_disabled_finalized_config = finalize_config(apm_disabled_config); + REQUIRE(apm_disabled_finalized_config); + Tracer apm_disabled_tracer{*apm_disabled_finalized_config}; + + SECTION( + "sampling priority != 2 (USER-KEEP) and no _dd.p.ts - headers should " + "be cleared") { + auto span = apm_disabled_tracer.create_span(); + span.trace_segment().override_sampling_priority(1); + + InjectionOptions options; + options.has_appsec_matches = true; + + MockDictWriter writer; + span.inject(writer, options); + + const auto& headers = writer.items; + // all headers should be empty when APM tracing is disabled and sampling + // priority != 2 + REQUIRE(headers.at("x-datadog-trace-id").empty()); + REQUIRE(headers.at("x-datadog-parent-id").empty()); + REQUIRE(headers.at("x-datadog-sampling-priority").empty()); + REQUIRE(headers.at("x-datadog-origin").empty()); + REQUIRE(headers.at("x-datadog-tags").empty()); + REQUIRE(headers.at("x-b3-traceid").empty()); + REQUIRE(headers.at("x-b3-spanid").empty()); + REQUIRE(headers.at("x-b3-sampled").empty()); + REQUIRE(headers.at("traceparent").empty()); + REQUIRE(headers.at("tracestate").empty()); + } + } +} diff --git a/test/test_tracer.cpp b/test/test_tracer.cpp index 48dda9d1..8c994034 100644 --- a/test/test_tracer.cpp +++ b/test/test_tracer.cpp @@ -1746,3 +1746,137 @@ TEST_CASE("move semantics") { Tracer tracer2{std::move(tracer1)}; (void)tracer2; } + +TEST_CASE("apm_tracing_enabled behavior") { + TracerConfig base_config; + base_config.service = "testsvc"; + base_config.name = "test.op"; + auto collector = std::make_shared(); + base_config.collector = collector; + base_config.logger = std::make_shared(); + + TimePoint current_time = default_clock(); + auto clock = [¤t_time]() { return current_time; }; + + SECTION( + "APM tracing disabled - span with _dd.p.ts is kept, _dd.apm.enabled tag " + "added") { + TracerConfig config = base_config; + config.apm_tracing_enabled = false; + + auto finalized_config = finalize_config(config, clock); + REQUIRE(finalized_config); + REQUIRE(!finalized_config->apm_tracing_enabled); + Tracer tracer{*finalized_config}; + + SpanConfig span_cfg; + span_cfg.tags[tags::internal::trace_source] = "02"; + { tracer.create_span(span_cfg); } + + REQUIRE(collector->chunks.size() == 1); + REQUIRE(collector->chunks.front().size() == 1); + const datadog::tracing::SpanData& span_data = + *collector->chunks.front().front(); + + REQUIRE(span_data.tags.at("_dd.p.dm") == "-5"); + REQUIRE(span_data.numeric_tags.at(tags::internal::apm_enabled) == 0); + REQUIRE(span_data.numeric_tags.at(tags::internal::sampling_priority) == 2); + } + + SECTION("APM tracing disabled - no _dd.p.ts - rate limited to 1/min") { + TracerConfig config = base_config; + config.apm_tracing_enabled = false; + + auto finalized_config = finalize_config(config, clock); + REQUIRE(finalized_config); + Tracer tracer{*finalized_config}; + { auto root1 = tracer.create_span(); } + REQUIRE(collector->chunks.size() == 1); + REQUIRE(collector->chunks.front().size() == 1); + const datadog::tracing::SpanData& span1_data = + *collector->chunks.front().front(); + CHECK(span1_data.numeric_tags.at(tags::internal::sampling_priority) == 2); + CHECK(span1_data.numeric_tags.at(tags::internal::apm_enabled) == 0); + CHECK(std::stoi(span1_data.tags.at("_dd.p.dm")) < -10); + + collector->chunks.clear(); + + current_time += + std::chrono::seconds(1); // Advance clock a bit, still within 1 min + { auto root2 = tracer.create_span(); } + REQUIRE(collector->chunks.size() == 1); + REQUIRE(collector->chunks.front().size() == 1); + const datadog::tracing::SpanData& span2_data = + *collector->chunks.front().front(); + CHECK(span2_data.numeric_tags.at(tags::internal::sampling_priority) == -1); + CHECK(span2_data.numeric_tags.at(tags::internal::apm_enabled) == 0); + + collector->chunks.clear(); + + current_time += std::chrono::minutes(1) + + std::chrono::seconds(1); // Advance clock past 1 min + { auto root3 = tracer.create_span(); } + REQUIRE(collector->chunks.size() == 1); + REQUIRE(collector->chunks.front().size() == 1); + const auto& span3_data = *collector->chunks.front().front(); + CHECK(span2_data.numeric_tags.at(tags::internal::sampling_priority) == 2); + CHECK(span3_data.numeric_tags.at(tags::internal::apm_enabled) == 0); + } + + SECTION("APM tracing disabled - extracted context behavior") { + TracerConfig config = base_config; + config.apm_tracing_enabled = false; + + auto finalized_config = finalize_config(config, clock); + REQUIRE(finalized_config); + Tracer tracer{*finalized_config}; + + // Case 1: extracted context with priority, but no _dd.p.ts → dropped + // To ensure this, let's make this the *second* span in this disabled state + // for this test section. The first consumes the limiter slot. + { auto dummy_span = tracer.create_span(); } + collector->chunks.clear(); + + const std::unordered_map headers_with_priority{ + {"x-datadog-trace-id", "123"}, + {"x-datadog-parent-id", "456"}, + {"x-datadog-sampling-priority", "2"} // USER_KEEP + }; + MockDictReader reader_with_priority{headers_with_priority}; + { + auto span = tracer.extract_span(reader_with_priority); + REQUIRE(span); + } + REQUIRE(collector->chunks.size() == 1); + REQUIRE(collector->chunks.front().size() == 1); + const SpanData& span1_data = *collector->chunks.front().front(); + // although incoming priority was USER_KEEP, we should still drop it + CHECK(span1_data.numeric_tags.at(tags::internal::sampling_priority) == -1); + CHECK(span1_data.numeric_tags.at(tags::internal::apm_enabled) == 0.); + + collector->chunks.clear(); + + // Case 2: Extracted context with priority AND _dd.p.ts -> Kept by AppSec + // rule + current_time += + std::chrono::minutes(1) + std::chrono::seconds(1); // Refresh limiter + const std::unordered_map + headers_with_priority_and_appsec{ + {"x-datadog-trace-id", "789"}, + {"x-datadog-parent-id", "101"}, + {"x-datadog-sampling-priority", + "-1"}, // USER_DROP, to show _dd.p.ts overrides + {tags::internal::trace_source, "02"}}; + MockDictReader reader_with_priority_and_appsec{ + headers_with_priority_and_appsec}; + { + auto span = tracer.extract_span(reader_with_priority_and_appsec); + REQUIRE(span); + } + REQUIRE(collector->chunks.size() == 1); + REQUIRE(collector->chunks.front().size() == 1); + const SpanData& span2_data = *collector->chunks.front().front(); + CHECK(span2_data.numeric_tags.at(tags::internal::sampling_priority) == 2); + CHECK(span2_data.numeric_tags.at(tags::internal::apm_enabled) == 0); + } +} diff --git a/test/test_tracer_config.cpp b/test/test_tracer_config.cpp index 4fc4b488..21476290 100644 --- a/test/test_tracer_config.cpp +++ b/test/test_tracer_config.cpp @@ -272,6 +272,25 @@ TRACER_CONFIG_TEST("TracerConfig::log_on_startup") { } } +TRACER_CONFIG_TEST("DD_APM_TRACING_ENABLED") { + TracerConfig config; + config.service = "testsvc"; // Required for finalize_config + + SECTION("default is true") { + const EnvGuard guard{"DD_APM_TRACING_ENABLED", ""}; + auto finalized = finalize_config(config); + REQUIRE(finalized); + REQUIRE(finalized->apm_tracing_enabled); + } + + SECTION("can be set to false") { + const EnvGuard guard{"DD_APM_TRACING_ENABLED", "false"}; + auto finalized = finalize_config(config); + REQUIRE(finalized); + REQUIRE(!finalized->apm_tracing_enabled); + } +} + TRACER_CONFIG_TEST("TracerConfig::report_traces") { TracerConfig config; config.service = "testsvc"; From be9b39fad21e9cf3ac177800495823acc08364d7 Mon Sep 17 00:00:00 2001 From: Gustavo Lopes Date: Tue, 3 Jun 2025 16:58:22 +0100 Subject: [PATCH 2/6] Set limiter_max_per_second and limiter_effective_rate on appsec traces --- src/datadog/trace_sampler.cpp | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/src/datadog/trace_sampler.cpp b/src/datadog/trace_sampler.cpp index 95cb083e..a672620c 100644 --- a/src/datadog/trace_sampler.cpp +++ b/src/datadog/trace_sampler.cpp @@ -129,15 +129,16 @@ SamplingDecision ApmDisabledTraceSampler::decide(const SpanData& span_data) { SamplingDecision decision; decision.origin = SamplingDecision::Origin::LOCAL; + auto now = clock_(); + uint64_t num_allowed; if (span_data.tags.find(tags::internal::trace_source) != span_data.tags.end()) { decision.mechanism = static_cast(SamplingMechanism::APP_SEC); decision.priority = static_cast(SamplingPriority::USER_KEEP); + last_kept_.store(now.wall, std::memory_order_relaxed); + num_allowed = num_allowed_.fetch_add(1, std::memory_order_relaxed) + 1; } else { - auto now = clock_(); auto last_kept = last_kept_.load(std::memory_order_relaxed); - auto num_asked = num_asked_.fetch_add(1, std::memory_order_relaxed) + 1; - uint64_t num_allowed; if (now.wall - last_kept >= INTERVAL) { if (last_kept_.compare_exchange_strong(last_kept, now.wall)) { decision.priority = static_cast(SamplingPriority::USER_KEEP); @@ -151,15 +152,16 @@ SamplingDecision ApmDisabledTraceSampler::decide(const SpanData& span_data) { decision.priority = static_cast(SamplingPriority::USER_DROP); num_allowed = num_allowed_.load(std::memory_order_relaxed); } + } - decision.limiter_max_per_second = ALLOWED_PER_SECOND; - double effective_rate = static_cast(num_allowed) / num_asked; - if (effective_rate > 1.0) { - // can happen due to the relaxed atomic operations - effective_rate = 1.0; - } - decision.limiter_effective_rate = Rate::from(effective_rate).value(); + auto num_asked = num_asked_.fetch_add(1, std::memory_order_relaxed) + 1; + decision.limiter_max_per_second = ALLOWED_PER_SECOND; + double effective_rate = static_cast(num_allowed) / num_asked; + if (effective_rate > 1.0) { + // can happen due to the relaxed atomic operations + effective_rate = 1.0; } + decision.limiter_effective_rate = Rate::from(effective_rate).value(); return decision; } From 36f02506a15e826fdbfd8c05fdd4ab476f0e9275 Mon Sep 17 00:00:00 2001 From: Gustavo Lopes Date: Tue, 3 Jun 2025 17:22:54 +0100 Subject: [PATCH 3/6] Rename the new injection option --- include/datadog/injection_options.h | 9 ++++++++- src/datadog/trace_segment.cpp | 7 +++++-- test/test_span.cpp | 20 ++++++++++---------- 3 files changed, 23 insertions(+), 13 deletions(-) diff --git a/include/datadog/injection_options.h b/include/datadog/injection_options.h index 85812d3e..6890794a 100644 --- a/include/datadog/injection_options.h +++ b/include/datadog/injection_options.h @@ -4,11 +4,18 @@ // parameters to `Span::inject` that alter the behavior of trace context // propagation. +#include + +#include "optional.h" + namespace datadog { namespace tracing { struct InjectionOptions { - bool has_appsec_matches{}; + // If DD_APM_TRACING_ENABLED=false and what we're injecting is not an APM + // trace, then the code for the trace source (e.g. 02 for Appsec) can be + // set here. + Optional> trace_source{}; }; } // namespace tracing diff --git a/src/datadog/trace_segment.cpp b/src/datadog/trace_segment.cpp index a7504b20..55b8ab07 100644 --- a/src/datadog/trace_segment.cpp +++ b/src/datadog/trace_segment.cpp @@ -350,8 +350,11 @@ bool TraceSegment::inject(DictWriter& writer, const SpanData& span, return false; } - if (inj_opts.has_appsec_matches) { - trace_tags_.emplace_back(tags::internal::trace_source, "02"); + if (inj_opts.trace_source) { + std::string trace_source = std::string(inj_opts.trace_source->begin(), + inj_opts.trace_source->end()); + trace_tags_.emplace_back(tags::internal::trace_source, + std::move(trace_source)); } // The sampling priority can change (it can be overridden on another thread), diff --git a/test/test_span.cpp b/test/test_span.cpp index 391b7806..9672397f 100644 --- a/test/test_span.cpp +++ b/test/test_span.cpp @@ -861,7 +861,7 @@ TEST_CASE("128-bit trace ID injection") { REQUIRE(found->second == "deadbeefdeadbeefcafebabecafebabe"); } -TEST_CASE("injection with has_appsec_matches option") { +TEST_CASE("injection with trace_source option") { TracerConfig config; config.service = "testsvc"; config.collector = std::make_shared(); @@ -872,16 +872,16 @@ TEST_CASE("injection with has_appsec_matches option") { REQUIRE(finalized_config); Tracer tracer{*finalized_config}; - SECTION("has_appsec_matches = false (default)") { + SECTION("trace_source is not set (default)") { auto span = tracer.create_span(); InjectionOptions options; - options.has_appsec_matches = false; + options.trace_source = std::nullopt; MockDictWriter writer; span.inject(writer, options); const auto& headers = writer.items; - // When has_appsec_matches is false, there should be no x-datadog-tags + // When there is no trace source, there should be no x-datadog-tags // header or if there is one, it should not contain _dd.p.ts if (headers.count("x-datadog-tags") > 0) { const auto decoded_tags = decode_tags(headers.at("x-datadog-tags")); @@ -893,10 +893,10 @@ TEST_CASE("injection with has_appsec_matches option") { } } - SECTION("has_appsec_matches = true") { + SECTION("trace_source is 02 (appsec)") { auto span = tracer.create_span(); InjectionOptions options; - options.has_appsec_matches = true; + options.trace_source = {'0', '2'}; MockDictWriter writer; span.inject(writer, options); @@ -918,7 +918,7 @@ TEST_CASE("injection with has_appsec_matches option") { REQUIRE(found_trace_source); } - SECTION("has_appsec_matches with existing trace tags") { + SECTION("trace source is 02 (appsec) with existing trace tags") { // Extract a span with existing trace tags const std::unordered_map headers{ {"x-datadog-trace-id", "123"}, @@ -930,7 +930,7 @@ TEST_CASE("injection with has_appsec_matches option") { REQUIRE(span); InjectionOptions options; - options.has_appsec_matches = true; + options.trace_source = {'0', '2'}; MockDictWriter writer; span->inject(writer, options); @@ -961,7 +961,7 @@ TEST_CASE("injection with has_appsec_matches option") { REQUIRE(found_another); } - SECTION("has_appsec_matches with APM tracing disabled") { + SECTION("trace_source with APM tracing disabled") { // Test the scenario where APM tracing is disabled TracerConfig apm_disabled_config; apm_disabled_config.service = "testsvc"; @@ -983,7 +983,7 @@ TEST_CASE("injection with has_appsec_matches option") { span.trace_segment().override_sampling_priority(1); InjectionOptions options; - options.has_appsec_matches = true; + options.trace_source = {'0', '2'}; MockDictWriter writer; span.inject(writer, options); From 7a3a6c511c84ad2835423223e94634b2b6724ff8 Mon Sep 17 00:00:00 2001 From: Damien Mehala Date: Thu, 19 Jun 2025 20:07:05 +0200 Subject: [PATCH 4/6] [skip ci] remove unecessary changes --- include/datadog/collector.h | 4 +- include/datadog/datadog_agent_config.h | 4 -- include/datadog/null_collector.h | 2 +- include/datadog/trace_segment.h | 8 +-- include/datadog/tracer.h | 8 +-- include/datadog/tracer_config.h | 16 ++--- src/datadog/config_manager.cpp | 6 +- src/datadog/config_manager.h | 5 +- src/datadog/datadog_agent.cpp | 10 +-- src/datadog/datadog_agent.h | 7 +- src/datadog/datadog_agent_config.cpp | 7 -- src/datadog/trace_sampler.cpp | 93 -------------------------- src/datadog/trace_sampler.h | 46 ------------- src/datadog/trace_segment.cpp | 58 ++-------------- src/datadog/tracer.cpp | 40 +++-------- test/mocks/collectors.h | 10 +-- test/test_datadog_agent.cpp | 4 +- 17 files changed, 41 insertions(+), 287 deletions(-) diff --git a/include/datadog/collector.h b/include/datadog/collector.h index 4c8dbe75..96f9e64d 100644 --- a/include/datadog/collector.h +++ b/include/datadog/collector.h @@ -19,7 +19,7 @@ namespace datadog { namespace tracing { struct SpanData; -class ErasedTraceSampler; +class TraceSampler; class Collector { public: @@ -29,7 +29,7 @@ class Collector { // occurs. virtual Expected send( std::vector>&& spans, - const std::shared_ptr& response_handler) = 0; + const std::shared_ptr& response_handler) = 0; // Return a JSON representation of this object's configuration. The JSON // representation is an object with the following properties: diff --git a/include/datadog/datadog_agent_config.h b/include/datadog/datadog_agent_config.h index 827a2776..a8a0742b 100644 --- a/include/datadog/datadog_agent_config.h +++ b/include/datadog/datadog_agent_config.h @@ -66,9 +66,6 @@ struct DatadogAgentConfig { // How often, in seconds, to query the Datadog Agent for remote configuration // updates. Optional remote_configuration_poll_interval_seconds; - // Whether APM tracing is enabled. This affects whether the - // "Datadog-Client-Computed-Stats: yes" header is sent with trace requests. - Optional apm_tracing_enabled; }; class FinalizedDatadogAgentConfig { @@ -90,7 +87,6 @@ class FinalizedDatadogAgentConfig { std::chrono::steady_clock::duration shutdown_timeout; std::chrono::steady_clock::duration remote_configuration_poll_interval; std::unordered_map metadata; - bool apm_tracing_enabled; // Origin detection Optional admission_controller_uid; diff --git a/include/datadog/null_collector.h b/include/datadog/null_collector.h index 0a53b660..e541f3db 100644 --- a/include/datadog/null_collector.h +++ b/include/datadog/null_collector.h @@ -11,7 +11,7 @@ namespace tracing { class NullCollector : public Collector { public: Expected send(std::vector>&&, - const std::shared_ptr&) override { + const std::shared_ptr&) override { return {}; } diff --git a/include/datadog/trace_segment.h b/include/datadog/trace_segment.h index 1bad7127..4db61f85 100644 --- a/include/datadog/trace_segment.h +++ b/include/datadog/trace_segment.h @@ -53,7 +53,7 @@ class Logger; struct SpanData; struct SpanDefaults; class SpanSampler; -class ErasedTraceSampler; +class TraceSampler; class ConfigManager; class TraceSegment { @@ -61,8 +61,7 @@ class TraceSegment { std::shared_ptr logger_; std::shared_ptr collector_; - std::shared_ptr trace_sampler_; - bool apm_tracing_enabled_; + std::shared_ptr trace_sampler_; std::shared_ptr span_sampler_; std::shared_ptr defaults_; @@ -84,8 +83,7 @@ class TraceSegment { public: TraceSegment(const std::shared_ptr& logger, const std::shared_ptr& collector, - std::shared_ptr trace_sampler, - bool apm_tracing_enabled, + const std::shared_ptr& trace_sampler, const std::shared_ptr& span_sampler, const std::shared_ptr& defaults, const std::shared_ptr& config_manager, diff --git a/include/datadog/tracer.h b/include/datadog/tracer.h index 45511c08..5c183795 100644 --- a/include/datadog/tracer.h +++ b/include/datadog/tracer.h @@ -29,7 +29,7 @@ namespace tracing { class ConfigManager; class DictReader; struct SpanConfig; -class ErasedTraceSampler; +class TraceSampler; class SpanSampler; class IDGenerator; class InMemoryFile; @@ -39,8 +39,6 @@ class Tracer { RuntimeID runtime_id_; TracerSignature signature_; std::shared_ptr config_manager_; - std::shared_ptr - apm_tracing_disabled_sampler_; // empty if enabled std::shared_ptr collector_; std::shared_ptr span_sampler_; std::shared_ptr generator_; @@ -106,10 +104,6 @@ class Tracer { // same JSON object that was logged when this Tracer was created. std::string config() const; - bool is_apm_tracing_enabled() const { - return apm_tracing_disabled_sampler_ == nullptr; - } - private: void store_config(); }; diff --git a/include/datadog/tracer_config.h b/include/datadog/tracer_config.h index 6b50a2f7..05ac8c78 100644 --- a/include/datadog/tracer_config.h +++ b/include/datadog/tracer_config.h @@ -28,7 +28,6 @@ class Collector; class Logger; class SpanSampler; class TraceSampler; -class ErasedTraceSampler; struct TracerConfig { // Set the service name. @@ -81,16 +80,6 @@ struct TracerConfig { // `telemetry/configuration.h` By default, the telemetry module is enabled. telemetry::Configuration telemetry; - // `apm_tracing_enabled` indicates whether APM traces and APM trace metrics - // are enabled. If `false`, APM-specific traces are dropped and APM trace - // metrics are not computed. This allows other products (e.g., AppSec) to - // operate independently. - // This is distinct from `report_traces`, which controls whether any traces - // are sent at all. - // `apm_tracing_enabled` is overridden by the `DD_APM_TRACING_ENABLED` - // environment variable. Defaults to `true`. - Optional apm_tracing_enabled; - // `trace_sampler` configures trace sampling. Trace sampling determines which // traces are sent to Datadog. See `trace_sampler_config.h`. TraceSamplerConfig trace_sampler; @@ -179,6 +168,9 @@ struct TracerConfig { /// By default, it uses `ThreadedEventScheduler`, which runs tasks on a /// separate thread. std::shared_ptr event_scheduler; + + /// TBD + Optional apm_tracing_enabled; }; // `FinalizedTracerConfig` contains `Tracer` implementation details derived from @@ -208,7 +200,6 @@ class FinalizedTracerConfig final { std::shared_ptr logger; bool log_on_startup; bool generate_128bit_trace_ids; - bool apm_tracing_enabled; Optional runtime_id; Clock clock; std::string integration_name; @@ -219,6 +210,7 @@ class FinalizedTracerConfig final { HTTPClient::URL agent_url; std::shared_ptr event_scheduler; std::shared_ptr http_client; + bool apm_tracing_enabled; }; // Return a `FinalizedTracerConfig` from the specified `config` and from any diff --git a/src/datadog/config_manager.cpp b/src/datadog/config_manager.cpp index 712ec935..36b054e5 100644 --- a/src/datadog/config_manager.cpp +++ b/src/datadog/config_manager.cpp @@ -130,8 +130,6 @@ ConfigManager::ConfigManager(const FinalizedTracerConfig& config) default_metadata_(config.metadata), trace_sampler_( std::make_shared(config.trace_sampler, clock_)), - erased_trace_sampler_( - std::make_shared(trace_sampler_)), rules_(config.trace_sampler.rules), span_defaults_(std::make_shared(config.defaults)), report_traces_(config.report_traces) {} @@ -165,9 +163,9 @@ void ConfigManager::on_revert(const Configuration&) { telemetry::capture_configuration_change(config_metadata); } -std::shared_ptr ConfigManager::trace_sampler() { +std::shared_ptr ConfigManager::trace_sampler() { std::lock_guard lock(mutex_); - return erased_trace_sampler_; + return trace_sampler_; } std::shared_ptr ConfigManager::span_defaults() { diff --git a/src/datadog/config_manager.h b/src/datadog/config_manager.h index e6a6025d..a19824a4 100644 --- a/src/datadog/config_manager.h +++ b/src/datadog/config_manager.h @@ -70,9 +70,6 @@ class ConfigManager : public remote_config::Listener { std::unordered_map default_metadata_; std::shared_ptr trace_sampler_; - // wraps trace_sampler_ and should be changed if trace_sampler_ is changed - // Could be created every time, but that would be a waste - std::shared_ptr erased_trace_sampler_; std::vector rules_; DynamicConfig> span_defaults_; @@ -96,7 +93,7 @@ class ConfigManager : public remote_config::Listener { void on_post_process() override{}; // Return the `TraceSampler` consistent with the most recent configuration. - std::shared_ptr trace_sampler(); + std::shared_ptr trace_sampler(); // Return the `SpanDefaults` consistent with the most recent configuration. std::shared_ptr span_defaults(); diff --git a/src/datadog/datadog_agent.cpp b/src/datadog/datadog_agent.cpp index 4378010e..6e788ae7 100644 --- a/src/datadog/datadog_agent.cpp +++ b/src/datadog/datadog_agent.cpp @@ -157,8 +157,7 @@ DatadogAgent::DatadogAgent( flush_interval_(config.flush_interval), request_timeout_(config.request_timeout), shutdown_timeout_(config.shutdown_timeout), - remote_config_(tracer_signature, rc_listeners, logger), - apm_tracing_enabled_(config.apm_tracing_enabled) { + remote_config_(tracer_signature, rc_listeners, logger) { assert(logger_); // Set HTTP headers @@ -212,7 +211,7 @@ DatadogAgent::~DatadogAgent() { Expected DatadogAgent::send( std::vector>&& spans, - const std::shared_ptr& response_handler) { + const std::shared_ptr& response_handler) { std::lock_guard lock(mutex_); trace_chunks_.push_back(TraceChunk{std::move(spans), response_handler}); return nullopt; @@ -273,7 +272,7 @@ void DatadogAgent::flush() { // One HTTP request to the Agent could possibly involve trace chunks from // multiple tracers, and thus multiple trace samplers might need to have // their rates updated. Unlikely, but possible. - std::unordered_set> response_handlers; + std::unordered_set> response_handlers; for (auto& chunk : trace_chunks) { response_handlers.insert(std::move(chunk.response_handler)); } @@ -285,9 +284,6 @@ void DatadogAgent::flush() { for (const auto& [key, value] : headers_) { writer.set(key, value); } - if (!apm_tracing_enabled_) { - writer.set("Datadog-Client-Computed-Stats", "yes"); - } }; // This is the callback for the HTTP response. It's invoked diff --git a/src/datadog/datadog_agent.h b/src/datadog/datadog_agent.h index 701bf122..fe016c46 100644 --- a/src/datadog/datadog_agent.h +++ b/src/datadog/datadog_agent.h @@ -21,17 +21,17 @@ namespace datadog { namespace tracing { -class ErasedTraceSampler; class FinalizedDatadogAgentConfig; class Logger; struct SpanData; +class TraceSampler; struct TracerSignature; class DatadogAgent : public Collector { public: struct TraceChunk { std::vector> spans; - std::shared_ptr response_handler; + std::shared_ptr response_handler; }; private: @@ -51,7 +51,6 @@ class DatadogAgent : public Collector { remote_config::Manager remote_config_; std::unordered_map headers_; - bool apm_tracing_enabled_; void flush(); @@ -64,7 +63,7 @@ class DatadogAgent : public Collector { Expected send( std::vector>&& spans, - const std::shared_ptr& response_handler) override; + const std::shared_ptr& response_handler) override; void get_and_apply_remote_configuration_updates(); diff --git a/src/datadog/datadog_agent_config.cpp b/src/datadog/datadog_agent_config.cpp index f0134a1c..9ffe6217 100644 --- a/src/datadog/datadog_agent_config.cpp +++ b/src/datadog/datadog_agent_config.cpp @@ -31,10 +31,6 @@ Expected load_datadog_agent_env_config() { env_config.remote_configuration_poll_interval_seconds = *res; } - if (auto apm_enabled_env = lookup(environment::DD_APM_TRACING_ENABLED)) { - env_config.apm_tracing_enabled = !falsy(*apm_enabled_env); - } - auto env_host = lookup(environment::DD_AGENT_HOST); auto env_port = lookup(environment::DD_TRACE_AGENT_PORT); @@ -139,9 +135,6 @@ Expected finalize_config( value_or(env_config->remote_configuration_enabled, user_config.remote_configuration_enabled, true); - result.apm_tracing_enabled = value_or(env_config->apm_tracing_enabled, - user_config.apm_tracing_enabled, true); - const auto [origin, url] = pick(env_config->url, user_config.url, "http://localhost:8126"); auto parsed_url = HTTPClient::URL::parse(url); diff --git a/src/datadog/trace_sampler.cpp b/src/datadog/trace_sampler.cpp index a672620c..d6f2d8bd 100644 --- a/src/datadog/trace_sampler.cpp +++ b/src/datadog/trace_sampler.cpp @@ -12,7 +12,6 @@ #include "json_serializer.h" #include "sampling_util.h" #include "span_data.h" -#include "tags.h" namespace datadog { namespace tracing { @@ -125,97 +124,5 @@ nlohmann::json TraceSampler::config_json() const { }); } -SamplingDecision ApmDisabledTraceSampler::decide(const SpanData& span_data) { - SamplingDecision decision; - decision.origin = SamplingDecision::Origin::LOCAL; - - auto now = clock_(); - uint64_t num_allowed; - if (span_data.tags.find(tags::internal::trace_source) != - span_data.tags.end()) { - decision.mechanism = static_cast(SamplingMechanism::APP_SEC); - decision.priority = static_cast(SamplingPriority::USER_KEEP); - last_kept_.store(now.wall, std::memory_order_relaxed); - num_allowed = num_allowed_.fetch_add(1, std::memory_order_relaxed) + 1; - } else { - auto last_kept = last_kept_.load(std::memory_order_relaxed); - if (now.wall - last_kept >= INTERVAL) { - if (last_kept_.compare_exchange_strong(last_kept, now.wall)) { - decision.priority = static_cast(SamplingPriority::USER_KEEP); - num_allowed = num_allowed_.fetch_add(1, std::memory_order_relaxed) + 1; - } else { - // another thread got to it first - decision.priority = static_cast(SamplingPriority::USER_DROP); - num_allowed = num_allowed_.load(std::memory_order_relaxed); - } - } else { - decision.priority = static_cast(SamplingPriority::USER_DROP); - num_allowed = num_allowed_.load(std::memory_order_relaxed); - } - } - - auto num_asked = num_asked_.fetch_add(1, std::memory_order_relaxed) + 1; - decision.limiter_max_per_second = ALLOWED_PER_SECOND; - double effective_rate = static_cast(num_allowed) / num_asked; - if (effective_rate > 1.0) { - // can happen due to the relaxed atomic operations - effective_rate = 1.0; - } - decision.limiter_effective_rate = Rate::from(effective_rate).value(); - - return decision; -} - -void ApmDisabledTraceSampler::handle_collector_response( - const CollectorResponse&) { - // do nothing -} - -nlohmann::json ApmDisabledTraceSampler::config_json() const { - return nlohmann::json::object({ - {"max_per_second", ALLOWED_PER_SECOND}, - }); -} - -template -struct ErasedTraceSampler::Model : Concept { - Model(Ptr&& samplerImpl) : impl_(std::move(samplerImpl)) {} - - SamplingDecision decide(const SpanData& span_data) override { - return impl_->decide(span_data); - } - - void handle_collector_response(const CollectorResponse& response) override { - impl_->handle_collector_response(response); - } - - nlohmann::json config_json() const override { return impl_->config_json(); } - - private: - Ptr impl_; -}; - -template -ErasedTraceSampler::ErasedTraceSampler(Ptr samplerImpl) { - impl_ = std::make_unique>(std::move(samplerImpl)); -} - -template ErasedTraceSampler::ErasedTraceSampler(std::shared_ptr); -template ErasedTraceSampler::ErasedTraceSampler( - std::unique_ptr); - -SamplingDecision ErasedTraceSampler::decide(const SpanData& span_data) { - return impl_->decide(span_data); -} - -void ErasedTraceSampler::handle_collector_response( - const CollectorResponse& response) { - impl_->handle_collector_response(response); -} - -nlohmann::json ErasedTraceSampler::config_json() const { - return impl_->config_json(); -} - } // namespace tracing } // namespace datadog diff --git a/src/datadog/trace_sampler.h b/src/datadog/trace_sampler.h index dc936471..034e6f31 100644 --- a/src/datadog/trace_sampler.h +++ b/src/datadog/trace_sampler.h @@ -88,7 +88,6 @@ #include #include -#include #include #include #include @@ -128,50 +127,5 @@ class TraceSampler { nlohmann::json config_json() const; }; -class ApmDisabledTraceSampler { - public: - ApmDisabledTraceSampler(const Clock& clock) : clock_(clock) {} - - SamplingDecision decide(const SpanData& span_data); - void handle_collector_response(const CollectorResponse& response); - nlohmann::json config_json() const; - - private: - static constexpr auto ALLOWED_PER_SECOND = 1.0 / 60.0; - // allow a bit more than the declared ALLOWED_PER_SECOND rate - static constexpr auto INTERVAL = std::chrono::seconds(50); - - // the Limiter is not used, it's difficult to test reliably - Clock clock_; - std::atomic last_kept_{}; - std::atomic num_allowed_{0}; - std::atomic num_asked_{0}; -}; - -/* Erases the actual type implementing the decide function */ -class ErasedTraceSampler { - public: - template - ErasedTraceSampler(Ptr samplerImpl); - - SamplingDecision decide(const SpanData& span_data); - void handle_collector_response(const CollectorResponse& response); - nlohmann::json config_json() const; - - private: - struct Concept { - virtual ~Concept() = default; - virtual SamplingDecision decide(const SpanData& span_data) = 0; - virtual void handle_collector_response( - const CollectorResponse& response) = 0; - virtual nlohmann::json config_json() const = 0; - }; - - template - struct Model; - - std::unique_ptr impl_; -}; - } // namespace tracing } // namespace datadog diff --git a/src/datadog/trace_segment.cpp b/src/datadog/trace_segment.cpp index 55b8ab07..b7f74c0f 100644 --- a/src/datadog/trace_segment.cpp +++ b/src/datadog/trace_segment.cpp @@ -5,13 +5,11 @@ #include #include #include -#include #include #include #include #include -#include #include #include #include @@ -85,7 +83,7 @@ void inject_trace_tags( TraceSegment::TraceSegment( const std::shared_ptr& logger, const std::shared_ptr& collector, - std::shared_ptr trace_sampler, bool apm_tracing_enabled, + const std::shared_ptr& trace_sampler, const std::shared_ptr& span_sampler, const std::shared_ptr& defaults, const std::shared_ptr& config_manager, @@ -100,8 +98,7 @@ TraceSegment::TraceSegment( std::unique_ptr local_root) : logger_(logger), collector_(collector), - trace_sampler_(std::move(trace_sampler)), - apm_tracing_enabled_(apm_tracing_enabled), + trace_sampler_(trace_sampler), span_sampler_(span_sampler), defaults_(defaults), runtime_id_(runtime_id), @@ -234,13 +231,6 @@ void TraceSegment::span_finished() { local_root.tags[tags::internal::sampling_decider] = "0"; } - // RFC seems to only mandate that this be set if the trace is kept. - // However, system-tests expect this to be always be set. - // Add it all the time; can't hurt - if (!apm_tracing_enabled_) { - local_root.numeric_tags[tags::internal::apm_enabled] = 0; - } - // Some tags are repeated on all spans. for (const auto& span_ptr : spans_) { SpanData& span = *span_ptr; @@ -289,23 +279,7 @@ void TraceSegment::make_sampling_decision_if_null() { return; } - SpanData& local_root = *spans_.front(); - - // ApmDisabledTraceSampler needs to know the value of _dd.p.ts. Copy it here - // so that the value is accessible in the span passed to decide(). - // The value may have been set because it was propagated from upstream or from - // a WAF run we already did - if (!apm_tracing_enabled_) { - auto it = std::find_if( - trace_tags_.begin(), trace_tags_.end(), - [](const auto& entry) { return entry.first == "_dd.p.ts"; }); - if (it != trace_tags_.end()) { - local_root.tags.insert_or_assign(it->first, it->second); - // don't move the strings or erase the entry from trace_tags_: - // besides filling meta, it may still be in the tags propagation header - } - } - + const SpanData& local_root = *spans_.front(); sampling_decision_ = trace_sampler_->decide(local_root); update_decision_maker_trace_tag(); @@ -343,20 +317,13 @@ bool TraceSegment::inject(DictWriter& writer, const SpanData& span) { } bool TraceSegment::inject(DictWriter& writer, const SpanData& span, - const InjectionOptions& inj_opts) { + const InjectionOptions&) { // If the only injection style is `NONE`, then don't do anything. if (injection_styles_.size() == 1 && injection_styles_[0] == PropagationStyle::NONE) { return false; } - if (inj_opts.trace_source) { - std::string trace_source = std::string(inj_opts.trace_source->begin(), - inj_opts.trace_source->end()); - trace_tags_.emplace_back(tags::internal::trace_source, - std::move(trace_source)); - } - // The sampling priority can change (it can be overridden on another thread), // and trace tags might change when that happens ("_dd.p.dm"). // So, we lock here, make a sampling decision if necessary, and then copy the @@ -371,23 +338,6 @@ bool TraceSegment::inject(DictWriter& writer, const SpanData& span, trace_tags = trace_tags_; } - // with APM tracing disabled, stop propagation unless we must keep the trace - if (!apm_tracing_enabled_) { - if (sampling_priority != 2) { - writer.set("x-datadog-trace-id", {}); - writer.set("x-datadog-parent-id", {}); - writer.set("x-datadog-sampling-priority", {}); - writer.set("x-datadog-origin", {}); - writer.set("x-datadog-tags", {}); - writer.set("x-b3-traceid", {}); - writer.set("x-b3-spanid", {}); - writer.set("x-b3-sampled", {}); - writer.set("traceparent", {}); - writer.set("tracestate", {}); - return true; - } - } - for (const auto style : injection_styles_) { switch (style) { case PropagationStyle::DATADOG: diff --git a/src/datadog/tracer.cpp b/src/datadog/tracer.cpp index d786deb3..8849c082 100644 --- a/src/datadog/tracer.cpp +++ b/src/datadog/tracer.cpp @@ -48,11 +48,6 @@ Tracer::Tracer(const FinalizedTracerConfig& config, signature_{runtime_id_, config.defaults.service, config.defaults.environment}, config_manager_(std::make_shared(config)), - apm_tracing_disabled_sampler_( - config.apm_tracing_enabled - ? nullptr - : std::make_shared( - std::make_unique(config.clock))), collector_(/* see constructor body */), span_sampler_( std::make_shared(config.span_sampler, config.clock)), @@ -185,22 +180,14 @@ Span Tracer::create_span(const SpanConfig& config) { hex_padded(span_data->trace_id.high)); } - std::shared_ptr trace_sampler; - if (is_apm_tracing_enabled()) { - trace_sampler = config_manager_->trace_sampler(); - } else { - trace_sampler = apm_tracing_disabled_sampler_; - } - const auto span_data_ptr = span_data.get(); telemetry::counter::increment(metrics::tracer::trace_segments_created, {"new_continued:new"}); const auto segment = std::make_shared( - logger_, collector_, std::move(trace_sampler), is_apm_tracing_enabled(), - span_sampler_, defaults, config_manager_, runtime_id_, injection_styles_, - hostname_, nullopt /* origin */, tags_header_max_size_, - std::move(trace_tags), nullopt /* sampling_decision */, - nullopt /* additional_w3c_tracestate */, + logger_, collector_, config_manager_->trace_sampler(), span_sampler_, + defaults, config_manager_, runtime_id_, injection_styles_, hostname_, + nullopt /* origin */, tags_header_max_size_, std::move(trace_tags), + nullopt /* sampling_decision */, nullopt /* additional_w3c_tracestate */, nullopt /* additional_datadog_w3c_tracestate*/, std::move(span_data)); Span span{span_data_ptr, segment, [generator = generator_]() { return generator->span_id(); }, @@ -391,7 +378,7 @@ Expected Tracer::extract_span(const DictReader& reader, } Optional sampling_decision; - if (merged_context.sampling_priority && is_apm_tracing_enabled()) { + if (merged_context.sampling_priority) { SamplingDecision decision; decision.priority = *merged_context.sampling_priority; // `decision.mechanism` is null. We might be able to infer it once we @@ -401,22 +388,15 @@ Expected Tracer::extract_span(const DictReader& reader, sampling_decision = decision; } - std::shared_ptr trace_sampler; - if (is_apm_tracing_enabled()) { - trace_sampler = config_manager_->trace_sampler(); - } else { - trace_sampler = apm_tracing_disabled_sampler_; - } - const auto span_data_ptr = span_data.get(); telemetry::counter::increment(metrics::tracer::trace_segments_created, {"new_continued:continued"}); const auto segment = std::make_shared( - logger_, collector_, std::move(trace_sampler), is_apm_tracing_enabled(), - span_sampler_, config_manager_->span_defaults(), config_manager_, - runtime_id_, injection_styles_, hostname_, - std::move(merged_context.origin), tags_header_max_size_, - std::move(merged_context.trace_tags), std::move(sampling_decision), + logger_, collector_, config_manager_->trace_sampler(), span_sampler_, + config_manager_->span_defaults(), config_manager_, runtime_id_, + injection_styles_, hostname_, std::move(merged_context.origin), + tags_header_max_size_, std::move(merged_context.trace_tags), + std::move(sampling_decision), std::move(merged_context.additional_w3c_tracestate), std::move(merged_context.additional_datadog_w3c_tracestate), std::move(span_data)); diff --git a/test/mocks/collectors.h b/test/mocks/collectors.h index 91043f62..c2f8768a 100644 --- a/test/mocks/collectors.h +++ b/test/mocks/collectors.h @@ -22,7 +22,7 @@ struct MockCollector : public Collector { std::vector>> chunks; Expected send(std::vector>&& spans, - const std::shared_ptr&) override { + const std::shared_ptr&) override { chunks.emplace_back(std::move(spans)); return {}; } @@ -51,7 +51,7 @@ struct MockCollectorWithResponse : public MockCollector { Expected send( std::vector>&& spans, - const std::shared_ptr& response_handler) override { + const std::shared_ptr& response_handler) override { MockCollector::send(std::move(spans), response_handler); response_handler->handle_collector_response(response); return {}; @@ -64,7 +64,7 @@ struct PriorityCountingCollector : public Collector { std::map sampling_priority_count; Expected send(std::vector>&& spans, - const std::shared_ptr&) override { + const std::shared_ptr&) override { const SpanData& root = root_span(spans); const auto priority = root.numeric_tags.at(tags::internal::sampling_priority); @@ -127,7 +127,7 @@ struct PriorityCountingCollectorWithResponse Expected send( std::vector>&& spans, - const std::shared_ptr& response_handler) override { + const std::shared_ptr& response_handler) override { PriorityCountingCollector::send(std::move(spans), response_handler); REQUIRE(response_handler); response_handler->handle_collector_response(response); @@ -141,7 +141,7 @@ struct FailureCollector : public Collector { Error failure{Error::OTHER, "send(...) failed because I told it to."}; Expected send(std::vector>&&, - const std::shared_ptr&) override { + const std::shared_ptr&) override { return failure; } diff --git a/test/test_datadog_agent.cpp b/test/test_datadog_agent.cpp index 4aa3fbaf..64c6daed 100644 --- a/test/test_datadog_agent.cpp +++ b/test/test_datadog_agent.cpp @@ -246,7 +246,7 @@ TEST_CASE("Datadog-Client-Computed-Stats header", "[datadog_agent]") { config.telemetry.enabled = false; SECTION("header sent when apm_tracing_enabled is false") { - config.agent.apm_tracing_enabled = false; + config.apm_tracing_enabled = false; auto finalized = finalize_config(config); REQUIRE(finalized); @@ -267,7 +267,7 @@ TEST_CASE("Datadog-Client-Computed-Stats header", "[datadog_agent]") { } SECTION("header not sent when apm_tracing_enabled is true") { - config.agent.apm_tracing_enabled = true; + config.apm_tracing_enabled = true; auto finalized = finalize_config(config); REQUIRE(finalized); From 55684de55c966e71b1a721c8ab29d150b7f86298 Mon Sep 17 00:00:00 2001 From: Damien Mehala Date: Sun, 22 Jun 2025 17:49:26 +0200 Subject: [PATCH 5/6] overall improvements --- BUILD.bazel | 2 + CMakeLists.txt | 1 + include/datadog/datadog_agent_config.h | 6 +- include/datadog/dict_writer.h | 3 + include/datadog/injection_options.h | 11 +- include/datadog/span.h | 3 + include/datadog/trace_sampler_config.h | 5 + include/datadog/trace_segment.h | 11 +- include/datadog/trace_source.h | 53 +++ include/datadog/tracer.h | 1 + include/datadog/tracer_config.h | 13 +- src/datadog/datadog_agent.cpp | 3 + src/datadog/datadog_agent_config.cpp | 8 +- src/datadog/extraction_util.cpp | 6 +- src/datadog/span.cpp | 5 + src/datadog/tags.h | 1 + src/datadog/trace_sampler.cpp | 6 +- src/datadog/trace_sampler_config.cpp | 30 ++ src/datadog/trace_segment.cpp | 54 ++- src/datadog/trace_source.cpp | 22 ++ src/datadog/tracer.cpp | 26 +- src/datadog/tracer_config.cpp | 43 ++- src/datadog/tracer_telemetry.cpp | 0 src/datadog/w3c_propagation.cpp | 5 + test/mocks/collectors.h | 2 + test/mocks/dict_writers.h | 2 + test/test_datadog_agent.cpp | 19 +- test/test_span.cpp | 485 ++++++++++++++----------- test/test_tracer.cpp | 371 +++++++++++++------ test/test_tracer_config.cpp | 4 +- 30 files changed, 819 insertions(+), 382 deletions(-) create mode 100644 include/datadog/trace_source.h create mode 100644 src/datadog/trace_source.cpp delete mode 100644 src/datadog/tracer_telemetry.cpp diff --git a/BUILD.bazel b/BUILD.bazel index a95c9d9d..890c00e0 100644 --- a/BUILD.bazel +++ b/BUILD.bazel @@ -42,6 +42,7 @@ cc_library( "src/datadog/string_util.cpp", "src/datadog/tag_propagation.cpp", "src/datadog/tags.cpp", + "src/datadog/trace_source.cpp", "src/datadog/telemetry_metrics.cpp", "src/datadog/threaded_event_scheduler.cpp", "src/datadog/tracer_config.cpp", @@ -117,6 +118,7 @@ cc_library( "include/datadog/tracer_config.h", "include/datadog/tracer_signature.h", "include/datadog/trace_id.h", + "include/datadog/trace_source.h", "include/datadog/trace_sampler_config.h", "include/datadog/trace_segment.h", "include/datadog/version.h", diff --git a/CMakeLists.txt b/CMakeLists.txt index ecbf5612..0a6936f2 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -150,6 +150,7 @@ target_sources(dd_trace_cpp-objects src/datadog/trace_sampler_config.cpp src/datadog/trace_sampler.cpp src/datadog/trace_segment.cpp + src/datadog/trace_source.cpp src/datadog/telemetry_metrics.cpp src/datadog/version.cpp src/datadog/w3c_propagation.cpp diff --git a/include/datadog/datadog_agent_config.h b/include/datadog/datadog_agent_config.h index a8a0742b..7db0d651 100644 --- a/include/datadog/datadog_agent_config.h +++ b/include/datadog/datadog_agent_config.h @@ -22,7 +22,6 @@ #include "expected.h" #include "http_client.h" #include "remote_config/listener.h" -#include "string_view.h" namespace datadog { namespace tracing { @@ -90,6 +89,11 @@ class FinalizedDatadogAgentConfig { // Origin detection Optional admission_controller_uid; + + // Indicate if stats computation should be delegated to the Agent. + // This feature is not supported yet, however, we need to inform the Agent to + // not compute stats when APM Tracing `DD_APM_TRACING_ENABLED` is disabled. + bool stats_computation_enabled; }; Expected finalize_config( diff --git a/include/datadog/dict_writer.h b/include/datadog/dict_writer.h index 572acdd0..89a4bfe3 100644 --- a/include/datadog/dict_writer.h +++ b/include/datadog/dict_writer.h @@ -21,6 +21,9 @@ class DictWriter { // implementation may, but is not required to, overwrite any previous value at // `key`. virtual void set(StringView key, StringView value) = 0; + + // Removes the entry associated with the given key. + virtual void erase(StringView){}; }; } // namespace tracing diff --git a/include/datadog/injection_options.h b/include/datadog/injection_options.h index 6890794a..f6f24b2d 100644 --- a/include/datadog/injection_options.h +++ b/include/datadog/injection_options.h @@ -4,19 +4,10 @@ // parameters to `Span::inject` that alter the behavior of trace context // propagation. -#include - -#include "optional.h" - namespace datadog { namespace tracing { -struct InjectionOptions { - // If DD_APM_TRACING_ENABLED=false and what we're injecting is not an APM - // trace, then the code for the trace source (e.g. 02 for Appsec) can be - // set here. - Optional> trace_source{}; -}; +struct InjectionOptions {}; } // namespace tracing } // namespace datadog diff --git a/include/datadog/span.h b/include/datadog/span.h index d15348b3..0018230f 100644 --- a/include/datadog/span.h +++ b/include/datadog/span.h @@ -49,6 +49,7 @@ #include "optional.h" #include "string_view.h" #include "trace_id.h" +#include "trace_source.h" namespace datadog { namespace tracing { @@ -162,6 +163,8 @@ class Span { // Set end time of this span. Doing so will override the default behavior of // using the current time in the destructor. void set_end_time(std::chrono::steady_clock::time_point); + // Specifies the product (AppSec, DBM) that created this span. + void set_source(Source); // Write information about this span and its trace into the specified `writer` // using all of the configured injection propagation styles. diff --git a/include/datadog/trace_sampler_config.h b/include/datadog/trace_sampler_config.h index 07a2ff53..094cceba 100644 --- a/include/datadog/trace_sampler_config.h +++ b/include/datadog/trace_sampler_config.h @@ -24,6 +24,7 @@ struct TraceSamplerRule final { Rate rate; SpanMatcher matcher; SamplingMechanism mechanism; + bool bypass_limiter = false; }; struct TraceSamplerConfig { @@ -50,6 +51,10 @@ class FinalizedTraceSamplerConfig { double max_per_second; std::vector rules; std::unordered_map metadata; + + public: + /// Returns the trace sampler configuration when APM Tracing is disabled. + static FinalizedTraceSamplerConfig apm_tracing_disabled_config(); }; Expected finalize_config( diff --git a/include/datadog/trace_segment.h b/include/datadog/trace_segment.h index 4db61f85..d95b71a4 100644 --- a/include/datadog/trace_segment.h +++ b/include/datadog/trace_segment.h @@ -32,7 +32,6 @@ #include #include -#include "expected.h" #include "optional.h" #include "propagation_style.h" #include "runtime_id.h" @@ -80,6 +79,8 @@ class TraceSegment { std::shared_ptr config_manager_; + bool tracing_enabled_; + public: TraceSegment(const std::shared_ptr& logger, const std::shared_ptr& collector, @@ -95,7 +96,8 @@ class TraceSegment { Optional sampling_decision, Optional additional_w3c_tracestate, Optional additional_datadog_w3c_tracestate, - std::unique_ptr local_root); + std::unique_ptr local_root, + bool tracing_enabled = true); const SpanDefaults& defaults() const; const Optional& hostname() const; @@ -118,10 +120,13 @@ class TraceSegment { void span_finished(); // Set the sampling decision to be a local, manual decision with the specified - // sampling `priority`. Overwrite any previous sampling decision. + // sampling `priority`. Overwrite any previous sampling decision. void override_sampling_priority(int priority); void override_sampling_priority(SamplingPriority priority); + // Retrieves the local root span. + SpanData& local_root() const; + private: // If `sampling_decision_` is null, use `trace_sampler_` to make a // sampling decision and assign it to `sampling_decision_`. diff --git a/include/datadog/trace_source.h b/include/datadog/trace_source.h new file mode 100644 index 00000000..4da82e1c --- /dev/null +++ b/include/datadog/trace_source.h @@ -0,0 +1,53 @@ +#pragma once + +#include + +namespace datadog { +namespace tracing { + +/// Enumerates the possible trace sources that can generate a span. +/// +/// This enum class identifies the different products that can create a span. +/// Each source is represented by a distinct bit flag, allowing for bitwise +/// operations. +enum class Source : char { + apm = 0x01, + appsec = 0x02, + datastream_monitoring = 0x04, + datajob_monitoring = 0x08, + database_monitoring = 0x10, +}; + +/// Validates if a given string corresponds to a valid trace source. +/// +/// This function checks whether the provided string matches any of the +/// predefined trace sources specified in the Source enum. It is useful for +/// ensuring that a source string obtained from an external input is valid +/// before further processing. +/// +/// @param source_str A string view representing the trace source to validate. +/// +/// @return true if the source string is valid and corresponds to a known trace +/// source, false otherwise. +bool validate_trace_source(StringView source_str); + +/// Converts a Source enum value to its corresponding string representation +inline constexpr StringView to_tag(Source source) { + switch (source) { + case Source::apm: + return "01"; + case Source::appsec: + return "02"; + case Source::database_monitoring: + return "04"; + case Source::datajob_monitoring: + return "08"; + case Source::datastream_monitoring: + return "10"; + } + + return ""; +} + +} // namespace tracing +} // namespace datadog diff --git a/include/datadog/tracer.h b/include/datadog/tracer.h index 5c183795..dd33a929 100644 --- a/include/datadog/tracer.h +++ b/include/datadog/tracer.h @@ -54,6 +54,7 @@ class Tracer { Baggage::Options baggage_opts_; bool baggage_injection_enabled_; bool baggage_extraction_enabled_; + bool tracing_enabled_; public: // Create a tracer configured using the specified `config`, and optionally: diff --git a/include/datadog/tracer_config.h b/include/datadog/tracer_config.h index 05ac8c78..a94e9578 100644 --- a/include/datadog/tracer_config.h +++ b/include/datadog/tracer_config.h @@ -169,8 +169,15 @@ struct TracerConfig { /// separate thread. std::shared_ptr event_scheduler; - /// TBD - Optional apm_tracing_enabled; + /// `tracing_enabled` indicates whether APM traces and APM trace metrics + /// are enabled. If `false`, APM-specific traces are and metrics are dropped + /// This allows other products to operate independently (for example, AppSec). + /// This is distinct from `report_traces`, which controls whether any traces + /// are sent at all. + /// + /// Overridden by the `DD_APM_TRACING_ENABLED` environment variable. Defaults + /// to `true`. + Optional tracing_enabled; }; // `FinalizedTracerConfig` contains `Tracer` implementation details derived from @@ -210,7 +217,7 @@ class FinalizedTracerConfig final { HTTPClient::URL agent_url; std::shared_ptr event_scheduler; std::shared_ptr http_client; - bool apm_tracing_enabled; + bool tracing_enabled; }; // Return a `FinalizedTracerConfig` from the specified `config` and from any diff --git a/src/datadog/datadog_agent.cpp b/src/datadog/datadog_agent.cpp index 6e788ae7..e761d8fe 100644 --- a/src/datadog/datadog_agent.cpp +++ b/src/datadog/datadog_agent.cpp @@ -167,6 +167,9 @@ DatadogAgent::DatadogAgent( tracer_signature.library_language_version); headers_.emplace("Datadog-Meta-Tracer-Version", tracer_signature.library_version); + if (config.stats_computation_enabled) { + headers_.emplace("Datadog-Client-Computed-Stats", "yes"); + } // Origin Detection headers are not necessary when Unix Domain Socket (UDS) // is used to communicate with the Datadog Agent. diff --git a/src/datadog/datadog_agent_config.cpp b/src/datadog/datadog_agent_config.cpp index 9ffe6217..6656ae0f 100644 --- a/src/datadog/datadog_agent_config.cpp +++ b/src/datadog/datadog_agent_config.cpp @@ -145,12 +145,16 @@ Expected finalize_config( result.metadata[ConfigName::AGENT_URL] = ConfigMetadata(ConfigName::AGENT_URL, url, origin); - /// Starting Agent X, the admission controller inject a unique identifier - /// through `DD_EXTERNAL_ENV`. This uid is used for origin detection. + // Starting Datadog Agent 7.62.0, the admission controller inject a unique + // identifier through `DD_EXTERNAL_ENV`. This uid is used for origin + // detection. if (auto external_env = lookup(environment::DD_EXTERNAL_ENV)) { result.admission_controller_uid = std::string(*external_env); } + // Not supported yet but required for APM tracing disablement. + result.stats_computation_enabled = false; + return result; } diff --git a/src/datadog/extraction_util.cpp b/src/datadog/extraction_util.cpp index dfeed0bc..94863108 100644 --- a/src/datadog/extraction_util.cpp +++ b/src/datadog/extraction_util.cpp @@ -1,8 +1,8 @@ #include "extraction_util.h" #include +#include -#include #include #include #include @@ -34,7 +34,7 @@ void handle_trace_tags(StringView trace_tags, ExtractedData& result, } for (auto& [key, value] : *maybe_trace_tags) { - if (!starts_with(key, "_dd.p.") && key != "_dd.p.ts") { + if (!starts_with(key, "_dd.p.")) { continue; } @@ -51,6 +51,8 @@ void handle_trace_tags(StringView trace_tags, ExtractedData& result, // been extracted (i.e. we look for X-Datadog-Trace-ID first). result.trace_id->high = *high; } + } else if (key == tags::internal::trace_source) { + if (!validate_trace_source(value)) continue; } result.trace_tags.emplace_back(std::move(key), std::move(value)); diff --git a/src/datadog/span.cpp b/src/datadog/span.cpp index f7419841..7d076996 100644 --- a/src/datadog/span.cpp +++ b/src/datadog/span.cpp @@ -159,6 +159,11 @@ void Span::set_end_time(std::chrono::steady_clock::time_point end_time) { end_time_ = end_time; } +void Span::set_source(Source source) { + trace_segment_->local_root().tags.emplace(tags::internal::trace_source, + to_tag(source)); +} + TraceSegment& Span::trace_segment() { return *trace_segment_; } const TraceSegment& Span::trace_segment() const { return *trace_segment_; } diff --git a/src/datadog/tags.h b/src/datadog/tags.h index 52a1af34..a6eb1751 100644 --- a/src/datadog/tags.h +++ b/src/datadog/tags.h @@ -41,6 +41,7 @@ extern const std::string sampling_decider; extern const std::string w3c_parent_id; extern const std::string trace_source; // _dd.p.ts extern const std::string apm_enabled; // _dd.apm.enabled + } // namespace internal // Return whether the specified `tag_name` is reserved for use internal to this diff --git a/src/datadog/trace_sampler.cpp b/src/datadog/trace_sampler.cpp index d6f2d8bd..153b2610 100644 --- a/src/datadog/trace_sampler.cpp +++ b/src/datadog/trace_sampler.cpp @@ -6,7 +6,6 @@ #include #include #include -#include #include "collector_response.h" #include "json_serializer.h" @@ -56,6 +55,11 @@ SamplingDecision TraceSampler::decide(const SpanData& span) { decision.configured_rate = rule.rate; const std::uint64_t threshold = max_id_from_rate(rule.rate); if (knuth_hash(span.trace_id.low) <= threshold) { + if (rule.bypass_limiter) { + decision.priority = int(SamplingPriority::USER_KEEP); + return decision; + } + const auto result = limiter_.allow(); if (result.allowed) { decision.priority = int(SamplingPriority::USER_KEEP); diff --git a/src/datadog/trace_sampler_config.cpp b/src/datadog/trace_sampler_config.cpp index 1d595e71..9981cd12 100644 --- a/src/datadog/trace_sampler_config.cpp +++ b/src/datadog/trace_sampler_config.cpp @@ -1,5 +1,6 @@ #include #include +#include #include #include @@ -8,6 +9,7 @@ #include "json_serializer.h" #include "parse_util.h" #include "string_util.h" +#include "tags.h" namespace datadog { namespace tracing { @@ -244,5 +246,33 @@ Expected finalize_config( return result; } +FinalizedTraceSamplerConfig +FinalizedTraceSamplerConfig::apm_tracing_disabled_config() { + FinalizedTraceSamplerConfig config; + + // Set a rule to always keep spans with an appsec trace source. + TraceSamplerRule appsec_rule; + appsec_rule.rate = Rate::one(); + appsec_rule.matcher.tags.emplace(tags::internal::trace_source, + to_tag(Source::appsec)); + appsec_rule.mechanism = SamplingMechanism::APP_SEC; + // The rule is not subject to the global limiter. + appsec_rule.bypass_limiter = true; + config.rules.emplace_back(std::move(appsec_rule)); + + // Set default sampling rate to 1.0. This is balanced by the limiter define + // below. + TraceSamplerRule all; + all.rate = Rate::one(); + all.mechanism = SamplingMechanism::DEFAULT; + config.rules.emplace_back(std::move(all)); + + // For service liveness (services showed in the service catalog for + // example), allow 1 trace/min. + config.max_per_second = 1. / 60; + + return config; +} + } // namespace tracing } // namespace datadog diff --git a/src/datadog/trace_segment.cpp b/src/datadog/trace_segment.cpp index b7f74c0f..cbf4e067 100644 --- a/src/datadog/trace_segment.cpp +++ b/src/datadog/trace_segment.cpp @@ -95,7 +95,7 @@ TraceSegment::TraceSegment( Optional sampling_decision, Optional additional_w3c_tracestate, Optional additional_datadog_w3c_tracestate, - std::unique_ptr local_root) + std::unique_ptr local_root, bool apm_tracing_enabled) : logger_(logger), collector_(collector), trace_sampler_(trace_sampler), @@ -112,7 +112,8 @@ TraceSegment::TraceSegment( additional_w3c_tracestate_(std::move(additional_w3c_tracestate)), additional_datadog_w3c_tracestate_( std::move(additional_datadog_w3c_tracestate)), - config_manager_(config_manager) { + config_manager_(config_manager), + tracing_enabled_(apm_tracing_enabled) { assert(logger_); assert(collector_); assert(trace_sampler_); @@ -224,6 +225,7 @@ void TraceSegment::span_finished() { } } } + if (decision.origin == SamplingDecision::Origin::DELEGATED && local_root.parent_id == 0) { // Convey the fact that, even though we are the root service, we delegated @@ -231,6 +233,13 @@ void TraceSegment::span_finished() { local_root.tags[tags::internal::sampling_decider] = "0"; } + // RFC seems to only mandate that this be set if the trace is kept. + // However, system-tests expect this to be always be set. + // Add it all the time; can't hurt + if (!tracing_enabled_) { + local_root.numeric_tags[tags::internal::apm_enabled] = 0; + } + // Some tags are repeated on all spans. for (const auto& span_ptr : spans_) { SpanData& span = *span_ptr; @@ -321,7 +330,7 @@ bool TraceSegment::inject(DictWriter& writer, const SpanData& span, // If the only injection style is `NONE`, then don't do anything. if (injection_styles_.size() == 1 && injection_styles_[0] == PropagationStyle::NONE) { - return false; + return true; } // The sampling priority can change (it can be overridden on another thread), @@ -338,6 +347,39 @@ bool TraceSegment::inject(DictWriter& writer, const SpanData& span, trace_tags = trace_tags_; } + auto& local_root_tags = spans_.front()->tags; + + auto ts_tag_found = std::find_if( + local_root_tags.cbegin(), local_root_tags.cend(), + [](const auto& p) { return p.first == tags::internal::trace_source; }); + + // When tracing (the product) is disabled, skip tracing context propagation + // when: + // - the local root span is NOT created by another product (no `_dd.p.ts`) + // - sampling priority is DROP + if (!tracing_enabled_) { + if (ts_tag_found == local_root_tags.cend() && sampling_priority <= 0) { + writer.erase("x-datadog-trace-id"); + writer.erase("x-datadog-parent-id"); + writer.erase("x-datadog-sampling-priority"); + writer.erase("x-datadog-origin"); + writer.erase("x-datadog-trace-id"); + writer.erase("x-datadog-tags"); + writer.erase("x-b3-traceid"); + writer.erase("x-b3-spanid"); + writer.erase("x-b3-sampled"); + writer.erase("x-datadog-origin"); + writer.erase("traceparent"); + writer.erase("tracestate"); + return false; + } + } + + // Add `_dd.p.ts` to `trace_tags` for context propagation. + if (ts_tag_found != local_root_tags.cend()) { + trace_tags.emplace_back(tags::internal::trace_source, ts_tag_found->second); + } + for (const auto style : injection_styles_) { switch (style) { case PropagationStyle::DATADOG: @@ -349,7 +391,7 @@ bool TraceSegment::inject(DictWriter& writer, const SpanData& span, writer.set("x-datadog-origin", *origin_); } inject_trace_tags(writer, trace_tags, tags_header_max_size_, - spans_.front()->tags, *logger_); + local_root_tags, *logger_); telemetry::counter::increment(metrics::tracer::trace_context::injected, {"header_style:datadog"}); @@ -366,7 +408,7 @@ bool TraceSegment::inject(DictWriter& writer, const SpanData& span, writer.set("x-datadog-origin", *origin_); } inject_trace_tags(writer, trace_tags, tags_header_max_size_, - spans_.front()->tags, *logger_); + local_root_tags, *logger_); telemetry::counter::increment(metrics::tracer::trace_context::injected, {"header_style:b3multi"}); break; @@ -390,5 +432,7 @@ bool TraceSegment::inject(DictWriter& writer, const SpanData& span, return true; } +SpanData& TraceSegment::local_root() const { return *spans_.front(); } + } // namespace tracing } // namespace datadog diff --git a/src/datadog/trace_source.cpp b/src/datadog/trace_source.cpp new file mode 100644 index 00000000..6bfdc008 --- /dev/null +++ b/src/datadog/trace_source.cpp @@ -0,0 +1,22 @@ +#include + +#include "parse_util.h" + +namespace datadog { +namespace tracing { + +bool validate_trace_source(StringView source_str) { + if (source_str.size() > 2) return false; + + auto maybe_ts_uint = parse_uint64(source_str, 10); + if (maybe_ts_uint.if_error()) return false; + + // Bit twiddling magic is coming from + // <3. + auto is_power_of_2 = [](uint64_t v) -> bool { return v && !(v & (v - 1)); }; + + return is_power_of_2(*maybe_ts_uint); +} + +} // namespace tracing +} // namespace datadog diff --git a/src/datadog/tracer.cpp b/src/datadog/tracer.cpp index 8849c082..472fb3b4 100644 --- a/src/datadog/tracer.cpp +++ b/src/datadog/tracer.cpp @@ -58,7 +58,8 @@ Tracer::Tracer(const FinalizedTracerConfig& config, tags_header_max_size_(config.tags_header_size), baggage_opts_(config.baggage_opts), baggage_injection_enabled_(false), - baggage_extraction_enabled_(false) { + baggage_extraction_enabled_(false), + tracing_enabled_(config.tracing_enabled) { telemetry::init(config.telemetry, signature_, logger_, config.http_client, config.event_scheduler, config.agent_url); if (config.report_hostname) { @@ -188,7 +189,8 @@ Span Tracer::create_span(const SpanConfig& config) { defaults, config_manager_, runtime_id_, injection_styles_, hostname_, nullopt /* origin */, tags_header_max_size_, std::move(trace_tags), nullopt /* sampling_decision */, nullopt /* additional_w3c_tracestate */, - nullopt /* additional_datadog_w3c_tracestate*/, std::move(span_data)); + nullopt /* additional_datadog_w3c_tracestate*/, std::move(span_data), + tracing_enabled_); Span span{span_data_ptr, segment, [generator = generator_]() { return generator->span_id(); }, clock_}; @@ -377,8 +379,24 @@ Expected Tracer::extract_span(const DictReader& reader, *merged_context.datadog_w3c_parent_id; } + // Trace source tag is not a trace tag, move it to a simple tag on the local + // root span. + if (const auto found = std::find_if( + merged_context.trace_tags.cbegin(), merged_context.trace_tags.cend(), + [](const auto& p) { + return p.first == tags::internal::trace_source; + }); + found != merged_context.trace_tags.cend()) { + span_data->tags.emplace(tags::internal::trace_source, found->second); + merged_context.trace_tags.erase(found); + } + + // When APM Tracing is disabled, the incoming sampling decision MAY be + // overridden based on locally generated spans. As such, the received sampling + // decision is intentionally ignored, and the tracer is expected to make its + // own decision in accordance with the locally enabled product configuration. Optional sampling_decision; - if (merged_context.sampling_priority) { + if (tracing_enabled_ && merged_context.sampling_priority) { SamplingDecision decision; decision.priority = *merged_context.sampling_priority; // `decision.mechanism` is null. We might be able to infer it once we @@ -399,7 +417,7 @@ Expected Tracer::extract_span(const DictReader& reader, std::move(sampling_decision), std::move(merged_context.additional_w3c_tracestate), std::move(merged_context.additional_datadog_w3c_tracestate), - std::move(span_data)); + std::move(span_data), tracing_enabled_); Span span{span_data_ptr, segment, [generator = generator_]() { return generator->span_id(); }, clock_}; diff --git a/src/datadog/tracer_config.cpp b/src/datadog/tracer_config.cpp index e7456bf2..1af94a05 100644 --- a/src/datadog/tracer_config.cpp +++ b/src/datadog/tracer_config.cpp @@ -6,7 +6,6 @@ #include #include #include -#include #include #include "datadog_agent.h" @@ -131,7 +130,7 @@ Expected load_tracer_env_config(Logger &logger) { } if (auto apm_enabled_env = lookup(environment::DD_APM_TRACING_ENABLED)) { - env_cfg.apm_tracing_enabled = !falsy(*apm_enabled_env); + env_cfg.tracing_enabled = !falsy(*apm_enabled_env); } // Baggage @@ -351,13 +350,6 @@ Expected finalize_config(const TracerConfig &user_config, final_config.metadata[ConfigName::REPORT_TRACES] = ConfigMetadata( ConfigName::REPORT_TRACES, to_string(final_config.report_traces), origin); - // APM Tracing Enabled - std::tie(origin, final_config.apm_tracing_enabled) = pick( - env_config->apm_tracing_enabled, user_config.apm_tracing_enabled, true); - final_config.metadata[ConfigName::APM_TRACING_ENABLED] = - ConfigMetadata(ConfigName::APM_TRACING_ENABLED, - to_string(final_config.apm_tracing_enabled), origin); - // Report hostname final_config.report_hostname = value_or(env_config->report_hostname, user_config.report_hostname, false); @@ -416,12 +408,6 @@ Expected finalize_config(const TracerConfig &user_config, if (auto *error = agent_finalized.if_error()) { return std::move(*error); } - if (!user_config.collector) { - final_config.collector = *agent_finalized; - final_config.metadata.merge(agent_finalized->metadata); - } else { - final_config.collector = user_config.collector; - } if (auto trace_sampler_config = finalize_config(user_config.trace_sampler)) { final_config.metadata.merge(trace_sampler_config->metadata); @@ -460,6 +446,33 @@ Expected finalize_config(const TracerConfig &user_config, return std::move(telemetry_final_config.error()); } + // APM Tracing Enabled + std::tie(origin, final_config.tracing_enabled) = + pick(env_config->tracing_enabled, user_config.tracing_enabled, true); + final_config.metadata[ConfigName::APM_TRACING_ENABLED] = + ConfigMetadata(ConfigName::APM_TRACING_ENABLED, + to_string(final_config.tracing_enabled), origin); + + // Whether APM tracing is enabled. This affects whether the + // "Datadog-Client-Computed-Stats: yes" header is sent with trace requests. + if (!final_config.tracing_enabled) { + agent_finalized->stats_computation_enabled = !final_config.tracing_enabled; + + // Overwrite the trace sampler configuration with a specific trace sampler + // configuration which: + // - always keep spans generated by other products; + // - allow one trace per minute for service liveness; + final_config.trace_sampler = + FinalizedTraceSamplerConfig::apm_tracing_disabled_config(); + } + + if (!user_config.collector) { + final_config.collector = *agent_finalized; + final_config.metadata.merge(agent_finalized->metadata); + } else { + final_config.collector = user_config.collector; + } + return final_config; } diff --git a/src/datadog/tracer_telemetry.cpp b/src/datadog/tracer_telemetry.cpp deleted file mode 100644 index e69de29b..00000000 diff --git a/src/datadog/w3c_propagation.cpp b/src/datadog/w3c_propagation.cpp index 244b923e..28e7f341 100644 --- a/src/datadog/w3c_propagation.cpp +++ b/src/datadog/w3c_propagation.cpp @@ -1,6 +1,7 @@ #include "w3c_propagation.h" #include +#include #include #include @@ -233,6 +234,10 @@ void parse_datadog_tracestate(ExtractedData& result, StringView datadog_value) { } } else if (key == "p") { result.datadog_w3c_parent_id = std::string(value); + } else if (key == "ts") { + if (validate_trace_source(value)) { + result.trace_tags.emplace_back(tags::internal::trace_source, value); + } } else if (starts_with(key, "t.")) { // The part of the key that follows "t." is the name of a trace tag, // except without the "_dd.p." prefix. diff --git a/test/mocks/collectors.h b/test/mocks/collectors.h index c2f8768a..d768713f 100644 --- a/test/mocks/collectors.h +++ b/test/mocks/collectors.h @@ -27,6 +27,8 @@ struct MockCollector : public Collector { return {}; } + void clear() { chunks.clear(); } + std::string config() const override; SpanData& first_span() const { diff --git a/test/mocks/dict_writers.h b/test/mocks/dict_writers.h index eb68a1be..f106b2b8 100644 --- a/test/mocks/dict_writers.h +++ b/test/mocks/dict_writers.h @@ -14,4 +14,6 @@ struct MockDictWriter : public DictWriter { void set(StringView key, StringView value) override { items.insert_or_assign(std::string(key), std::string(value)); } + + void erase(StringView key) override { items.erase(std::string(key)); } }; diff --git a/test/test_datadog_agent.cpp b/test/test_datadog_agent.cpp index 64c6daed..92ee9a06 100644 --- a/test/test_datadog_agent.cpp +++ b/test/test_datadog_agent.cpp @@ -232,7 +232,7 @@ DATADOG_AGENT_TEST("Remote Configuration") { } } -TEST_CASE("Datadog-Client-Computed-Stats header", "[datadog_agent]") { +DATADOG_AGENT_TEST("Datadog-Client-Computed-Stats header") { const auto logger = std::make_shared(std::cerr, MockLogger::ERRORS_ONLY); const auto event_scheduler = std::make_shared(); @@ -246,7 +246,7 @@ TEST_CASE("Datadog-Client-Computed-Stats header", "[datadog_agent]") { config.telemetry.enabled = false; SECTION("header sent when apm_tracing_enabled is false") { - config.apm_tracing_enabled = false; + config.tracing_enabled = false; auto finalized = finalize_config(config); REQUIRE(finalized); @@ -257,17 +257,15 @@ TEST_CASE("Datadog-Client-Computed-Stats header", "[datadog_agent]") { Span span = tracer.create_span(); } - http_client->drain(std::chrono::steady_clock::now()); - // the header is present - auto header_it = http_client->request_headers.items.find( + const auto header_it = http_client->request_headers.items.find( "Datadog-Client-Computed-Stats"); REQUIRE(header_it != http_client->request_headers.items.end()); - REQUIRE(header_it->second == "yes"); + CHECK(header_it->second == "yes"); } SECTION("header not sent when apm_tracing_enabled is true") { - config.apm_tracing_enabled = true; + config.tracing_enabled = true; auto finalized = finalize_config(config); REQUIRE(finalized); @@ -278,11 +276,8 @@ TEST_CASE("Datadog-Client-Computed-Stats header", "[datadog_agent]") { Span span = tracer.create_span(); } - http_client->drain(std::chrono::steady_clock::now()); - // verify trhat the header is not present - auto header_it = http_client->request_headers.items.find( - "Datadog-Client-Computed-Stats"); - REQUIRE(header_it == http_client->request_headers.items.end()); + CHECK(http_client->request_headers.items.count( + "Datadog-Client-Computed-Stats") == 0); } } diff --git a/test/test_span.cpp b/test/test_span.cpp index 9672397f..5cbcb364 100644 --- a/test/test_span.cpp +++ b/test/test_span.cpp @@ -19,7 +19,6 @@ #include #include "catch.hpp" -#include "datadog/sampling_mechanism.h" #include "matchers.h" #include "mocks/collectors.h" #include "mocks/dict_readers.h" @@ -29,8 +28,11 @@ #include "test.h" using namespace datadog::tracing; +using namespace std::chrono_literals; -TEST_CASE("set_tag") { +#define TEST_SPAN(x) TEST_CASE(x, "[span]") + +TEST_SPAN("set_tag") { TracerConfig config; config.service = "testsvc"; const auto collector = std::make_shared(); @@ -91,7 +93,7 @@ TEST_CASE("set_tag") { } } -TEST_CASE("lookup_tag") { +TEST_SPAN("lookup_tag") { TracerConfig config; config.service = "testsvc"; config.collector = std::make_shared(); @@ -134,7 +136,7 @@ TEST_CASE("lookup_tag") { } } -TEST_CASE("remove_tag") { +TEST_SPAN("remove_tag") { TracerConfig config; config.service = "testsvc"; config.collector = std::make_shared(); @@ -166,7 +168,7 @@ TEST_CASE("remove_tag") { } } -TEST_CASE("set_metric") { +TEST_SPAN("set_metric") { TracerConfig config; config.service = "testsvc"; const auto collector = std::make_shared(); @@ -222,7 +224,7 @@ TEST_CASE("set_metric") { } } -TEST_CASE("lookup_metric") { +TEST_SPAN("lookup_metric") { TracerConfig config; config.service = "testsvc"; config.collector = std::make_shared(); @@ -251,7 +253,7 @@ TEST_CASE("lookup_metric") { } } -TEST_CASE("remove_metric") { +TEST_SPAN("remove_metric") { TracerConfig config; config.service = "testsvc"; config.collector = std::make_shared(); @@ -282,7 +284,7 @@ TEST_CASE("remove_metric") { } } -TEST_CASE("span duration") { +TEST_SPAN("span duration") { TracerConfig config; config.service = "testsvc"; auto collector = std::make_shared(); @@ -328,7 +330,7 @@ TEST_CASE("span duration") { } } -TEST_CASE(".error() and .set_error*()") { +TEST_SPAN(".error() and .set_error*()") { struct TestCase { std::string name; std::function mutate; @@ -408,7 +410,7 @@ TEST_CASE(".error() and .set_error*()") { } } -TEST_CASE("property setters and getters") { +TEST_SPAN("property setters and getters") { // Verify that modifications made by `Span::set_...` are visible both in the // corresponding getter method and in the resulting span data sent to the // collector. @@ -465,7 +467,7 @@ TEST_CASE("property setters and getters") { // Trace context injection is implemented in `TraceSegment`, but it's part of // the interface of `Span`, so the test is here. -TEST_CASE("injection") { +TEST_SPAN("injection") { TracerConfig config; config.service = "testsvc"; config.collector = std::make_shared(); @@ -483,6 +485,30 @@ TEST_CASE("injection") { }; Tracer tracer{*finalized_config, std::make_shared(42)}; + SECTION("APM Disabled cancel context propagation") { + config.tracing_enabled = false; + auto finalized_disable_config = finalize_config(config); + REQUIRE(finalized_disable_config); + + Tracer apm_disabled_tracer{*finalized_disable_config}; + + auto apm_span = tracer.create_span(); + MockDictWriter writer; + apm_span.inject(writer); + + REQUIRE(writer.items.empty() == false); + + // Consume the span that MUST be kept for service liveness. + { apm_disabled_tracer.create_span(); } + + auto span = apm_disabled_tracer.create_span(); + + // reuse the same writer. Since span generated from apm_disabled_tracer is + // not marked by a product injection should be cancelled. + span.inject(writer); + CHECK(writer.items.empty()); + } + SECTION("trace ID, parent ID ,and sampling priority") { auto span = tracer.create_span(); REQUIRE(span.trace_id() == 42); @@ -497,7 +523,6 @@ TEST_CASE("injection") { REQUIRE(headers.at("x-datadog-trace-id") == "42"); REQUIRE(headers.at("x-datadog-parent-id") == "42"); REQUIRE(headers.at("x-datadog-sampling-priority") == "3"); - REQUIRE(headers.count("x-datadog-delegate-trace-sampling") == 0); REQUIRE(headers.at("x-b3-traceid") == "000000000000002a"); REQUIRE(headers.at("x-b3-spanid") == "000000000000002a"); REQUIRE(headers.at("x-b3-sampled") == "1"); @@ -546,9 +571,49 @@ TEST_CASE("injection") { REQUIRE_THAT(*input, ContainsSubset(*output)); } } + + SECTION("trace source is not set (default)") { + auto span = tracer.create_span(); + + MockDictWriter writer; + span.inject(writer); + + const auto& headers = writer.items; + REQUIRE(headers.count("x-datadog-tags") > 0); + + // When there is no trace source, there should be no `_dd.p.ts` tag. + const auto decoded_tags = decode_tags(headers.at("x-datadog-tags")); + REQUIRE(decoded_tags); + auto found = std::find_if( + decoded_tags->begin(), decoded_tags->end(), [](const auto& tag) { + return tag.first == tags::internal::trace_source; + }); + CHECK(found == decoded_tags->end()); + } + + SECTION("trace source is propagated") { + auto span = tracer.create_span(); + span.set_source(Source::database_monitoring); + + MockDictWriter writer; + span.inject(writer); + + const auto& headers = writer.items; + REQUIRE(headers.count("x-datadog-tags") > 0); + + const auto decoded_tags = decode_tags(headers.at("x-datadog-tags")); + REQUIRE(decoded_tags); + auto found = std::find_if( + decoded_tags->begin(), decoded_tags->end(), [](const auto& tag) { + return tag.first == tags::internal::trace_source; + }); + + REQUIRE(found != decoded_tags->cend()); + CHECK(found->second == to_tag(Source::database_monitoring)); + } } -TEST_CASE("injection can be disabled using the \"none\" style") { +TEST_SPAN("injection can be disabled using the \"none\" style") { TracerConfig config; config.service = "testsvc"; config.name = "spanny"; @@ -567,7 +632,7 @@ TEST_CASE("injection can be disabled using the \"none\" style") { REQUIRE(writer.items == empty); } -TEST_CASE("injecting W3C traceparent header") { +TEST_SPAN("injecting W3C traceparent header") { TracerConfig config; config.service = "testsvc"; config.collector = std::make_shared(); @@ -656,7 +721,7 @@ TEST_CASE("injecting W3C traceparent header") { } } -TEST_CASE("injecting W3C tracestate header") { +TEST_SPAN("injecting W3C tracestate header") { // Concerns: // - the basics: // - sampling priority @@ -702,86 +767,157 @@ TEST_CASE("injecting W3C tracestate header") { static const auto traceparent_drop = "00-00000000000000000000000000000001-0000000000000001-00"; - // clang-format off auto test_case = GENERATE(values({ - {__LINE__, "sampling priority", - {{"x-datadog-trace-id", "1"}, {"x-datadog-parent-id", "1"}, - {"x-datadog-sampling-priority", "2"}}, - "dd=s:2;p:$parent_id"}, - - {__LINE__, "origin", - {{"x-datadog-trace-id", "1"}, {"x-datadog-parent-id", "1"}, - {"x-datadog-origin", "France"}}, - // The "s:-1" comes from the 0% sample rate. - "dd=s:-1;p:$parent_id;o:France"}, - - {__LINE__, "trace tags", - {{"x-datadog-trace-id", "1"}, {"x-datadog-parent-id", "1"}, - {"x-datadog-tags", "_dd.p.foo=x,_dd.p.bar=y,ignored=wrong_prefix"}}, - // The "s:-1" comes from the 0% sample rate. - "dd=s:-1;p:$parent_id;t.foo:x;t.bar:y"}, - - {__LINE__, "extra fields", - {{"traceparent", traceparent_drop}, {"tracestate", "dd=foo:bar;boing:boing"}}, - // The "s:0" comes from the sampling decision in `traceparent_drop`. - "dd=s:0;p:$parent_id;foo:bar;boing:boing"}, - - {__LINE__, "all of the above", - {{"traceparent", traceparent_drop}, - {"tracestate", "dd=o:France;t.foo:x;t.bar:y;foo:bar;boing:boing"}}, - // The "s:0" comes from the sampling decision in `traceparent_drop`. - "dd=s:0;p:$parent_id;o:France;t.foo:x;t.bar:y;foo:bar;boing:boing"}, - - {__LINE__, "replace invalid characters in origin", - {{"x-datadog-trace-id", "1"}, {"x-datadog-parent-id", "1"}, - {"x-datadog-origin", "France, is a country=nation; so is 台北."}}, - // The "s:-1" comes from the 0% sample rate. - "dd=s:-1;p:$parent_id;o:France_ is a country~nation_ so is ______."}, - - {__LINE__, "replace invalid characters in trace tag key", - {{"x-datadog-trace-id", "1"}, {"x-datadog-parent-id", "1"}, - {"x-datadog-tags", "_dd.p.a;d台北x =foo,_dd.p.ok=bar"}}, - // The "s:-1" comes from the 0% sample rate. - "dd=s:-1;p:$parent_id;t.a_d______x_:foo;t.ok:bar"}, - - {__LINE__, "replace invalid characters in trace tag value", - {{"x-datadog-trace-id", "1"}, {"x-datadog-parent-id", "1"}, - {"x-datadog-tags", "_dd.p.wacky=hello fr~d; how are คุณ?"}}, - // The "s:-1" comes from the 0% sample rate. - "dd=s:-1;p:$parent_id;t.wacky:hello fr_d_ how are _________?"}, - - {__LINE__, "replace equal signs with tildes in trace tag value", - {{"x-datadog-trace-id", "1"}, {"x-datadog-parent-id", "1"}, - {"x-datadog-tags", "_dd.p.base64_thingy=d2Fra2EhIHdhaw=="}}, - // The "s:-1" comes from the 0% sample rate. - "dd=s:-1;p:$parent_id;t.base64_thingy:d2Fra2EhIHdhaw~~"}, - - {__LINE__, "oversized origin truncates it and subsequent fields", - {{"x-datadog-trace-id", "1"}, {"x-datadog-parent-id", "1"}, - {"x-datadog-origin", "long cat is looooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong"}, - {"x-datadog-tags", "_dd.p.foo=bar,_dd.p.honk=honk"}}, - // The "s:-1" comes from the 0% sample rate. - "dd=s:-1;p:$parent_id"}, - - {__LINE__, "oversized trace tag truncates it and subsequent fields", - {{"x-datadog-trace-id", "1"}, {"x-datadog-parent-id", "1"}, - {"x-datadog-tags", "_dd.p.foo=bar,_dd.p.long_cat_is=looooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong,_dd.p.lost=forever"}}, - // The "s:-1" comes from the 0% sample rate. - "dd=s:-1;p:$parent_id;t.foo:bar"}, - - {__LINE__, "oversized extra field truncates itself and subsequent fields", - {{"traceparent", traceparent_drop}, - {"tracestate", "dd=foo:bar;long_cat_is:looooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong;lost:forever"}}, - // The "s:0" comes from the sampling decision in `traceparent_drop`. - "dd=s:0;p:$parent_id;foo:bar"}, - - {__LINE__, "non-Datadog tracestate", - {{"traceparent", traceparent_drop}, - {"tracestate", "foo=bar,boing=boing"}}, - // The "s:0" comes from the sampling decision in `traceparent_drop`. - "dd=s:0;p:$parent_id,foo=bar,boing=boing"}, + {__LINE__, + "sampling priority", + { + {"x-datadog-trace-id", "1"}, + {"x-datadog-parent-id", "1"}, + {"x-datadog-sampling-priority", "2"}, + }, + "dd=s:2;p:$parent_id"}, + + {__LINE__, + "origin", + { + {"x-datadog-trace-id", "1"}, + {"x-datadog-parent-id", "1"}, + {"x-datadog-origin", "France"}, + }, + // The "s:-1" comes from the 0% sample rate. + "dd=s:-1;p:$parent_id;o:France"}, + + {__LINE__, + "trace tags", + { + {"x-datadog-trace-id", "1"}, + {"x-datadog-parent-id", "1"}, + {"x-datadog-tags", "_dd.p.foo=x,_dd.p.bar=y,ignored=wrong_prefix"}, + }, + // The "s:-1" comes from the 0% sample rate. + "dd=s:-1;p:$parent_id;t.foo:x;t.bar:y"}, + + {__LINE__, + "extra fields", + { + {"traceparent", traceparent_drop}, + {"tracestate", "dd=foo:bar;boing:boing"}, + }, + // The "s:0" comes from the sampling decision in `traceparent_drop`. + "dd=s:0;p:$parent_id;foo:bar;boing:boing"}, + + {__LINE__, + "all of the above", + { + {"traceparent", traceparent_drop}, + {"tracestate", "dd=o:France;t.foo:x;t.bar:y;foo:bar;boing:boing"}, + }, + // The "s:0" comes from the sampling decision in `traceparent_drop`. + "dd=s:0;p:$parent_id;o:France;t.foo:x;t.bar:y;foo:bar;boing:boing"}, + + {__LINE__, + "replace invalid characters in origin", + { + {"x-datadog-trace-id", "1"}, + {"x-datadog-parent-id", "1"}, + {"x-datadog-origin", "France, is a country=nation; so is 台北."}, + }, + // The "s:-1" comes from the 0% sample rate. + "dd=s:-1;p:$parent_id;o:France_ is a country~nation_ so is ______."}, + + {__LINE__, + "replace invalid characters in trace tag key", + { + {"x-datadog-trace-id", "1"}, + {"x-datadog-parent-id", "1"}, + {"x-datadog-tags", "_dd.p.a;d台北x =foo,_dd.p.ok=bar"}, + }, + // The "s:-1" comes from the 0% sample rate. + "dd=s:-1;p:$parent_id;t.a_d______x_:foo;t.ok:bar"}, + + {__LINE__, + "replace invalid characters in trace tag value", + { + {"x-datadog-trace-id", "1"}, + {"x-datadog-parent-id", "1"}, + {"x-datadog-tags", "_dd.p.wacky=hello fr~d; how are คุณ?"}, + }, + // The "s:-1" comes from the 0% sample rate. + "dd=s:-1;p:$parent_id;t.wacky:hello fr_d_ how are _________?"}, + + {__LINE__, + "replace equal signs with tildes in trace tag value", + { + {"x-datadog-trace-id", "1"}, + {"x-datadog-parent-id", "1"}, + {"x-datadog-tags", "_dd.p.base64_thingy=d2Fra2EhIHdhaw=="}, + }, + // The "s:-1" comes from the 0% sample rate. + "dd=s:-1;p:$parent_id;t.base64_thingy:d2Fra2EhIHdhaw~~"}, + + {__LINE__, + "oversized origin truncates it and subsequent fields", + { + {"x-datadog-trace-id", "1"}, + {"x-datadog-parent-id", "1"}, + {"x-datadog-origin", + "long cat is " + "looooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo" + "ooo" + "oooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo" + "ooo" + "oooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo" + "ooo" + "ooooooooooooooooooooooooooooooooong"}, + {"x-datadog-tags", "_dd.p.foo=bar,_dd.p.honk=honk"}, + }, + // The "s:-1" comes from the 0% sample rate. + "dd=s:-1;p:$parent_id"}, + + {__LINE__, + "oversized trace tag truncates it and subsequent fields", + { + {"x-datadog-trace-id", "1"}, + {"x-datadog-parent-id", "1"}, + {"x-datadog-tags", + "_dd.p.foo=bar,_dd.p.long_cat_is=" + "looooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo" + "ooo" + "oooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo" + "ooo" + "oooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo" + "ooo" + "ooooooooooooooooooong,_dd.p.lost=forever"}, + }, + // The "s:-1" comes from the 0% sample rate. + "dd=s:-1;p:$parent_id;t.foo:bar"}, + + {__LINE__, + "oversized extra field truncates itself and subsequent fields", + { + {"traceparent", traceparent_drop}, + {"tracestate", + "dd=foo:bar;long_cat_is:" + "looooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo" + "ooo" + "oooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo" + "ooo" + "oooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo" + "ooo" + "ooooooooooooooooooooooooooooooooong;lost:forever"}, + }, + // The "s:0" comes from the sampling decision in `traceparent_drop`. + "dd=s:0;p:$parent_id;foo:bar"}, + + {__LINE__, + "non-Datadog tracestate", + { + {"traceparent", traceparent_drop}, + {"tracestate", "foo=bar,boing=boing"}, + }, + // The "s:0" comes from the sampling decision in `traceparent_drop`. + "dd=s:0;p:$parent_id,foo=bar,boing=boing"}, })); - // clang-format on CAPTURE(test_case.name); CAPTURE(test_case.line); @@ -808,7 +944,7 @@ TEST_CASE("injecting W3C tracestate header") { REQUIRE(logger->error_count() == 0); } -TEST_CASE("128-bit trace ID injection") { +TEST_SPAN("128-bit trace ID injection") { TracerConfig config; config.service = "testsvc"; config.logger = std::make_shared(); @@ -861,146 +997,81 @@ TEST_CASE("128-bit trace ID injection") { REQUIRE(found->second == "deadbeefdeadbeefcafebabecafebabe"); } -TEST_CASE("injection with trace_source option") { +TEST_SPAN("injection behaviour when apm tracing is disabled") { + // Test Case: + // ========== + // Assess only traces to KEEP are propagated, UNLESS, the trace source is set. TracerConfig config; config.service = "testsvc"; config.collector = std::make_shared(); config.logger = std::make_shared(); - config.injection_styles = {PropagationStyle::DATADOG}; + config.tracing_enabled = false; - auto finalized_config = finalize_config(config); + TimePoint current_time = default_clock(); + auto clock = [¤t_time]() { return current_time; }; + + auto finalized_config = finalize_config(config, clock); REQUIRE(finalized_config); Tracer tracer{*finalized_config}; - SECTION("trace_source is not set (default)") { + const auto contains_tracing_context = [](const auto& headers) { + return headers.count("x-datadog-trace-id") == 1 && + headers.count("x-datadog-parent-id") == 1; + }; + + // This span is permitted for propagation as it falls within the 1 + // trace/second window. + { auto span = tracer.create_span(); - InjectionOptions options; - options.trace_source = std::nullopt; MockDictWriter writer; - span.inject(writer, options); + REQUIRE(writer.items.empty()); - const auto& headers = writer.items; - // When there is no trace source, there should be no x-datadog-tags - // header or if there is one, it should not contain _dd.p.ts - if (headers.count("x-datadog-tags") > 0) { - const auto decoded_tags = decode_tags(headers.at("x-datadog-tags")); - REQUIRE(decoded_tags); - auto found = - std::find_if(decoded_tags->begin(), decoded_tags->end(), - [](const auto& tag) { return tag.first == "_dd.p.ts"; }); - REQUIRE(found == decoded_tags->end()); - } + span.inject(writer); + CHECK(contains_tracing_context(writer.items)); } - SECTION("trace_source is 02 (appsec)") { + // Advance the clock but we are still in the same 1 trace/second window. + current_time += 10s; + + // A trace has already been permitted within the current 1 trace/second + // window. Consequently, the following span will be marked as DROPPED and will + // not propagate. + { auto span = tracer.create_span(); - InjectionOptions options; - options.trace_source = {'0', '2'}; MockDictWriter writer; - span.inject(writer, options); - - const auto& headers = writer.items; - REQUIRE(headers.count("x-datadog-tags") == 1); - - const auto decoded_tags = decode_tags(headers.at("x-datadog-tags")); - REQUIRE(decoded_tags); + REQUIRE(writer.items.empty()); - // Check that _dd.p.ts=02 is present in the trace tags - bool found_trace_source = false; - for (const auto& [key, value] : *decoded_tags) { - if (key == "_dd.p.ts" && value == "02") { - found_trace_source = true; - break; - } - } - REQUIRE(found_trace_source); + span.inject(writer); + REQUIRE(writer.items.empty()); } - SECTION("trace source is 02 (appsec) with existing trace tags") { - // Extract a span with existing trace tags - const std::unordered_map headers{ - {"x-datadog-trace-id", "123"}, - {"x-datadog-parent-id", "456"}, - {"x-datadog-sampling-priority", "1"}, - {"x-datadog-tags", "_dd.p.existing=value,_dd.p.another=test"}}; - MockDictReader reader{headers}; - auto span = tracer.extract_span(reader); - REQUIRE(span); - - InjectionOptions options; - options.trace_source = {'0', '2'}; + // The following span set the trace source, thus it is propagated even if we + // are still within the 1 trace/second window. + { + auto span = tracer.create_span(); + span.set_source(Source::appsec); MockDictWriter writer; - span->inject(writer, options); - - const auto& output_headers = writer.items; - REQUIRE(output_headers.count("x-datadog-tags") == 1); - - const auto decoded_tags = decode_tags(output_headers.at("x-datadog-tags")); - REQUIRE(decoded_tags); - - // Check that _dd.p.ts=02 is present along with existing tags - bool found_trace_source = false; - bool found_existing = false; - bool found_another = false; - - for (const auto& [key, value] : *decoded_tags) { - if (key == "_dd.p.ts" && value == "02") { - found_trace_source = true; - } else if (key == "_dd.p.existing" && value == "value") { - found_existing = true; - } else if (key == "_dd.p.another" && value == "test") { - found_another = true; - } - } + REQUIRE(writer.items.empty()); - REQUIRE(found_trace_source); - REQUIRE(found_existing); - REQUIRE(found_another); + span.inject(writer); + CHECK(contains_tracing_context(writer.items)); } - SECTION("trace_source with APM tracing disabled") { - // Test the scenario where APM tracing is disabled - TracerConfig apm_disabled_config; - apm_disabled_config.service = "testsvc"; - apm_disabled_config.collector = std::make_shared(); - apm_disabled_config.logger = std::make_shared(); - apm_disabled_config.injection_styles = { - PropagationStyle::DATADOG, PropagationStyle::B3, PropagationStyle::W3C}; - // Disable APM tracing - apm_disabled_config.apm_tracing_enabled = false; - - auto apm_disabled_finalized_config = finalize_config(apm_disabled_config); - REQUIRE(apm_disabled_finalized_config); - Tracer apm_disabled_tracer{*apm_disabled_finalized_config}; + // Progress beyond the current window; a span without trace state will be + // retained + current_time += 1min; - SECTION( - "sampling priority != 2 (USER-KEEP) and no _dd.p.ts - headers should " - "be cleared") { - auto span = apm_disabled_tracer.create_span(); - span.trace_segment().override_sampling_priority(1); + // Following span should be propagated. + { + auto span = tracer.create_span(); - InjectionOptions options; - options.trace_source = {'0', '2'}; + MockDictWriter writer; + REQUIRE(writer.items.empty()); - MockDictWriter writer; - span.inject(writer, options); - - const auto& headers = writer.items; - // all headers should be empty when APM tracing is disabled and sampling - // priority != 2 - REQUIRE(headers.at("x-datadog-trace-id").empty()); - REQUIRE(headers.at("x-datadog-parent-id").empty()); - REQUIRE(headers.at("x-datadog-sampling-priority").empty()); - REQUIRE(headers.at("x-datadog-origin").empty()); - REQUIRE(headers.at("x-datadog-tags").empty()); - REQUIRE(headers.at("x-b3-traceid").empty()); - REQUIRE(headers.at("x-b3-spanid").empty()); - REQUIRE(headers.at("x-b3-sampled").empty()); - REQUIRE(headers.at("traceparent").empty()); - REQUIRE(headers.at("tracestate").empty()); - } + span.inject(writer); + CHECK(contains_tracing_context(writer.items)); } } diff --git a/test/test_tracer.cpp b/test/test_tracer.cpp index 8c994034..ab53bf16 100644 --- a/test/test_tracer.cpp +++ b/test/test_tracer.cpp @@ -48,11 +48,14 @@ std::ostream& operator<<(std::ostream& stream, } // namespace datadog using namespace datadog::tracing; +using namespace std::chrono_literals; + +#define TEST_TRACER(x) TEST_CASE(x, "[tracer]") // Verify that the `.defaults.*` (`SpanDefaults`) properties of a tracer's // configuration do determine the default properties of spans created by the // tracer. -TEST_CASE("tracer span defaults") { +TEST_TRACER("tracer span defaults") { TracerConfig config; config.service = "foosvc"; config.service_type = "crawler"; @@ -307,7 +310,7 @@ TEST_CASE("tracer span defaults") { } } -TEST_CASE("span extraction") { +TEST_TRACER("span extraction") { TracerConfig config; config.service = "testsvc"; const auto collector = std::make_shared(); @@ -983,9 +986,11 @@ TEST_CASE("span extraction") { "wow", // tracestate 0, // expected_sampling_priority "France", // expected_origin - {{"_dd.p.foo", "thing1"}, - {"_dd.p.bar", "thing2"}}, // expected_trace_tags - nullopt, // expected_additional_w3c_tracestate + { + {"_dd.p.foo", "thing1"}, + {"_dd.p.bar", "thing2"}, + }, // expected_trace_tags + nullopt, // expected_additional_w3c_tracestate "x:wow;y:wow", // expected_additional_datadog_w3c_tracestate "00000000000d69ac", // expected_datadog_w3c_parent_id }, @@ -1145,6 +1150,47 @@ TEST_CASE("span extraction") { nullopt, // expected_additional_datadog_w3c_tracestate "0000000000000000", // expected_datadog_w3c_parent_id, }, + + { + __LINE__, + "invalid trace state (1/2)", + traceparent_keep, + "dd=ts:0001", + 1, + nullopt, + {}, + nullopt, + nullopt, + "0000000000000000", // expected_datadog_w3c_parent_id, + }, + + { + __LINE__, + "invalid trace state (2/2)", + traceparent_keep, + "dd=ts:AA", + 1, + nullopt, + {}, + nullopt, + nullopt, + "0000000000000000", // expected_datadog_w3c_parent_id, + }, + + { + __LINE__, + "valid trace state", + traceparent_keep, + "dd=o:dsm;ts:04", + 1, + "dsm", + { + {"_dd.p.ts", "04"}, + }, + nullopt, + nullopt, + "0000000000000000", // expected_datadog_w3c_parent_id, + }, })); CAPTURE(test_case.name); @@ -1393,7 +1439,7 @@ TEST_CASE("span extraction") { } } -TEST_CASE("baggage usage") { +TEST_TRACER("baggage usage") { TracerConfig config; config.logger = std::make_shared(); config.collector = std::make_shared(); @@ -1434,7 +1480,7 @@ TEST_CASE("baggage usage") { } } -TEST_CASE("report hostname") { +TEST_TRACER("report hostname") { TracerConfig config; config.service = "testsvc"; config.collector = std::make_shared(); @@ -1456,7 +1502,7 @@ TEST_CASE("report hostname") { } } -TEST_CASE("128-bit trace IDs") { +TEST_TRACER("128-bit trace IDs") { // Use a clock that always returns a hard-coded `TimePoint`. // May 6, 2010 14:45:13 America/New_York const std::time_t flash_crash = 1273171513; @@ -1566,7 +1612,7 @@ TEST_CASE("128-bit trace IDs") { REQUIRE(*high == trace_id.high); } -TEST_CASE( +TEST_TRACER( "_dd.p.tid invalid or inconsistent with trace ID results in error tag") { struct TestCase { int line; @@ -1618,7 +1664,7 @@ TEST_CASE( test_case.expected_error_prefix + test_case.tid_tag_value); } -TEST_CASE("heterogeneous extraction") { +TEST_TRACER("heterogeneous extraction") { // These test cases verify that when W3C is among the configured extraction // styles, then non-Datadog and unexpected Datadog fields in an incoming // `tracestate` are extracted, under certain conditions, even when trace @@ -1731,7 +1777,7 @@ TEST_CASE("heterogeneous extraction") { REQUIRE(writer.items == test_case.expected_injected_headers); } -TEST_CASE("move semantics") { +TEST_TRACER("move semantics") { // Verify that `Tracer` can be moved. TracerConfig config; config.service = "testsvc"; @@ -1747,136 +1793,231 @@ TEST_CASE("move semantics") { (void)tracer2; } -TEST_CASE("apm_tracing_enabled behavior") { - TracerConfig base_config; - base_config.service = "testsvc"; - base_config.name = "test.op"; +TEST_TRACER("APM tracing disabled") { + TracerConfig config; + config.service = "testsvc"; + config.name = "test.op"; auto collector = std::make_shared(); - base_config.collector = collector; - base_config.logger = std::make_shared(); + config.collector = collector; + config.logger = std::make_shared(); + config.tracing_enabled = false; TimePoint current_time = default_clock(); auto clock = [¤t_time]() { return current_time; }; - SECTION( - "APM tracing disabled - span with _dd.p.ts is kept, _dd.apm.enabled tag " - "added") { - TracerConfig config = base_config; - config.apm_tracing_enabled = false; + SECTION("sampling behaviour") { + SECTION("span with _dd.p.ts is kept") { + auto finalized_config = finalize_config(config, clock); + REQUIRE(finalized_config); + REQUIRE(!finalized_config->tracing_enabled); + Tracer tracer{*finalized_config}; - auto finalized_config = finalize_config(config, clock); - REQUIRE(finalized_config); - REQUIRE(!finalized_config->apm_tracing_enabled); - Tracer tracer{*finalized_config}; + { + auto span = tracer.create_span(); + span.set_source(Source::appsec); + } - SpanConfig span_cfg; - span_cfg.tags[tags::internal::trace_source] = "02"; - { tracer.create_span(span_cfg); } + REQUIRE(collector->chunks.size() == 1); + REQUIRE(collector->chunks.front().size() == 1); + const datadog::tracing::SpanData& span_data = + *collector->chunks.front().front(); - REQUIRE(collector->chunks.size() == 1); - REQUIRE(collector->chunks.front().size() == 1); - const datadog::tracing::SpanData& span_data = - *collector->chunks.front().front(); + CHECK(span_data.tags.at("_dd.p.dm") == "-5"); + CHECK(span_data.numeric_tags.at(tags::internal::apm_enabled) == 0); + CHECK(span_data.numeric_tags.at(tags::internal::sampling_priority) == 2); + } - REQUIRE(span_data.tags.at("_dd.p.dm") == "-5"); - REQUIRE(span_data.numeric_tags.at(tags::internal::apm_enabled) == 0); - REQUIRE(span_data.numeric_tags.at(tags::internal::sampling_priority) == 2); - } + SECTION("spans without _dd.p.ts are rate limited to 1/min") { + auto finalized_config = finalize_config(config, clock); + REQUIRE(finalized_config); + Tracer tracer{*finalized_config}; + { auto root1 = tracer.create_span(); } + REQUIRE(collector->chunks.size() == 1); + REQUIRE(collector->chunks.front().size() == 1); + + { + const datadog::tracing::SpanData& span1_data = + *collector->chunks.front().front(); + CHECK(span1_data.numeric_tags.at(tags::internal::sampling_priority) == + 2); + CHECK(span1_data.numeric_tags.at(tags::internal::apm_enabled) == 0); + CHECK(span1_data.tags.at("_dd.p.dm") == "-0"); + } - SECTION("APM tracing disabled - no _dd.p.ts - rate limited to 1/min") { - TracerConfig config = base_config; - config.apm_tracing_enabled = false; + collector->chunks.clear(); - auto finalized_config = finalize_config(config, clock); - REQUIRE(finalized_config); - Tracer tracer{*finalized_config}; - { auto root1 = tracer.create_span(); } - REQUIRE(collector->chunks.size() == 1); - REQUIRE(collector->chunks.front().size() == 1); - const datadog::tracing::SpanData& span1_data = - *collector->chunks.front().front(); - CHECK(span1_data.numeric_tags.at(tags::internal::sampling_priority) == 2); - CHECK(span1_data.numeric_tags.at(tags::internal::apm_enabled) == 0); - CHECK(std::stoi(span1_data.tags.at("_dd.p.dm")) < -10); + { + current_time += 1s; // Advance clock a bit, still within 1 min window + tracer.create_span(); + } - collector->chunks.clear(); + REQUIRE(collector->chunks.size() == 1); + REQUIRE(collector->chunks.front().size() == 1); + + // Expect the span to be dropped because we already ingested 1 trace in + // the current 1 min window. + { + const datadog::tracing::SpanData& span2_data = + *collector->chunks.front().front(); + CHECK(span2_data.numeric_tags.at(tags::internal::sampling_priority) == + -1); + CHECK(span2_data.numeric_tags.at(tags::internal::apm_enabled) == 0); + } - current_time += - std::chrono::seconds(1); // Advance clock a bit, still within 1 min - { auto root2 = tracer.create_span(); } - REQUIRE(collector->chunks.size() == 1); - REQUIRE(collector->chunks.front().size() == 1); - const datadog::tracing::SpanData& span2_data = - *collector->chunks.front().front(); - CHECK(span2_data.numeric_tags.at(tags::internal::sampling_priority) == -1); - CHECK(span2_data.numeric_tags.at(tags::internal::apm_enabled) == 0); + collector->chunks.clear(); - collector->chunks.clear(); + { + auto span = tracer.create_span(); + span.set_source(Source::appsec); + } - current_time += std::chrono::minutes(1) + - std::chrono::seconds(1); // Advance clock past 1 min - { auto root3 = tracer.create_span(); } - REQUIRE(collector->chunks.size() == 1); - REQUIRE(collector->chunks.front().size() == 1); - const auto& span3_data = *collector->chunks.front().front(); - CHECK(span2_data.numeric_tags.at(tags::internal::sampling_priority) == 2); - CHECK(span3_data.numeric_tags.at(tags::internal::apm_enabled) == 0); - } + REQUIRE(collector->chunks.size() == 1); + REQUIRE(collector->chunks.front().size() == 1); + + // Expect the span to be kept because the trace source is set. + { + const datadog::tracing::SpanData& span2_data = + *collector->chunks.front().front(); + CHECK(span2_data.numeric_tags.at(tags::internal::sampling_priority) == + 2); + CHECK(span2_data.numeric_tags.at(tags::internal::apm_enabled) == 0); + } - SECTION("APM tracing disabled - extracted context behavior") { - TracerConfig config = base_config; - config.apm_tracing_enabled = false; + collector->chunks.clear(); + { + current_time += 1min + 1s; // Advance clock past 1 min + tracer.create_span(); + } + + REQUIRE(collector->chunks.size() == 1); + REQUIRE(collector->chunks.front().size() == 1); + + // Expect to be ingested. + { + const auto& span3_data = *collector->chunks.front().front(); + CHECK(span3_data.numeric_tags.at(tags::internal::sampling_priority) == + 2); + CHECK(span3_data.numeric_tags.at(tags::internal::apm_enabled) == 0); + } + } + } + + SECTION("extracted context behavior") { auto finalized_config = finalize_config(config, clock); REQUIRE(finalized_config); Tracer tracer{*finalized_config}; - // Case 1: extracted context with priority, but no _dd.p.ts → dropped - // To ensure this, let's make this the *second* span in this disabled state - // for this test section. The first consumes the limiter slot. - { auto dummy_span = tracer.create_span(); } + // When APM Tracing is disabled, we allow one trace per second for service + // liveness. To ensure consistency, consume the limiter slot. + { tracer.create_span(); } collector->chunks.clear(); - const std::unordered_map headers_with_priority{ - {"x-datadog-trace-id", "123"}, - {"x-datadog-parent-id", "456"}, - {"x-datadog-sampling-priority", "2"} // USER_KEEP - }; - MockDictReader reader_with_priority{headers_with_priority}; - { - auto span = tracer.extract_span(reader_with_priority); - REQUIRE(span); - } - REQUIRE(collector->chunks.size() == 1); - REQUIRE(collector->chunks.front().size() == 1); - const SpanData& span1_data = *collector->chunks.front().front(); - // although incoming priority was USER_KEEP, we should still drop it - CHECK(span1_data.numeric_tags.at(tags::internal::sampling_priority) == -1); - CHECK(span1_data.numeric_tags.at(tags::internal::apm_enabled) == 0.); + // Case 1: extracted context with priority, but no `_dd.p.ts` → depends if + // local spans are marked by a product. + SECTION("no trace source extracted") { + const std::unordered_map headers_with_priority{ + {"x-datadog-trace-id", "123"}, + {"x-datadog-parent-id", "456"}, + {"x-datadog-sampling-priority", "2"} // USER_KEEP + }; - collector->chunks.clear(); + SECTION( + "tracer apply its own sampling decision in accordance with the " + "locally enabled product") { + { + MockDictReader reader{headers_with_priority}; + auto span = tracer.extract_span(reader); + REQUIRE(span); + } + + REQUIRE(collector->chunks.size() == 1); + REQUIRE(collector->chunks.front().size() == 1); + + // although incoming priority was USER_KEEP, we should still drop it + // because we already consumed the only slot from the limiter. + { + const SpanData& span_data = *collector->chunks.front().front(); + CHECK(span_data.numeric_tags.at(tags::internal::sampling_priority) == + -1); + CHECK(span_data.numeric_tags.at(tags::internal::apm_enabled) == 0.); + collector->chunks.clear(); + } + + // Mark the span as generated by the Appsec product. This should ensure + // the span is retained. + { + MockDictReader reader{headers_with_priority}; + auto span = tracer.extract_span(reader); + REQUIRE(span); + span->set_source(Source::appsec); + } + + REQUIRE(collector->chunks.size() == 1); + REQUIRE(collector->chunks.front().size() == 1); + + { + const SpanData& span_data = *collector->chunks.front().front(); + CHECK(span_data.numeric_tags.at(tags::internal::sampling_priority) == + 2); + CHECK(span_data.tags.at(tags::internal::decision_maker) == "-5"); + CHECK(span_data.tags.at(tags::internal::trace_source) == + to_tag(Source::appsec)); + CHECK(span_data.numeric_tags.at(tags::internal::apm_enabled) == 0.); + collector->chunks.clear(); + } + + // Advance the clock to reset the limiter. + current_time += 1min + 10s; + + // This span qualifies as the one trace per minute allowed for service + // liveness, so it will be retained. + { + MockDictReader reader{headers_with_priority}; + auto span = tracer.extract_span(reader); + REQUIRE(span); + } + + REQUIRE(collector->chunks.size() == 1); + REQUIRE(collector->chunks.front().size() == 1); + + { + const SpanData& span_data = *collector->chunks.front().front(); + CHECK(span_data.numeric_tags.at(tags::internal::sampling_priority) == + 2); + CHECK(span_data.tags.at(tags::internal::decision_maker) == "-0"); + CHECK(span_data.numeric_tags.at(tags::internal::apm_enabled) == 0.); + collector->chunks.clear(); + } + } + } // Case 2: Extracted context with priority AND _dd.p.ts -> Kept by AppSec - // rule - current_time += - std::chrono::minutes(1) + std::chrono::seconds(1); // Refresh limiter - const std::unordered_map - headers_with_priority_and_appsec{ - {"x-datadog-trace-id", "789"}, - {"x-datadog-parent-id", "101"}, - {"x-datadog-sampling-priority", - "-1"}, // USER_DROP, to show _dd.p.ts overrides - {tags::internal::trace_source, "02"}}; - MockDictReader reader_with_priority_and_appsec{ - headers_with_priority_and_appsec}; - { - auto span = tracer.extract_span(reader_with_priority_and_appsec); - REQUIRE(span); + // rule. + SECTION("trace source is extracted") { + const std::unordered_map + headers_with_priority_and_appsec{ + {"x-datadog-trace-id", "789"}, + {"x-datadog-parent-id", "101"}, + // USER_DROP, to show _dd.p.ts overrides + {"x-datadog-sampling-priority", "-1"}, + {"x-datadog-tags", "_dd.p.ts=02"}}; + + { + MockDictReader reader{headers_with_priority_and_appsec}; + auto span = tracer.extract_span(reader); + REQUIRE(span); + } + + REQUIRE(collector->chunks.size() == 1); + REQUIRE(collector->chunks.front().size() == 1); + + { + const SpanData& span2_data = *collector->chunks.front().front(); + CHECK(span2_data.numeric_tags.at(tags::internal::sampling_priority) == + 2); + CHECK(span2_data.numeric_tags.at(tags::internal::apm_enabled) == 0); + } } - REQUIRE(collector->chunks.size() == 1); - REQUIRE(collector->chunks.front().size() == 1); - const SpanData& span2_data = *collector->chunks.front().front(); - CHECK(span2_data.numeric_tags.at(tags::internal::sampling_priority) == 2); - CHECK(span2_data.numeric_tags.at(tags::internal::apm_enabled) == 0); } } diff --git a/test/test_tracer_config.cpp b/test/test_tracer_config.cpp index 21476290..712f98d8 100644 --- a/test/test_tracer_config.cpp +++ b/test/test_tracer_config.cpp @@ -280,14 +280,14 @@ TRACER_CONFIG_TEST("DD_APM_TRACING_ENABLED") { const EnvGuard guard{"DD_APM_TRACING_ENABLED", ""}; auto finalized = finalize_config(config); REQUIRE(finalized); - REQUIRE(finalized->apm_tracing_enabled); + REQUIRE(finalized->tracing_enabled); } SECTION("can be set to false") { const EnvGuard guard{"DD_APM_TRACING_ENABLED", "false"}; auto finalized = finalize_config(config); REQUIRE(finalized); - REQUIRE(!finalized->apm_tracing_enabled); + REQUIRE(!finalized->tracing_enabled); } } From ff2c7eab8af1afb14018101430f3156d068fe9eb Mon Sep 17 00:00:00 2001 From: Damien Mehala Date: Thu, 24 Jul 2025 11:36:56 +0200 Subject: [PATCH 6/6] fix trace source validation --- src/datadog/trace_source.cpp | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/datadog/trace_source.cpp b/src/datadog/trace_source.cpp index 6bfdc008..c962d864 100644 --- a/src/datadog/trace_source.cpp +++ b/src/datadog/trace_source.cpp @@ -11,11 +11,7 @@ bool validate_trace_source(StringView source_str) { auto maybe_ts_uint = parse_uint64(source_str, 10); if (maybe_ts_uint.if_error()) return false; - // Bit twiddling magic is coming from - // <3. - auto is_power_of_2 = [](uint64_t v) -> bool { return v && !(v & (v - 1)); }; - - return is_power_of_2(*maybe_ts_uint); + return true; } } // namespace tracing