feat(process-tags): emit svc.user/svc.auto per-span and via sidecar

Leiyks · Leiyks · commit 558b1fb5bb03 · 2026-05-28T17:02:39.000+02:00
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
diff --git a/components-rs/sidecar.h b/components-rs/sidecar.h
@@ -225,6 +225,17 @@ ddog_MaybeError ddog_sidecar_session_set_config(struct ddog_SidecarTransport **t
 ddog_MaybeError ddog_sidecar_session_set_process_tags(struct ddog_SidecarTransport **transport,
                                                       const struct ddog_Vec_Tag *process_tags);
 
+/**
+ * Records the source of the default service name for the session so the
+ * sidecar can inject svc.user:true or svc.auto:<default_service_name> into
+ * outgoing process tag payloads. Pass an empty `default_service_name` to
+ * signal the user explicitly set DD_SERVICE; pass a non-empty value (already
+ * normalized via `ddog_normalize_process_tag_value`) to signal the tracer
+ * auto-resolved that name.
+ */
+ddog_MaybeError ddog_sidecar_session_set_default_service_name(struct ddog_SidecarTransport **transport,
+                                                              ddog_CharSlice default_service_name);
+
 /**
  * Enqueues a telemetry log action to be processed internally.
  * Non-blocking. Logs might be dropped if the internal queue is full.
diff --git a/ext/ddtrace.c b/ext/ddtrace.c
@@ -416,10 +416,6 @@ bool ddtrace_alter_dd_service(zval *old_value, zval *new_value, zend_string *new
     dd_alter_prop(XtOffsetOf(ddtrace_span_properties, property_service), old_value, new_value, new_str);
     if (DDTRACE_G(request_initialized)) {
         ddtrace_sidecar_submit_root_span_data_direct(&DDTRACE_G(sidecar), NULL, new_str, get_DD_ENV(), get_DD_VERSION());
-        if (ddtrace_process_tags_enabled()) {
-            ddtrace_process_tags_reload_with_service(new_str);
-            ddtrace_sidecar_update_process_tags();
-        }
     }
     return true;
 }
diff --git a/ext/process_tags.c b/ext/process_tags.c
@@ -11,7 +11,6 @@
 #include <string.h>
 #include "process_tags.h"
 #include "configuration.h"
-#include "serializer.h"
 #include "Zend/zend_smart_str.h"
 #include "components-rs/ddtrace.h"
 #include "SAPI.h"
@@ -25,8 +24,6 @@
 #define TAG_ENTRYPOINT_WORKDIR "entrypoint.workdir"
 #define TAG_ENTRYPOINT_TYPE "entrypoint.type"
 #define TAG_RUNTIME_SAPI "runtime.sapi"
-#define TAG_SVC_USER "svc.user"
-#define TAG_SVC_AUTO "svc.auto"
 
 #define TYPE_SCRIPT "script"
 #define TYPE_EXECUTABLE "executable"
@@ -47,7 +44,6 @@ typedef struct {
 } process_tags_t;
 
 static process_tags_t process_tags = {0};
-static zend_string *pending_service_override = NULL;
 
 static void clear_process_tags(void) {
     for (size_t i = 0; i < process_tags.count; i++) {
@@ -172,17 +168,6 @@ static void collect_process_tags(void) {
 
     add_process_tag(TAG_RUNTIME_SAPI, sapi_module.name);
 
-    zend_string *dd_service = pending_service_override ? pending_service_override : get_DD_SERVICE();
-    if (dd_service && ZSTR_LEN(dd_service) > 0) {
-        add_process_tag(TAG_SVC_USER, "true");
-    } else {
-        zend_string *default_svc = ddtrace_default_service_name();
-        if (default_svc) {
-            add_process_tag(TAG_SVC_AUTO, ZSTR_VAL(default_svc));
-            zend_string_release(default_svc);
-        }
-    }
-
     const char *script = NULL;
     if (SG(request_info).path_translated && *SG(request_info).path_translated) {
         script = SG(request_info).path_translated;
@@ -351,13 +336,6 @@ void ddtrace_process_tags_reload(void) {
     clear_process_tags();
     init_process_tags();
 }
-
-void ddtrace_process_tags_reload_with_service(zend_string *service_override) {
-    pending_service_override = service_override;
-    clear_process_tags();
-    init_process_tags();
-    pending_service_override = NULL;
-}
 void ddtrace_process_tags_mshutdown(void) {
     clear_process_tags();
 }
diff --git a/ext/process_tags.h b/ext/process_tags.h
@@ -11,11 +11,6 @@
 void ddtrace_process_tags_first_rinit(void);
 // Reload process tags in current request
 void ddtrace_process_tags_reload(void);
-// Reload process tags using the supplied service-name override. Use when the
-// DD_SERVICE INI is being changed but the new value has not yet been committed
-// to the ZAI config runtime (e.g. inside an `ini_change` callback).
-// Pass NULL to fall back to get_DD_SERVICE().
-void ddtrace_process_tags_reload_with_service(zend_string *service_override);
 
 // Called at MSHUTDOWN to free resources
 void ddtrace_process_tags_mshutdown(void);
diff --git a/ext/serializer.c b/ext/serializer.c
@@ -1577,7 +1577,44 @@ ddog_SpanBytes *ddtrace_serialize_span_to_rust_span(ddtrace_span_data *span, ddo
     if (is_first_span) {
         zend_string *process_tags = ddtrace_process_tags_get_serialized();
         if (ZSTR_LEN(process_tags)) {
-            ddog_add_str_span_meta_zstr(rust_span, "_dd.tags.process", process_tags);
+            // Per-RFC "Signal Service Name Source via Process Tags", append a
+            // svc.user:true or svc.auto:<default_service_name> tag computed
+            // from the request-active state. Static process_tags are
+            // process-global; service name is request-local in PHP, so the
+            // svc.* signal must be derived per span.
+            zend_string *dd_service = get_DD_SERVICE();
+            bool has_user_service = dd_service && ZSTR_LEN(dd_service) > 0;
+            zend_string *default_svc = NULL;
+            const char *svc_auto_normalized = NULL;
+            if (!has_user_service) {
+                default_svc = ddtrace_default_service_name();
+                if (default_svc) {
+                    svc_auto_normalized = ddog_normalize_process_tag_value((ddog_CharSlice){
+                        .ptr = ZSTR_VAL(default_svc), .len = ZSTR_LEN(default_svc)
+                    });
+                }
+            }
+            if (has_user_service || svc_auto_normalized) {
+                smart_str combined = {0};
+                smart_str_appendl(&combined, ZSTR_VAL(process_tags), ZSTR_LEN(process_tags));
+                if (has_user_service) {
+                    smart_str_appends(&combined, ",svc.user:true");
+                } else {
+                    smart_str_appends(&combined, ",svc.auto:");
+                    smart_str_appends(&combined, svc_auto_normalized);
+                }
+                smart_str_0(&combined);
+                ddog_add_str_span_meta_zstr(rust_span, "_dd.tags.process", combined.s);
+                smart_str_free(&combined);
+            } else {
+                ddog_add_str_span_meta_zstr(rust_span, "_dd.tags.process", process_tags);
+            }
+            if (svc_auto_normalized) {
+                ddog_free_normalized_tag_value(svc_auto_normalized);
+            }
+            if (default_svc) {
+                zend_string_release(default_svc);
+            }
         }
     }
 
diff --git a/ext/sidecar.c b/ext/sidecar.c
@@ -7,6 +7,7 @@
 #include "ddtrace_export.h"
 #include "dogstatsd.h"
 #include "logging.h"
+#include "serializer.h"
 #include <components-rs/common.h>
 #include <components-rs/ddtrace.h>
 #include <components-rs/sidecar.h>
@@ -149,6 +150,25 @@ void ddtrace_sidecar_update_process_tags(void) {
     }
 
     ddog_sidecar_session_set_process_tags(&DDTRACE_G(sidecar), process_tags);
+
+    zend_string *dd_service = get_DD_SERVICE();
+    if (dd_service && ZSTR_LEN(dd_service) > 0) {
+        ddog_sidecar_session_set_default_service_name(&DDTRACE_G(sidecar),
+            (ddog_CharSlice){ .ptr = "", .len = 0 });
+    } else {
+        zend_string *default_svc = ddtrace_default_service_name();
+        if (default_svc) {
+            const char *normalized = ddog_normalize_process_tag_value((ddog_CharSlice){
+                .ptr = ZSTR_VAL(default_svc), .len = ZSTR_LEN(default_svc)
+            });
+            if (normalized) {
+                ddog_sidecar_session_set_default_service_name(&DDTRACE_G(sidecar),
+                    (ddog_CharSlice){ .ptr = normalized, .len = strlen(normalized) });
+                ddog_free_normalized_tag_value(normalized);
+            }
+            zend_string_release(default_svc);
+        }
+    }
 }
 
 static ddog_SidecarTransport *dd_sidecar_connection_factory_ex(bool is_fork);
diff --git a/libdatadog b/libdatadog
@@ -1 +1 @@
-Subproject commit cea1e44edddd9124f75d5095f31026904a1f58d8
+Subproject commit 5d644c41ada3e9842e3ba77aff640175255a8d55
diff --git a/tests/Frameworks/Custom/Version_Autoloaded/src/SetServiceController.php b/tests/Frameworks/Custom/Version_Autoloaded/src/SetServiceController.php
@@ -0,0 +1,13 @@
+<?php
+
+namespace App;
+
+class SetServiceController
+{
+    public function render()
+    {
+        ini_set('datadog.service', 'request-svc');
+        header('Content-type: text/plain; charset=utf-8');
+        echo 'service set';
+    }
+}
diff --git a/tests/Frameworks/Custom/Version_Autoloaded/src/routes.php b/tests/Frameworks/Custom/Version_Autoloaded/src/routes.php
@@ -2,6 +2,7 @@
 
 return function(FastRoute\RouteCollector $r) {
     $r->addRoute('GET', '/simple', '\App\SimpleController');
+    $r->addRoute('GET', '/set_service', '\App\SetServiceController');
     $r->addRoute('GET', '/simple_view', '\App\SimpleViewController');
     $r->addRoute('GET', '/error', '\App\ErrorController');
     $r->addRoute('GET', '/telemetry', '\App\TelemetryFlushController');
diff --git a/tests/Integrations/Custom/Autoloaded/ProcessTagsWebTest.php b/tests/Integrations/Custom/Autoloaded/ProcessTagsWebTest.php
@@ -65,4 +65,28 @@ public function testProcessTagsEnabledForWebSapi()
         );
         $this->assertEquals($tags['entrypoint.type'], 'script');
     }
+
+    /**
+     * Proves the per-span svc.* design has no static-state leak between
+     * requests served by the same FPM worker: request 1 calls
+     * ini_set('datadog.service', ...) and must report svc.user:true; request 2
+     * reuses the same worker but does not override the service, and must
+     * report svc.auto:<default> (not the leaked svc.user from request 1).
+     */
+    public function testSvcTagDoesNotLeakBetweenRequests()
+    {
+        $tracesUser = $this->tracesFromWebRequest(function () {
+            return $this->call(new RequestSpec(__FUNCTION__ . '_user', 'GET', '/set_service', []));
+        });
+        $userTags = $tracesUser[0][0]['meta']['_dd.tags.process'];
+        $this->assertStringContainsString('svc.user:true', $userTags);
+        $this->assertStringNotContainsString('svc.auto:', $userTags);
+
+        $tracesAuto = $this->tracesFromWebRequest(function () {
+            return $this->call(new RequestSpec(__FUNCTION__ . '_auto', 'GET', '/simple', []));
+        });
+        $autoTags = $tracesAuto[0][0]['meta']['_dd.tags.process'];
+        $this->assertStringNotContainsString('svc.user', $autoTags);
+        $this->assertStringContainsString('svc.auto:', $autoTags);
+    }
 }
