Conversation
…tats_computation gate The header was still sent unconditionally based on the INI flag alone in ext/coms.c and ext/auto_flush.c, while serializer.c already gates actual stats computation on ddog_agent_has_stats_computation(). Add the same agent-capability check to both header-emission sites so the header value matches the serializer behaviour and the TRACE_STATS_COMPUTATION_CLIENT_DROP_P0S_FALSE system test passes.
🎉 All green!❄️ No new flaky tests detected 🎯 Code Coverage (details) 🔗 Commit SHA: 509ad2e | Docs | Datadog PR Page | Give us feedback! |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 509ad2e340
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| dd_append_header(&list, "Datadog-Meta-Lang-Version", php_version_rt.ptr, php_version_rt.len); | ||
| dd_append_header(&list, "Datadog-Meta-Tracer-Version", ZEND_STRL(PHP_DDTRACE_VERSION)); | ||
| if (!get_global_DD_APM_TRACING_ENABLED() || (ddtrace_sidecar_for_signal && get_global_DD_TRACE_STATS_COMPUTATION_ENABLED())) { | ||
| if (!get_global_DD_APM_TRACING_ENABLED() || (ddtrace_sidecar_for_signal && get_global_DD_TRACE_STATS_COMPUTATION_ENABLED() && ddog_agent_has_stats_computation())) { |
There was a problem hiding this comment.
Avoid freezing the stats header before agent info is ready
In background-sender mode with a sidecar present for telemetry/appsec, this header list is built once by ddtrace_coms_init_and_start_writer() during the first dd_rinit_once(), but dd_initialize_request() only calls ddtrace_agent_info_rinit()/ddtrace_apply_agent_info() later. At that point ddog_agent_has_stats_computation() is still false, so Datadog-Client-Computed-Stats is permanently omitted from the cached dd_agent_curl_headers even after agent info later enables stats and the serializer starts feeding the concentrator. This makes the legacy sender under-report the header for agents that do support client-side stats; the gate needs to be evaluated when per-request headers are assembled or after agent info is applied.
Useful? React with 👍 / 👎.
Problem
Commit 68e11c7 ("Check for agent version in client side stats") added
ddog_agent_has_stats_computation()as a gate inserializer.cto suppress stats computation when the agent doesn't reportclient_drop_p0s: true. However, theDatadog-Client-Computed-Stats: trueHTTP header — sent with every trace payload — was not updated in two other locations:ext/coms.c(legacy background sender path)ext/auto_flush.c(sidecar sender path)Both still check only
DD_TRACE_STATS_COMPUTATION_ENABLED(an INI flag), so the header is sent astrueeven when the agent gate would suppress actual stats computation. The system testTRACE_STATS_COMPUTATION_CLIENT_DROP_P0S_FALSEfails on every CI pipeline as a result.Fix
Replace
get_global_DD_TRACE_STATS_COMPUTATION_ENABLED()/get_DD_TRACE_STATS_COMPUTATION_ENABLED()withddog_agent_has_stats_computation()in both locations, making the header consistent with the serializer gate.Also adds
#include <components-rs/ddtrace.h>toext/auto_flush.cwhere the declaration was not yet visible.Test
System test:
TRACE_STATS_COMPUTATION_CLIENT_DROP_P0S_FALSE