dd-trace-php
dd-trace-php copied to clipboard
w3c phase 2: add last parent_id to tracestate
Description
With this change span_id will be encoded in both the tracestate and traceparent headers. This will allow us to re-connect datadog traces that contain non-datadog spans.
Ex: Host A: Datadog Spans ->tracecontext-> Host B: Otel Dynatrace Spans -->tracecontext-> Host C: Otel Datadog Spans
- By adding span_id to the tracestate Host C will be able to detect upstream Datadog spans from Host A. This span_id will be sent to Datadog intake services via
_dd.parent_id
tag. The Datadog internal services will use this tag to reconnect visualize spans.
Tests will be added in this PR: https://github.com/DataDog/system-tests/pull/2181
Reviewer checklist
- [ ] Test coverage seems ok.
- [ ] Appropriate labels assigned.
Codecov Report
Merging #2549 (5799170) into master (91227e5) will decrease coverage by
23.59%
. The diff coverage is100.00%
.
Additional details and impacted files
@@ Coverage Diff @@
## master #2549 +/- ##
=============================================
- Coverage 75.77% 52.18% -23.59%
Complexity 2563 2563
=============================================
Files 241 241
Lines 27024 27043 +19
Branches 976 976
=============================================
- Hits 20478 14113 -6365
- Misses 6026 12410 +6384
Partials 520 520
Flag | Coverage Δ | |
---|---|---|
appsec-extension | 69.13% <ø> (ø) |
|
tracer-extension | 78.73% <100.00%> (+0.03%) |
:arrow_up: |
tracer-php | 12.71% <ø> (-62.01%) |
:arrow_down: |
Flags with carried forward coverage won't be shown. Click here to find out more.
Files | Coverage Δ | |
---|---|---|
ext/ddtrace.c | 73.13% <100.00%> (ø) |
|
ext/distributed_tracing_headers.c | 84.46% <100.00%> (+0.51%) |
:arrow_up: |
ext/handlers_http.h | 94.40% <100.00%> (+0.37%) |
:arrow_up: |
... and 61 files with indirect coverage changes
Continue to review full report in Codecov by Sentry.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update 91227e5...5799170. Read the comment docs.
Benchmarks
Benchmark execution time: 2024-03-19 12:14:47
Comparing candidate commit 8fcf55131e533bd0feaebdd810af1edaa9964df3 in PR branch munir/add-p-to-php
with baseline commit 91227e576ddc17ad4e7434c288c9d8d85d063688 in branch master
.
Found 3 performance improvements and 9 performance regressions! Performance is the same for 169 metrics, 1 unstable metrics.
scenario:ContextPropagationBench/benchExtractTraceContext128Bit
- 🟥
execution_time
[+75.995ns; +124.005ns] or [+3.447%; +5.624%]
scenario:ContextPropagationBench/benchExtractTraceContext128Bit-opcache
- 🟥
execution_time
[+45.059ns; +130.941ns] or [+2.050%; +5.957%]
scenario:LaravelBench/benchLaravelBaseline-opcache
- 🟥
execution_time
[+76.841µs; +228.919µs] or [+3.067%; +9.137%]
scenario:PDOBench/benchPDOBaseline-opcache
- 🟩
execution_time
[-14.636µs; -13.410µs] or [-7.786%; -7.134%]
scenario:PDOBench/benchPDOOverhead-opcache
- 🟩
execution_time
[-16.453µs; -14.379µs] or [-5.593%; -4.888%]
scenario:PDOBench/benchPDOOverheadWithDBM-opcache
- 🟩
execution_time
[-16.476µs; -14.336µs] or [-5.063%; -4.405%]
scenario:SamplingRuleMatchingBench/benchRegexMatching2
- 🟥
execution_time
[+164.649ns; +431.151ns] or [+2.706%; +7.087%]
scenario:SamplingRuleMatchingBench/benchRegexMatching2-opcache
- 🟥
execution_time
[+122.734ns; +338.866ns] or [+2.031%; +5.608%]
scenario:SamplingRuleMatchingBench/benchRegexMatching3
- 🟥
execution_time
[+150.797ns; +420.003ns] or [+2.483%; +6.916%]
scenario:SamplingRuleMatchingBench/benchRegexMatching4
- 🟥
execution_time
[+189.401ns; +402.599ns] or [+3.094%; +6.577%]
scenario:SpanBench/benchOpenTelemetryAPI
- 🟥
mem_peak
[+795.710KB; +795.716KB] or [+2.062%; +2.062%]
scenario:SpanBench/benchOpenTelemetryAPI-opcache
- 🟥
mem_peak
[+793.138KB; +793.147KB] or [+2.288%; +2.288%]
We will need to ignore/filter out the p
value in snapshot tests
@mabdinur Yep, needs a change in the test-agent to filter them there.
@bwoebi thanks for fixing the failing tests and updating the OpenTelemetry SpanContext. A few parametric tests are failing due to the changes we made to tracestate. Once those failures are addressed we can merge this PR 🥳