diff --git a/tests/ext/svc_runtime_change.phpt b/tests/ext/svc_runtime_change.phpt
@@ -1,33 +1,37 @@
 --TEST--
-Changing datadog.service at runtime recomputes svc.user/svc.auto process tags
+Changing datadog.service at runtime recomputes svc.user/svc.auto process tags per-span
 --ENV--
 DD_EXPERIMENTAL_PROPAGATE_PROCESS_TAGS_ENABLED=1
 DD_TRACE_GENERATE_ROOT_SPAN=0
 DD_TRACE_AUTO_FLUSH_ENABLED=0
 --FILE--
 <?php
+function assert_tags($label) {
+    $spans = dd_trace_serialize_closed_spans();
+    $tags = $spans[0]['meta']['_dd.tags.process'];
+    $hasUser = strpos($tags, 'svc.user:true') !== false;
+    $hasAuto = strpos($tags, 'svc.auto:') !== false;
+    echo "$label svc.user=" . ($hasUser ? 'YES' : 'NO') . " svc.auto=" . ($hasAuto ? 'YES' : 'NO') . "\n";
+}
+
 $span1 = \DDTrace\start_span();
 $span1->name = 'before';
 \DDTrace\close_span();
-
-$spans = dd_trace_serialize_closed_spans();
-$tagsBefore = $spans[0]['meta']['_dd.tags.process'];
-echo "BEFORE has svc.auto: " . (strpos($tagsBefore, 'svc.auto:') !== false ? 'YES' : 'NO') . "\n";
-echo "BEFORE has svc.user: " . (strpos($tagsBefore, 'svc.user') !== false ? 'YES' : 'NO') . "\n";
+assert_tags('BEFORE  ');
 
 ini_set('datadog.service', 'changed-svc');
