Fix assumptions about what a top_level span is#3916
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 531586746b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| 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); |
There was a problem hiding this comment.
Use resolved version when keying concentrator writes
ddtrace_feed_span_to_concentrator now ignores pre->version and reads only span->root->property_version when it is already a string, which drops versions that were resolved via deprecated meta["version"] (still supported in ddtrace_precompute_span) or via non-string values converted during precompute. In those cases spans are emitted with a version tag but stats are keyed with an empty version, causing version-level stats to be mis-grouped or split from trace data.
Useful? React with 👍 / 👎.
|
Benchmarks [ tracer ]Benchmark execution time: 2026-05-25 16:40:49 Comparing candidate commit 5315867 in PR branch Found 0 performance improvements and 14 performance regressions! Performance is the same for 178 metrics, 2 unstable metrics. scenario:PDOBench/benchPDOOverhead
scenario:PDOBench/benchPDOOverhead-opcache
scenario:PDOBench/benchPDOOverheadWithDBM
scenario:PDOBench/benchPDOOverheadWithDBM-opcache
scenario:PHPRedisBench/benchRedisOverhead
scenario:PHPRedisBench/benchRedisOverhead-opcache
scenario:SpanBench/benchDatadogAPI
scenario:SpanBench/benchDatadogAPI-opcache
scenario:SpanBench/benchOpenTelemetryInteroperability
scenario:SpanBench/benchOpenTelemetryInteroperability-opcache
scenario:TraceFlushBench/benchFlushTrace
scenario:TraceFlushBench/benchFlushTrace-opcache
scenario:TraceSerializationBench/benchSerializeTrace
scenario:TraceSerializationBench/benchSerializeTrace-opcache
|
f69edb3 to
9883729
Compare
9883729 to
2dc3d7b
Compare
Otherwise span stats will be broken for nested services.
Also properly handling version propagation, its removal according to UST was too aggressive until now.