Update libdatadog; bgs telemetry fix - #4073
Merged
Merged
Conversation
Update libdatadog so set_session_config no longer drains every runtime in the session for non-fork connections. A sibling FPM worker could otherwise delete an active runtime, causing queued telemetry actions to be discarded after it is recreated without applications.
The in-process background sender (tracer/coms.c, the default on non-Windows
PHP below 8.3 except where the sidecar sender is forced, such as Lambda) counts
logical trace-send operations and their final outcomes and reports them as
tracers.trace_api.{requests,responses,errors}. They are flushed on a queue id
of their own, and the sidecar drops any payload whose queue has no application
registered ("No application found for instance ... and queue_id ..."). Nothing
registered one, so every flush was discarded. Since b026bfc (2026-06-02)
introduced process-global accumulation, each due flush has also drained those
counters with atomic_exchange() before handing them over, throwing that
interval's counters away.
What broke it, and when
-----------------------
91222ad ("feat: reduce telemetry sent", #3316, 2025-07-28) updated
libdatadog and adapted the telemetry calls. In
ddtrace_telemetry_register_services() it dropped
ddog_sidecar_telemetry_flushServiceData(
&sidecar, ddtrace_sidecar_instance_id, &dd_bgs_queued_id, meta,
DDOG_CHARSLICE_C("background_sender-php-service"),
DDOG_CHARSLICE_C("none"));
which was precisely that queue's application registration, leaving only the
registration-only buffer flush and its "FIXME: it seems we must call
enqueue_actions (even with an empty list of actions) for things to work
properly".
The libdatadog rework it pulled in (de42dc229, "feat(sidecar): add telemetry
clients expiration", #1077, same day) had just replaced the AppOrQueue
buffering - where enqueue_actions on an unknown queue id created an entry and
held the actions until a service turned up - with a hard requirement that the
application already exist. Before that rework the missing registration would
merely have delayed the points; after it, it discards them. Both halves
shipped together in 1.11.0 (2025-07-29).
Confirmed in the field
----------------------
No version >= 1.11.0 has produced a single trace_api.* point from
instrumented_service:background_sender-php-service in any window queried;
points exist only for <= 1.10.0. Controls that rule out a bad query or a
missing fleet: tracers.spans_created{tracer_version:1.11.0} confirms a large
pre-8.3 population on >= 1.11.0 that reports other tracers metrics normally;
weekly trace_api volume was flat-to-rising before 1.11.0 and decays
monotonically from the week after it shipped, leaving it substantially lower
to date.
Beware a name collision when checking this: the sidecar's own self telemetry
registers the same four metric names in the same namespace and tags them with
the PHP tracer version, and it accounts for 93% of all PHP trace_api volume.
Discriminate on instrumented_service (background_sender-php-service vs
datadog-ipc-helper) or on language_version (a real version vs the literal
"sidecar").
The fix
-------
- ddtrace_telemetry_bgs_init() generates the queue id once per process from
ddtrace_activate_once(), only in the branch that actually starts the sender,
so Windows, and PHP 8.3+ with the default sender configuration, never
register an application for metrics they do not produce.
- dd_bgs_register_application() announces the application from
ddtrace_telemetry_finalize(). Not from the post-connect callback, where the
old registration lived: dd_activate_once() connects the sidecar before it
calls ddtrace_activate_once(), so on the first connection the queue id does
not exist yet. A process-global atomic records whether registration was
successfully queued on a transport; the post-connect callback
(ddtrace_telemetry_register_services) clears it, so the next finalize
re-announces the application after a reconnect or when a new ZTS thread
establishes a transport.
- While unregistered, ddtrace_telemetry_flush_bgs_metrics_if_due() keeps
accumulating instead of draining into the void.
- The final flush also hands the application back with
ddog_sidecar_application_remove(): nothing else in the tracer explicitly
removes it. When remote config is enabled, set_universal_service_tags() can
attach a subscription to the application, so leaving it behind can pin that
subscription for the life of the runtime.
Attribution is unchanged: the application carries the same synthetic
background_sender-php-service / none pair as before 1.11.0 (these counters
describe the sender, not the traced application), so queries written against
the old data keep working. tests/ext/telemetry/{simple,broken_pipe}.phpt still
filter that service name out of the telemetry they inspect; those branches have
been dead since 1.11.0 and are live again.
Why the final flush is in MSHUTDOWN and not GSHUTDOWN
-----------------------------------------------------
By GSHUTDOWN the process-wide datadog_sidecar_instance_id is already gone,
even where the per-thread transport is still alive. MSHUTDOWN calls
datadog_sidecar_shutdown(), which NULLs that id - the id both the buffer flush
and the application removal use to address the application - and, in thread
mode on the master process, also drops the main thread's transport. The engine
runs the globals dtor (PHP_GSHUTDOWN(datadog) -> ddtrace_gshutdown()) only
after module_shutdown_func has returned, in NTS and ZTS alike. A flush from
there can never work in either threading mode; probes printed a live id in
MSHUTDOWN and (nil) in GSHUTDOWN on the same thread.
The call therefore sits after background-sender teardown has synchronously
produced the last counters, and before datadog_sidecar_shutdown() drops the
id: a point at which the final metrics exist and can still be sent. One
unconditional call site, with no ZTS branch, because the application is keyed
by (session id, runtime id, queue id), all three process-wide, so any thread's
connection can address it; only the transport is per-thread.
Tests
-----
Two integration test classes, run on 8.2 release and release-zts.
TelemetryBackgroundSenderTests covers the request path: counters produced by
request traffic and flushed by a later request.
TelemetryBackgroundSenderShutdownTests covers what only happens as a process
goes away - the metrics accumulated since the last flush - using a
single-request CLI process. Its trace is queued before telemetry finalize, and
the sender is synchronously drained later in MSHUTDOWN, exercising the
process-exit path. Both tests pass on both variants; in negative-control runs
with the MSHUTDOWN flush gated off, the shutdown case fails on both variants
and the request-path one keeps passing, which is the whole point of the split.
The split is what makes that discrimination possible. Every process reports
under the same synthetic service and the sidecar merges same-service telemetry
into a single worker, so a payload says nothing about which process produced it:
a container that has served requests cannot be used to prove anything about
shutdown. The shutdown class therefore owns a container in which no request is
ever served.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
cataphract
requested review from
dd-oleksii and
leoromanovsky
and removed request for
a team
July 28, 2026 10:29
|
bwoebi
approved these changes
Jul 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Updates libdatadog to include a fix for spurious runtimeinfo reclamation, and fixes bgs telemetry.
Why this, hopefully, extended telemetry test 1) flaky failures and 2) error messages about the missing application for bgs telemetry should stop
Also includes some fixes for appsec telemetry tests and cmake tracer build in light of the move of the tracer code.
Reviewer checklist