-
 $span2 = \DDTrace\start_span();
-$span2->name = 'after';
+$span2->name = 'after_set';
 \DDTrace\close_span();
+assert_tags('AFTER   ');
 
-$spans = dd_trace_serialize_closed_spans();
-$tagsAfter = $spans[0]['meta']['_dd.tags.process'];
-echo "AFTER has svc.user:true: " . (strpos($tagsAfter, 'svc.user:true') !== false ? 'YES' : 'NO') . "\n";
-echo "AFTER has svc.auto:    : " . (strpos($tagsAfter, 'svc.auto:') !== false ? 'YES' : 'NO') . "\n";
+ini_restore('datadog.service');
+$span3 = \DDTrace\start_span();
+$span3->name = 'after_restore';
+\DDTrace\close_span();
+assert_tags('REVERTED');
 ?>
 --EXPECT--
-BEFORE has svc.auto: YES
-BEFORE has svc.user: NO
-AFTER has svc.user:true: YES
-AFTER has svc.auto:    : NO
+BEFORE   svc.user=NO svc.auto=YES
+AFTER    svc.user=YES svc.auto=NO
+REVERTED svc.user=NO svc.auto=YES
diff --git a/tests/ext/telemetry/telemetry_process_tags.phpt b/tests/ext/telemetry/telemetry_process_tags.phpt
@@ -46,7 +46,7 @@ if ($i == 300) {
 ?>
 --EXPECTF--
 Included
-string(%d) "entrypoint.basedir:telemetry,entrypoint.name:telemetry_process_tags,entrypoint.type:script,entrypoint.workdir:%s,runtime.sapi:cli,svc.auto:telemetry_process_tags.php"
+string(%d) "entrypoint.basedir:telemetry,entrypoint.name:telemetry_process_tags,entrypoint.type:script,entrypoint.workdir:%s,runtime.sapi:cli"
 --CLEAN--
 <?php
 

Original file line number	Diff line number	Diff line change
`@@ -416,10 +416,6 @@ bool ddtrace_alter_dd_service(zval old_value, zval new_value, zend_string *new`
`416`	`416`	`dd_alter_prop(XtOffsetOf(ddtrace_span_properties, property_service), old_value, new_value, new_str);`
`417`	`417`	`if (DDTRACE_G(request_initialized)) {`
`418`	`418`	`ddtrace_sidecar_submit_root_span_data_direct(&DDTRACE_G(sidecar), NULL, new_str, get_DD_ENV(), get_DD_VERSION());`
`419`		`- if (ddtrace_process_tags_enabled()) {`
`420`		`- ddtrace_process_tags_reload_with_service(new_str);`
`421`		`- ddtrace_sidecar_update_process_tags();`
`422`		`- }`
`423`	`419`	`}`
`424`	`420`	`return true;`
`425`	`421`	`}`