feat(process-tags): signal service name source via svc.user/svc.auto#3921
feat(process-tags): signal service name source via svc.user/svc.auto#3921Leiyks wants to merge 6 commits into
Conversation
|
✨ Fix all issues with BitsAI or with Cursor
|
Benchmarks [ tracer ]Benchmark execution time: 2026-06-02 15:21:16 Comparing candidate commit 84b004e in PR branch Found 0 performance improvements and 6 performance regressions! Performance is the same for 188 metrics, 0 unstable metrics.
|
f56593e to
642cf4d
Compare
Emit one of two mutually-exclusive process tags so the Datadog backend can distinguish user-set vs tracer-resolved service names: - `svc.user:true` when DD_SERVICE is non-empty (env, INI, OTEL fallback, remote config) - `svc.auto:<default_service_name>` when DD_SERVICE is empty and the tracer auto-resolved the name Runtime mutations of `datadog.service` (via `ini_set` or remote config) trigger a recompute. The new `ddtrace_process_tags_reload_with_service` override threads the ZAI `new_str` through because the value is not yet committed to `get_DD_SERVICE()` at the time the `ini_change` callback fires. Implements: RFC "Signal Service Name Source via Process Tags" https://docs.google.com/document/d/1c47iSTWxIOHMHfZTF2nT9xfyQaIBP9KJvI9sRn5SvpM
Addresses senior review on the prior PR commit. Process tags are per-process (set once, propagated by the sidecar), but the active service name in PHP is request-local (mutable via `ini_set` and OTEL/RC fallbacks). Baking `svc.user`/`svc.auto` into the static process_tags string leaked the latest request's override into subsequent FPM requests. Two cooperating paths now: 1. **Per-span** (`ext/serializer.c::ddtrace_serialize_span_to_rust_span`): computes svc.user/svc.auto from `get_DD_SERVICE()` at serialization time and appends to that span's `_dd.tags.process`. Each span sees exactly its own request's state — no cross-request leak. 2. **Sidecar** (`ext/sidecar.c::ddtrace_sidecar_update_process_tags`): sends the process-level svc source to libdatadog via the new `ddog_sidecar_session_set_default_service_name` FFI. The sidecar injects svc.user/svc.auto into outgoing telemetry/RC/runtime_info payloads at emission time, eliminating the static-string conflict. The libdatadog half is in DataDog/libdatadog#2053; the submodule is bumped here to that commit. Reverts the static svc.* emission and `ddtrace_alter_dd_service` reload hook from 5a55f2d. Tests: - 5 new `.phpt` tests (CLI per-span correctness incl. ini_set + ini_restore) - New PHPUnit `testSvcTagDoesNotLeakBetweenRequests` against the FPM weblog: two sequential requests on the same worker prove svc.* reflects per-request state with no leak. Implements: RFC "Signal Service Name Source via Process Tags" https://docs.google.com/document/d/1c47iSTWxIOHMHfZTF2nT9xfyQaIBP9KJvI9sRn5SvpM
Addresses review feedback on PR:
- Wrap `ddog_sidecar_session_set_default_service_name` calls in
`ddtrace_ffi_try` so transport errors surface in the trace log
instead of being silently dropped.
- Use `DDOG_CHARSLICE_C("")` instead of hand-rolled CharSlice struct
literal for the user-defined case (matches the rest of sidecar.c).
- Call `ddtrace_sidecar_update_process_tags()` at the end of
`ddtrace_sidecar_handle_fork` so the child's fresh sidecar session
re-learns the svc.* source after fork; without this, child
telemetry/RC/stats payloads would drop the svc.* tag entirely
until the next external trigger.
Submodule bump picks up the companion stats-payload fix in
DataDog/libdatadog#2053.
cfaa401 to
a210c1c
Compare
…ection The sidecar now appends `svc.auto:<default_service_name>` to outgoing process_tags payloads (telemetry, remote config, live debugger) when DD_SERVICE is unset. Update the three expectation strings to include the new tag. Verified telemetry test locally; the RC and live-debugger tests use the same injection mechanism and will be exercised in CI.
| } | ||
| if (has_user_service || svc_auto_normalized) { | ||
| smart_str combined = {0}; | ||
| smart_str_appendl(&combined, ZSTR_VAL(process_tags), ZSTR_LEN(process_tags)); |
There was a problem hiding this comment.
| smart_str_appendl(&combined, ZSTR_VAL(process_tags), ZSTR_LEN(process_tags)); | |
| smart_str_append(&combined, process_tags); |
simpler
| zend_string *dd_service = get_DD_SERVICE(); | ||
| if (dd_service && ZSTR_LEN(dd_service) > 0) { |
There was a problem hiding this comment.
DD_SERVICE is dynamic per request, not per session.
Some request may have it, another not.
You probably can store ddtrace_default_service_name() on the sidecar as session bound (at which point setting that one via ddog_sidecar_session_set_config is enough), but the DD_SERVICE may or may not exist for any particular request, and change between requests. You may have to submit that one alongside with ddog_sidecar_set_universal_service_tags().
... let's talk about that?!
Addresses CI failures in dynamic_config_* and debugger_* tests. The sidecar was injecting svc.auto into the Vec<Tag> stored in the RC Target, but the tracer's RC read side (sidecar.c:808) uses bare process_tags — Target hashes diverged → SHM lookups missed → no RC configs delivered to userland. Companion libdatadog commit reverts the sidecar-side injection on the RC path. Trace per-span svc.* and telemetry svc.* are unaffected. Also addresses bwoebi review comment: simplify smart_str_appendl to smart_str_append in ext/serializer.c. RC/live-debugger payloads will need a libdatadog follow-up (Target identity vs. payload metadata split, or custom Hash/Eq) before they can carry svc.* per the RFC.
Summary
Implements the PHP-tracer side of RFC Signal Service Name Source via Process Tags.
Surfaces one of two mutually-exclusive process tags so the Datadog backend can distinguish user-set vs tracer-auto-resolved service names:
svc.user:true— whenDD_SERVICEis non-empty (env var, INI, OTEL fallback, remote config)svc.auto:<default_service_name>— whenDD_SERVICEis empty and the tracer auto-resolved the name (e.g.web.request, script basename,cli.command)Per the RFC caveats, no conclusions are drawn from the absence of both tags.
Architecture
Process tags are per-process; the active service name in PHP is per-request (mutable via
ini_set, OTEL fallbacks, remote-config-driven INI updates). The earlier approach of bakingsvc.user/svc.autointo the static process_tags string leaked the latest request's override into subsequent FPM requests served by the same worker — addressed by the senior review.Two cooperating paths now:
1. Per-span emission (traces) —
ext/serializer.cIn
ddtrace_serialize_span_to_rust_span, after attaching the static_dd.tags.processstring, the per-spansvc.user:true/svc.auto:<normalized-default>tag is appended based on the request-active state (get_DD_SERVICE()at serialization time, normalized viaddog_normalize_process_tag_value). Each span sees its own request's state — cross-request leak is impossible by construction.2. Sidecar (telemetry / remote config / runtime info) —
ext/sidecar.cddtrace_sidecar_update_process_tagsnow also calls the new libdatadog FFIddog_sidecar_session_set_default_service_name(transport, default_service_name):default_service_name→ sidecar treats it as user-defined and injectssvc.user:truedefault_service_name(pre-normalized) → sidecar injectssvc.auto:<default>The sidecar performs the injection at payload emission time (via the new
SessionInfo::process_tags_with_svc_source()helper), so all of telemetry / remote config / runtime_info / stats payloads see consistent svc.* tagging without the tracer having to bake it into the static string.The libdatadog half lives in DataDog/libdatadog#2053; the submodule pointer in this PR is bumped to that branch tip and must be moved to the merged SHA before this PR merges.
Behavior
_dd.tags.processDD_SERVICE=...env varsvc.user:truesvc.user:truedatadog.service=...system INIsvc.user:truesvc.user:trueOTEL_SERVICE_NAME=...(fallback)svc.user:truesvc.user:trueOTEL_RESOURCE_ATTRIBUTES=service.name=...(fallback)svc.user:truesvc.user:trueini_set('datadog.service', ...)(per-request)svc.user:true(this span only)svc.auto:<basename>.php(CLI) /svc.auto:web.request(web)Companion PR
DataDog/libdatadog#2053 — exposes
ddog_sidecar_session_set_default_service_nameand the sidecar-side injection logic. This PR's CI will be red on the submodule pointer until #2053 lands.