Skip to content

Implement DD_APM_TRACING_ENABLED=false #217

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

cataphract
Copy link
Contributor

@cataphract cataphract commented Jun 3, 2025

Description

Implement DD_APM_TRACING_ENABLED=false, as described in the relevant RFC ("Configuration to Disable APM Tracing"). It also tries to keep at least about roughly 1 trace/second, so the application is deemed active.

@cataphract cataphract requested a review from a team as a code owner June 3, 2025 14:21
@cataphract cataphract requested review from dubloom and removed request for a team June 3, 2025 14:21
@cataphract cataphract force-pushed the glopes/dd-trace-enabled=false branch from c624e7c to 6063d6a Compare June 3, 2025 14:37
@cataphract cataphract requested review from dmehala, pablomartinezbernardo and Copilot and removed request for dubloom June 3, 2025 14:37
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR introduces a mechanism to disable APM tracing via DD_APM_TRACING_ENABLED=false, while allowing at least one trace per second to be sampled when disabled, and propagates the new config flag through agent, tracer, and telemetry components.

  • Added ApmDisabledTraceSampler for rate-limited sampling when tracing is disabled
  • Introduced apm_tracing_enabled flag across config loading, agent, tracer, and telemetry
  • Switched to ErasedTraceSampler to unify sampler interfaces

Reviewed Changes

Copilot reviewed 30 out of 30 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/datadog/trace_sampler.cpp Implemented ApmDisabledTraceSampler, updated erasure model
src/datadog/telemetry/telemetry_impl.cpp Added APM_TRACING_ENABLED serialization in telemetry names
src/datadog/tags.{h,cpp} Declared _dd.p.ts and _dd.apm.enabled internal tags
src/datadog/extraction_util.cpp Extended trace tag parsing for _dd.p.ts
src/datadog/datadog_agent_config.{h,cpp} Loaded and finalized apm_tracing_enabled from env/user
src/datadog/datadog_agent.{h,cpp} Stored flag on agent and conditionally set HTTP header
src/datadog/config_manager.{h,cpp} Wrapped TraceSampler in ErasedTraceSampler
include/datadog/{tracer_config,tracer}.h Exposed APM tracing flag and disabled sampler in tracer API
include/datadog/trace_segment.h Added member apm_tracing_enabled_ to TraceSegment
include/datadog/{collector,config,environment}.h Updated enums and environment lookup for new flag
include/datadog/sampling_mechanism.h Updated enum comment for APP_SEC mechanism
include/datadog/injection_options.h Added has_appsec_matches field
Comments suppressed due to low confidence (2)

include/datadog/tracer.h:42

  • [nitpick] The field name apm_tracing_disabled_sampler_ is potentially confusing; consider renaming to something like apm_disabled_sampler_ or disabled_trace_sampler_ to clarify its intent.
std::shared_ptr<ErasedTraceSampler> apm_tracing_disabled_sampler_;

src/datadog/trace_sampler.cpp:128

  • Consider adding unit tests for ApmDisabledTraceSampler to verify rate-limiting behavior and ensure that at least one trace per second is retained when tracing is disabled.
SamplingDecision ApmDisabledTraceSampler::decide(const SpanData& span_data) {

@cataphract cataphract force-pushed the glopes/dd-trace-enabled=false branch from e7329bb to 749f18b Compare June 3, 2025 16:24
@cataphract cataphract changed the title Implement DD_TRACE_ENABLED=false Implement DD_APM_TRACING_ENABLED=false Jun 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant