From 23a00962f9e9ecbdae4223b7b5a8ea51f7d5b5b2 Mon Sep 17 00:00:00 2001 From: Bob Weinand Date: Sat, 23 May 2026 15:55:27 +0200 Subject: [PATCH 1/2] Fix some span stats assumptions --- components-rs/stats.rs | 4 +++- ext/serializer.c | 26 +++++++++++++------------- ext/span_stats.c | 2 +- 3 files changed, 17 insertions(+), 15 deletions(-) diff --git a/components-rs/stats.rs b/components-rs/stats.rs index bdc4b181c5..e2e831c48e 100644 --- a/components-rs/stats.rs +++ b/components-rs/stats.rs @@ -517,6 +517,8 @@ pub unsafe extern "C" fn ddog_sidecar_add_php_span_to_concentrator( let env_str = char_slice_str(env).to_owned(); let version_str = char_slice_str(version).to_owned(); let owned_span = php_span_to_owned_input(span); - let _ = add_span_to_concentrator(transport, env_str, version_str, owned_span); + if let Err(e) = add_span_to_concentrator(transport, env_str, version_str, owned_span) { + trace!("Failed to send span to concentrator via IPC: {e}"); + } } diff --git a/ext/serializer.c b/ext/serializer.c index bae4bbb12d..eb2e411913 100644 --- a/ext/serializer.c +++ b/ext/serializer.c @@ -1404,12 +1404,13 @@ ddog_SpanBytes *ddtrace_serialize_span_to_rust_span(ddtrace_span_data *span, ddo } // Determine sampling before allocating the rust span to avoid unnecessary work. + bool p0_trace = ddtrace_fetch_priority_sampling_from_span(span->root) <= 0; bool span_sampling_applied = false; double span_sampling_rate = 1.0; double span_sampling_max_per_second = 0.0; bool span_sampling_has_max = false; - if (!is_inferred_span && zend_hash_num_elements(get_DD_SPAN_SAMPLING_RULES()) && ddtrace_fetch_priority_sampling_from_span(span->root) <= 0) { + if (p0_trace && !is_inferred_span && zend_hash_num_elements(get_DD_SPAN_SAMPLING_RULES())) { zval *rule; ZEND_HASH_FOREACH_VAL(get_DD_SPAN_SAMPLING_RULES(), rule) { if (Z_TYPE_P(rule) != IS_ARRAY) { @@ -1532,20 +1533,19 @@ ddog_SpanBytes *ddtrace_serialize_span_to_rust_span(ddtrace_span_data *span, ddo break; } ZEND_HASH_FOREACH_END(); + } - - if (!span_sampling_applied && DDTRACE_G(sidecar) && get_DD_TRACE_STATS_COMPUTATION_ENABLED() && ddog_agent_has_stats_computation()) { - if (inferred_span) { - // Inferred span won't be serialized, so feed it to the concentrator here. - ddtrace_span_precomputed inferred_pre; - ddtrace_precompute_span(inferred_span, &inferred_pre); - ddtrace_feed_span_to_concentrator(inferred_span, &inferred_pre); - ddtrace_free_span_precomputed(&inferred_pre); - } - ddtrace_feed_span_to_concentrator(span, &pre); - ddtrace_free_span_precomputed(&pre); - return NULL; + if (p0_trace && !span_sampling_applied && DDTRACE_G(sidecar) && get_DD_TRACE_STATS_COMPUTATION_ENABLED() && ddog_agent_has_stats_computation()) { + if (inferred_span) { + // Inferred span won't be serialized, so feed it to the concentrator here. + ddtrace_span_precomputed inferred_pre; + ddtrace_precompute_span(inferred_span, &inferred_pre); + ddtrace_feed_span_to_concentrator(inferred_span, &inferred_pre); + ddtrace_free_span_precomputed(&inferred_pre); } + ddtrace_feed_span_to_concentrator(span, &pre); + ddtrace_free_span_precomputed(&pre); + return NULL; } bool is_first_span = ddog_get_trace_size(trace) == 0; diff --git a/ext/span_stats.c b/ext/span_stats.c index e3290aff41..fffd9e150b 100644 --- a/ext/span_stats.c +++ b/ext/span_stats.c @@ -166,7 +166,7 @@ void ddtrace_precompute_span(ddtrace_span_data *span, ddtrace_span_precomputed * // Stats eligibility fields — fetched once here to avoid duplicate lookups in the two // call sites (ddtrace_span_concentrator_feed_cb and ddtrace_feed_span_to_concentrator). - pre->has_top_level = ddtrace_span_is_entrypoint_root(span); + pre->has_top_level = ddtrace_span_is_entrypoint_root(span) || (span->std.ce == ddtrace_ce_root_span_data && ROOTSPANDATA(&span->std)->parent_id == 0) || (span->parent && !zend_is_identical(&span->property_service, &span->parent->property_service)); zval *is_measured = pre->metrics ? zend_hash_str_find(pre->metrics, ZEND_STRL("_dd.measured")) : NULL; pre->is_measured = is_measured && zval_get_double(is_measured) != 0.0; pre->is_partial_snapshot = false; From 2dc3d7b978196f6ef1c66d9ac4243cb0d25ecbe8 Mon Sep 17 00:00:00 2001 From: Bob Weinand Date: Mon, 25 May 2026 17:23:01 +0200 Subject: [PATCH 2/2] Properly handle version propagation --- ext/ddtrace.c | 13 +++++++-- ext/span_stats.c | 11 ++++++-- tests/ext/ust.phpt | 27 ++++++++----------- ...n_ai.open_ai_test.test_open_ai_service.txt | 2 +- ...n_ai.open_ai_test.test_open_ai_service.txt | 10 +++---- ..._ai.open_ai_test.test_open_ai_service.json | 6 +++-- 6 files changed, 41 insertions(+), 28 deletions(-) diff --git a/ext/ddtrace.c b/ext/ddtrace.c index 625c9cfa03..109e407104 100644 --- a/ext/ddtrace.c +++ b/ext/ddtrace.c @@ -1136,10 +1136,19 @@ static zval *ddtrace_span_data_readonly(zend_object *object, zend_string *member ddtrace_span_data *span = OBJ_SPANDATA(obj); // As per unified service tagging spec if a span is created with a service name different from the global - // service name it will not inherit the global version value + // service name it will not inherit the global version value, unless it has no ancestor traces. if (zend_string_equals_literal(prop_name, "service")) { cache_slot = NULL; - if (ZSTR_LEN(get_DD_SERVICE()) || !ddtrace_span_is_entrypoint_root(span)) { + bool is_top_level_root = span->std.ce == ddtrace_ce_root_span_data; + if (is_top_level_root) { + for (ddtrace_span_stack *s = span->stack->parent_stack; s != NULL; s = s->parent_stack) { + if (s->active) { + is_top_level_root = false; + break; + } + } + } + if (!is_top_level_root) { if (!zend_is_identical(&span->property_service, value)) { zval_ptr_dtor(&span->property_version); ZVAL_EMPTY_STRING(&span->property_version); diff --git a/ext/span_stats.c b/ext/span_stats.c index fffd9e150b..aab1be2ab8 100644 --- a/ext/span_stats.c +++ b/ext/span_stats.c @@ -388,8 +388,15 @@ static void ddtrace_span_concentrator_feed_cb(const ddog_SpanConcentrator *c, vo } void ddtrace_feed_span_to_concentrator(ddtrace_span_data *span, const ddtrace_span_precomputed *pre) { - ddog_CharSlice env_slice = dd_zend_string_to_CharSlice(pre->env); - ddog_CharSlice version_slice = dd_zend_string_to_CharSlice(pre->version); + ddog_CharSlice env_slice = dd_zend_string_to_CharSlice(pre->env); + // versions can change per service, always look at root span + zend_string *version_zstr = NULL; + zval *root_version_zv = &span->root->property_version; + ZVAL_DEREF(root_version_zv); + if (Z_TYPE_P(root_version_zv) == IS_STRING && ZSTR_LEN(Z_STR_P(root_version_zv)) > 0) { + version_zstr = Z_STR_P(root_version_zv); + } + ddog_CharSlice version_slice = dd_zend_string_to_CharSlice(version_zstr); // Use the process-level DD_SERVICE as the concentrator key so all spans from this PHP // process share one SHM concentrator regardless of per-request service overrides. ddog_CharSlice service_slice = dd_zend_string_to_CharSlice(get_global_DD_SERVICE()); diff --git a/tests/ext/ust.phpt b/tests/ext/ust.phpt index 35687d757e..50bd80dea3 100644 --- a/tests/ext/ust.phpt +++ b/tests/ext/ust.phpt @@ -1,5 +1,5 @@ --TEST-- -Foo +Unified service tagging removes version from spans --ENV-- DD_SERVICE=version_test DD_VERSION=5.2.0 @@ -13,13 +13,14 @@ include __DIR__ . '/sandbox/dd_dumper.inc'; $s1 = \DDTrace\start_trace_span(); $s1->name = "s1"; -\DDTrace\close_span(); $s2 = \DDTrace\start_trace_span(); $s2->name = "s2"; $s2->service = "no dd_service"; \DDTrace\close_span(); +\DDTrace\close_span(); + var_dump(dd_clean_spans()); ?> @@ -36,11 +37,11 @@ array(2) { ["duration"]=> int(%d) ["name"]=> - string(2) "s2" + string(2) "s1" ["resource"]=> - string(2) "s2" + string(2) "s1" ["service"]=> - string(13) "no dd_service" + string(12) "version_test" ["type"]=> string(3) "cli" ["meta"]=> @@ -53,6 +54,8 @@ array(2) { string(8) "env_test" ["runtime-id"]=> string(36) "%s" + ["version"]=> + string(5) "5.2.0" } ["metrics"]=> array(%d) { @@ -81,11 +84,11 @@ array(2) { ["duration"]=> int(%d) ["name"]=> - string(2) "s1" + string(2) "s2" ["resource"]=> - string(2) "s1" + string(2) "s2" ["service"]=> - string(12) "version_test" + string(13) "no dd_service" ["type"]=> string(3) "cli" ["meta"]=> @@ -98,8 +101,6 @@ array(2) { string(8) "env_test" ["runtime-id"]=> string(36) "%s" - ["version"]=> - string(5) "5.2.0" } ["metrics"]=> array(%d) { @@ -107,12 +108,6 @@ array(2) { float(1) ["_sampling_priority_v1"]=> float(1) - ["php.compilation.total_time_ms"]=> - float(%f) - ["php.memory.peak_real_usage_bytes"]=> - float(%f) - ["php.memory.peak_usage_bytes"]=> - float(%f) ["process_id"]=> float(%f) } diff --git a/tests/snapshots/logs/tests.integrations.open_ai.open_ai_test.test_open_ai_service.txt b/tests/snapshots/logs/tests.integrations.open_ai.open_ai_test.test_open_ai_service.txt index a8caede28c..56480beda1 100644 --- a/tests/snapshots/logs/tests.integrations.open_ai.open_ai_test.test_open_ai_service.txt +++ b/tests/snapshots/logs/tests.integrations.open_ai.open_ai_test.test_open_ai_service.txt @@ -1 +1 @@ -{"message":"sampled listModels","status":"info","timestamp":"2024-07-18T07:58:56.301000+00:00","env":"test","service":"openai","openai.request.method":"GET","openai.request.endpoint":"\/v1\/models","openai.organization.name":"org-1234","openai.user.api_key":"sk-...9d5d","dd.trace_id":"7579939180383610820","dd.span_id":"7579939180383610820"} +{"message":"sampled listModels","status":"info","timestamp":"2024-07-18T07:58:56.301000+00:00","env":"test","service":"openai","openai.request.method":"GET","openai.request.endpoint":"\/v1\/models","openai.organization.name":"org-1234","openai.user.api_key":"sk-...9d5d","dd.trace_id":"7579939180383610820","dd.span_id":"7579939180383610820","version":"1.0"} diff --git a/tests/snapshots/metrics/tests.integrations.open_ai.open_ai_test.test_open_ai_service.txt b/tests/snapshots/metrics/tests.integrations.open_ai.open_ai_test.test_open_ai_service.txt index ea00470453..8fcf7cd6e4 100644 --- a/tests/snapshots/metrics/tests.integrations.open_ai.open_ai_test.test_open_ai_service.txt +++ b/tests/snapshots/metrics/tests.integrations.open_ai.open_ai_test.test_open_ai_service.txt @@ -1,5 +1,5 @@ -openai.request.duration:205208|d|#env:test,service:openai,env:test,service:openai,openai.organization.name:org-1234,openai.user.api_key:sk-...9d5d,openai.request.endpoint:/v1/models -openai.ratelimit.requests:3000|g|#env:test,service:openai,env:test,service:openai,openai.organization.name:org-1234,openai.user.api_key:sk-...9d5d,openai.request.endpoint:/v1/models -openai.ratelimit.tokens:250000|g|#env:test,service:openai,env:test,service:openai,openai.organization.name:org-1234,openai.user.api_key:sk-...9d5d,openai.request.endpoint:/v1/models -openai.ratelimit.remaining.requests:2999|g|#env:test,service:openai,env:test,service:openai,openai.organization.name:org-1234,openai.user.api_key:sk-...9d5d,openai.request.endpoint:/v1/models -openai.ratelimit.remaining.tokens:249989|g|#env:test,service:openai,env:test,service:openai,openai.organization.name:org-1234,openai.user.api_key:sk-...9d5d,openai.request.endpoint:/v1/models +openai.request.duration:205208|d|#env:test,service:openai,env:test,service:openai,openai.organization.name:org-1234,openai.user.api_key:sk-...9d5d,openai.request.endpoint:/v1/models,version:1.0 +openai.ratelimit.requests:3000|g|#env:test,service:openai,env:test,service:openai,openai.organization.name:org-1234,openai.user.api_key:sk-...9d5d,openai.request.endpoint:/v1/models,version:1.0 +openai.ratelimit.tokens:250000|g|#env:test,service:openai,env:test,service:openai,openai.organization.name:org-1234,openai.user.api_key:sk-...9d5d,openai.request.endpoint:/v1/models,version:1.0 +openai.ratelimit.remaining.requests:2999|g|#env:test,service:openai,env:test,service:openai,openai.organization.name:org-1234,openai.user.api_key:sk-...9d5d,openai.request.endpoint:/v1/models,version:1.0 +openai.ratelimit.remaining.tokens:249989|g|#env:test,service:openai,env:test,service:openai,openai.organization.name:org-1234,openai.user.api_key:sk-...9d5d,openai.request.endpoint:/v1/models,version:1.0 diff --git a/tests/snapshots/tests.integrations.open_ai.open_ai_test.test_open_ai_service.json b/tests/snapshots/tests.integrations.open_ai.open_ai_test.test_open_ai_service.json index 21013bf057..4c52b844cd 100644 --- a/tests/snapshots/tests.integrations.open_ai.open_ai_test.test_open_ai_service.json +++ b/tests/snapshots/tests.integrations.open_ai.open_ai_test.test_open_ai_service.json @@ -19,7 +19,8 @@ "openai.response.object": "list", "openai.user.api_key": "sk-...9d5d", "runtime-id": "87cccdb5-2d7c-453e-bc51-70997f55724c", - "span.kind": "client" + "span.kind": "client", + "version": "1.0" }, "metrics": { "_dd.agent_psr": 1, @@ -44,6 +45,7 @@ "http.status_code": "200", "http.url": "https://api.openai.com/v1/models?foo=bar", "network.destination.name": "api.openai.com", - "span.kind": "client" + "span.kind": "client", + "version": "1.0" } }]]