feat(process-tags): signal service name source via svc.user/svc.auto#3921
Conversation
|
Benchmarks [ tracer ]Benchmark execution time: 2026-06-25 21:22:19 Comparing candidate commit d322c40 in PR branch Found 0 performance improvements and 3 performance regressions! Performance is the same for 190 metrics, 1 unstable metrics.
|
Leiyks
force-pushed
the
leiyks/svc-source-process-tags
branch
2 times, most recently
from
cfaa401 to
a210c1c
Compare
Leiyks
force-pushed
the
leiyks/svc-source-process-tags
branch
5 times, most recently
from
f9c7288 to
3d7cb6d
Compare
| if (!zend_hash_str_exists(root_meta, ZEND_STRL("_dd.svc_src"))) { | ||
| zval root_svc; | ||
| ZVAL_UNDEF(&root_svc); | ||
| datadog_convert_to_string(&root_svc, &span->root->property_service); |
There was a problem hiding this comment.
By the way, why to a zval, datadog_convert_to_str would do it too. You don't need the zval actually.
bwoebi
left a comment
bwoebi
left a comment
There was a problem hiding this comment.
Looks correct from logic, just a few code nits
|
And there's one phpt test with wrong expectations, the code is correct. |
Leiyks
requested review from
leoromanovsky and
typotter
and removed request for
a team
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2c8a3f1e2c
ℹ️ 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".
| zend_string *default_svc = datadog_default_service_name(); | ||
| if (default_svc) { | ||
| const char *normalized = ddog_normalize_process_tag_value(dd_zend_string_to_CharSlice(default_svc)); | ||
| if (normalized) { | ||
| datadog_ffi_try("Failed updating sidecar default service name", | ||
| ddog_sidecar_session_set_default_service_name(&DATADOG_G(sidecar), | ||
| (ddog_CharSlice){ .ptr = normalized, .len = strlen(normalized) })); |
There was a problem hiding this comment.
This only sends the new default_service_name from datadog_sidecar_update_process_tags(), which is called from the one-time first-RINIT path and the explicit reload path. Fresh sidecar sessions created later by dd_sidecar_on_reconnect()/datadog_sidecar_ensure_active() run dd_sidecar_post_connect() and resend UST, but never call this setter, so after a sidecar restart or a late worker-thread connection the sidecar has no default service name and its telemetry/runtime-info process tags lose svc.auto:<default> until process tags are manually reloaded.
Useful? React with 👍 / 👎.
| if (get_global_DD_EXPERIMENTAL_PROPAGATE_PROCESS_TAGS_ENABLED()) { | ||
| datadog_sidecar_refresh_user_service_defined(); | ||
| } |
There was a problem hiding this comment.
Refreshing the user_service_defined flag after ddtrace_sidecar_submit_span_data_direct_defaults() is too late for the sidecar payloads that function can emit: it sends universal service tags and, when !root, may immediately flush buffered telemetry. In FPM/vhost or remote-config cases where the resolved DD_SERVICE source changes between requests, those first payloads for the new request are tagged using the previous request's svc.user/svc.auto state.
Useful? React with 👍 / 👎.
) ## Summary Adds `ddog_sidecar_session_set_default_service_name(transport, default_service_name)` so tracers can communicate whether the application's service name was user-set or tracer-auto-resolved. The sidecar stores this per-session and **injects `svc.user:true`** or **`svc.auto:<default>`** into outgoing payloads (telemetry, remote config, runtime info) at emission time — eliminating the need for tracers to bake svc.* into their static process_tags string (which would conflict with request-local service mutations in languages like PHP). Implements the sidecar half of the RFC ["Signal Service Name Source via Process Tags"](https://docs.google.com/document/d/1c47iSTWxIOHMHfZTF2nT9xfyQaIBP9KJvI9sRn5SvpM). ## FFI ```c ddog_MaybeError ddog_sidecar_session_set_default_service_name( struct ddog_SidecarTransport **transport, struct ddog_CharSlice default_service_name); ``` - Empty `default_service_name` → `ServiceNameSource::UserDefined` → sidecar emits `svc.user:true` - Non-empty `default_service_name` (pre-normalized via `ddog_normalize_process_tag_value`) → `ServiceNameSource::AutoResolved(name)` → sidecar emits `svc.auto:<name>` - Never called → no svc.* tag emitted (matches the RFC: *"no conclusions should be drawn from the absence of both"*) ## Internals - New `ServiceNameSource` enum in `service/mod.rs` - New `Arc<Mutex<Option<ServiceNameSource>>>` field on `SessionInfo` - New `SessionInfo::process_tags_with_svc_source()` helper — single source of truth used by all consumers of session process_tags (telemetry, RC, runtime_info, sidecar_server) - RPC method + outbox slot + sender method + blocking helper, mirroring `set_session_process_tags` ## Companion PR [DataDog/dd-trace-php#3921](DataDog/dd-trace-php#3921) — PHP tracer wires `ddog_sidecar_session_set_default_service_name` in `ext/sidecar.c` and bumps the submodule to a commit including this change. Co-authored-by: alexandre.rulleau <alexandre.rulleau@datadoghq.com>
Leiyks
force-pushed
the
leiyks/svc-source-process-tags
branch
from
a11d2ce to
7694af0
Compare
Implements the PHP-tracer side of RFC "Signal Service Name Source via Process Tags". Surfaces one of two mutually-exclusive process tags so the backend can distinguish user-set vs tracer-auto-resolved service names: - svc.user:true — DD_SERVICE non-empty (env, INI, OTEL fallback, RC) - svc.auto:<name> — DD_SERVICE empty; tracer auto-resolved the default Per the RFC caveats, no conclusions are drawn from the absence of both. In ddtrace_serialize_span_to_rust_span's is_first_span block, the auto-resolved default name is read directly from the root span's property_service when its _dd.svc_src is absent (Service Override Source Attribution RFC: cleared svc_src ↔ service is the global default), avoiding a second pass through datadog_default_service_name(). Each span sees its own request's state — no FPM cross-request leak. datadog_sidecar_update_process_tags now also calls ddog_sidecar_session_set_default_service_name(transport, …): - Empty CharSlice → sidecar injects svc.user:true - Normalized default → sidecar injects svc.auto:<default> Injection happens at payload emission time in libdatadog (companion PR DataDog/libdatadog#2053), so telemetry / remote-config / runtime-info / stats payloads all see consistent svc.* tagging without baking it into the static process_tags string. - New CLI .phpt tests covering svc.user, svc.auto, OTEL fallback as user-defined, and ini_set-driven runtime mutation. - New web-SAPI test ProcessTagsWebTest::testSvcTagDoesNotLeakBetweenRequests proving the per-span design holds across FPM workers. - Existing process_tags.phpt / telemetry_process_tags.phpt updated to expect the appended svc.auto tag.
Co-authored-by: Bob Weinand <bobwei9@hotmail.com>
…expectation - serializer.c: drop the temp zval and use datadog_convert_to_str directly (zend_string out, released by caller) per review. - process_tags.phpt: the test sets \$parent_span->service = 'test_service' manually, so the root span carries _dd.svc_src='m' (Service Override Source Attribution RFC); the per-span emission correctly skips svc.auto because the service isn't the global default. Remove the spurious svc.auto:process_tags.php from the expected output.
Leiyks
force-pushed
the
leiyks/svc-source-process-tags
branch
from
7694af0 to
d322c40
Compare
2 participants
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.