feat(sidecar): expose default_service_name for svc.* process tags#2053
feat(sidecar): expose default_service_name for svc.* process tags#2053Leiyks wants to merge 5 commits into
Conversation
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
Clippy Allow Annotation ReportComparing clippy allow annotations between branches:
Summary by Rule
Annotation Counts by File
Annotation Stats by Crate
About This ReportThis report tracks Clippy allow annotations for specific rules, showing how they've changed in this PR. Decreasing the number of these annotations generally improves code quality. |
📚 Documentation Check Results📦
|
🔒 Cargo Deny Results📦
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2053 +/- ##
==========================================
- Coverage 72.92% 72.85% -0.07%
==========================================
Files 460 460
Lines 76463 76537 +74
==========================================
+ Hits 55758 55760 +2
- Misses 20705 20777 +72
🚀 New features to boost your workflow:
|
|
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.
5b35326 to
975e3af
Compare
Artifact Size Benchmark Reportaarch64-alpine-linux-musl
aarch64-unknown-linux-gnu
libdatadog-x64-windows
libdatadog-x86-windows
x86_64-alpine-linux-musl
x86_64-unknown-linux-gnu
|
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.
0ebc259 to
91436f4
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. ## Per-span emission (traces) — tracer/serializer.c 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. ## Sidecar (telemetry / remote config / runtime info) — ext/sidecar.c 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. ## Tests - 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.
Adds `ddog_sidecar_session_set_default_service_name` so tracers can signal whether DD_SERVICE was user-set or auto-resolved (and the resolved name). The sidecar stores this per-session and injects `svc.user:true` or `svc.auto:<default>` into outgoing process-tags payloads (telemetry, remote config, runtime info), per RFC "Signal Service Name Source via Process Tags": https://docs.google.com/document/d/1c47iSTWxIOHMHfZTF2nT9xfyQaIBP9KJvI9sRn5SvpM The companion change in dd-trace-php (PR #3921) wires the new FFI and emits the per-span counterpart on traces directly.
Stats payloads were the one consumer not routed through `process_tags_with_svc_source()` because StatsConfig holds a pre-joined `String`. Two changes: - Build StatsConfig.process_tags from `session.process_tags_with_svc_source()` at construction time so concentrators created after the source is set carry svc.*. - Refresh StatsConfig.process_tags from both `set_session_process_tags` and `set_session_default_service_name` so live updates propagate. Addresses review feedback on initial PR.
…tent The `Target` struct in datadog-remote-config derives Hash over all fields including process_tags. Both the sidecar (RC write side) and the tracer (RC read side via ddog_remote_configs_service_env_change) must agree on the Target hash, and the tracer passes the bare process_tags Vec. Augmenting only the sidecar side with svc.* via `process_tags_with_svc_source()` made the Target hashes diverge, orphaning RC configs in SHM (probes never installed, dynamic config never applied). For now revert this single call site to bare process_tags. A proper follow-up should either exclude process_tags from Target's Hash/Eq impl, or separate Target's identity fields from the payload metadata fields. Trace + telemetry svc.* injection is unaffected.
32e3de1 to
ba0146f
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.
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.
…defined flag Addresses review feedback: DD_SERVICE is request-mutable, not session-bound. The previous single FFI conflated the two facts (empty CharSlice meaning "user-defined", non-empty meaning the auto-resolved name), which forced the tracer to re-push state to the sidecar every time DD_SERVICE changed. Split into two independent session fields: - auto_resolved_service_name: Option<String> — process-stable, set once via ddog_sidecar_session_set_default_service_name(name). - user_service_defined: bool — per-request mutable, refreshed via the new ddog_sidecar_session_set_user_service_defined(is_defined) on each RINIT. process_tags_with_svc_source() now emits svc.user:true when user_service_defined is true, else svc.auto:<name> when an auto-resolved name is stored. The old ServiceNameSource enum is dropped.
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.
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:trueorsvc.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".
FFI
default_service_name→ServiceNameSource::UserDefined→ sidecar emitssvc.user:truedefault_service_name(pre-normalized viaddog_normalize_process_tag_value) →ServiceNameSource::AutoResolved(name)→ sidecar emitssvc.auto:<name>Internals
ServiceNameSourceenum inservice/mod.rsArc<Mutex<Option<ServiceNameSource>>>field onSessionInfoSessionInfo::process_tags_with_svc_source()helper — single source of truth used by all consumers of session process_tags (telemetry, RC, runtime_info, sidecar_server)set_session_process_tagsCompanion PR
DataDog/dd-trace-php#3921 — PHP tracer wires
ddog_sidecar_session_set_default_service_nameinext/sidecar.cand bumps the submodule to a commit including this change.