w3c phase 2: add last parent_id to tracestate by mabdinur · Pull Request #2549 · DataDog/dd-trace-php

mabdinur · 2024-02-29T22:39:20Z

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: DataDog/system-tests#2181

Reviewer checklist

Test coverage seems ok.
Appropriate labels assigned.

codecov-commenter · 2024-02-29T22:53:24Z

Codecov Report

Merging #2549 (5799170) into master (91227e5) will decrease coverage by 23.59%.
The diff coverage is 100.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%)`	⬆️
tracer-php	`12.71% <ø> (-62.01%)`	⬇️

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%)`	⬆️
ext/handlers_http.h	`94.40% <100.00%> (+0.37%)`	⬆️

... 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.

pr-commenter · 2024-03-01T17:27:27Z

Benchmarks

Benchmark execution time: 2024-03-19 12:14:47

Comparing candidate commit 8fcf551 in PR branch munir/add-p-to-php with baseline commit 91227e5 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%]

mabdinur · 2024-03-06T14:36:18Z

We will need to ignore/filter out the p value in snapshot tests

bwoebi · 2024-03-06T15:09:22Z

@mabdinur Yep, needs a change in the test-agent to filter them there.

Signed-off-by: Bob Weinand <[email protected]>

…ation Signed-off-by: Bob Weinand <[email protected]>

mabdinur · 2024-03-19T14:08:35Z

@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 🥳

PROFeNoM

LGTM 🚢

app p to php

4d2f61b

mabdinur force-pushed the munir/add-p-to-php branch from 4f7b95b to 4d2f61b Compare February 29, 2024 22:49

mabdinur commented Mar 1, 2024

View reviewed changes

Comment thread ext/distributed_tracing_headers.c Outdated

bwoebi force-pushed the munir/add-p-to-php branch from 22ca5ed to c47bc79 Compare March 1, 2024 16:54

mabdinur commented Mar 1, 2024

View reviewed changes

Comment thread ext/handlers_http.h

Comment thread ext/ddtrace.c

mabdinur requested a review from bouwkast March 4, 2024 14:34

mabdinur marked this pull request as ready for review March 6, 2024 14:35

mabdinur requested a review from a team as a code owner March 6, 2024 14:35

mabdinur force-pushed the munir/add-p-to-php branch 2 times, most recently from ebac26a to 4d2f61b Compare March 8, 2024 14:48

bwoebi and others added 2 commits March 8, 2024 10:04

Finish implementation of _dd.parent_id

1248352

Signed-off-by: Bob Weinand <[email protected]>

ignore tracestate in field in snapshots

167ab76

mabdinur commented Mar 8, 2024

View reviewed changes

Comment thread tests/OpenTelemetry/Integration/API/TracerTest.php Outdated

mabdinur commented Mar 8, 2024

View reviewed changes

Comment thread tests/OpenTelemetry/Integration/API/TracerTest.php Outdated

mabdinur commented Mar 8, 2024

View reviewed changes

Comment thread tests/OpenTelemetry/Integration/API/TracerTest.php Outdated

mabdinur commented Mar 8, 2024

View reviewed changes

Comment thread tests/OpenTelemetry/Integration/InteroperabilityTest.php Outdated

mabdinur force-pushed the munir/add-p-to-php branch 2 times, most recently from 33ab400 to 032994f Compare March 12, 2024 22:14

mabdinur force-pushed the munir/add-p-to-php branch from 88d4487 to 9cc9690 Compare March 18, 2024 17:56

fix tests

1d54cb8

mabdinur force-pushed the munir/add-p-to-php branch from 9cc9690 to 1d54cb8 Compare March 18, 2024 17:59

Merge branch 'master' into munir/add-p-to-php

fe1dcd2

mabdinur force-pushed the munir/add-p-to-php branch from 3f267ac to fe1dcd2 Compare March 18, 2024 18:24

mabdinur and others added 3 commits March 18, 2024 14:36

handle errors

b953744

fix testGetSpanContextWithRemoteParent

5799170

Take the proper span context into account for OTel tracecontext gener…

8fcf551

…ation Signed-off-by: Bob Weinand <[email protected]>

PROFeNoM mentioned this pull request Mar 19, 2024

fix(otel): Strict attributes and tracestate comparisons DataDog/system-tests#2236

Merged

9 tasks

PROFeNoM approved these changes Mar 19, 2024

View reviewed changes

PROFeNoM merged commit 4c0dcf6 into master Mar 19, 2024

PROFeNoM deleted the munir/add-p-to-php branch March 19, 2024 16:43

github-actions Bot added this to the 0.99.0 milestone Mar 19, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

w3c phase 2: add last parent_id to tracestate#2549

w3c phase 2: add last parent_id to tracestate#2549
PROFeNoM merged 8 commits intomasterfrom
munir/add-p-to-php

mabdinur commented Feb 29, 2024

Uh oh!

codecov-commenter commented Feb 29, 2024 •

edited

Loading

Uh oh!

Uh oh!

pr-commenter Bot commented Mar 1, 2024 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

mabdinur commented Mar 6, 2024

Uh oh!

bwoebi commented Mar 6, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mabdinur commented Mar 19, 2024

Uh oh!

PROFeNoM left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

mabdinur commented Feb 29, 2024

Description

Reviewer checklist

Uh oh!

codecov-commenter commented Feb 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

pr-commenter Bot commented Mar 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Benchmarks

scenario:ContextPropagationBench/benchExtractTraceContext128Bit

scenario:ContextPropagationBench/benchExtractTraceContext128Bit-opcache

scenario:LaravelBench/benchLaravelBaseline-opcache

scenario:PDOBench/benchPDOBaseline-opcache

scenario:PDOBench/benchPDOOverhead-opcache

scenario:PDOBench/benchPDOOverheadWithDBM-opcache

scenario:SamplingRuleMatchingBench/benchRegexMatching2

scenario:SamplingRuleMatchingBench/benchRegexMatching2-opcache

scenario:SamplingRuleMatchingBench/benchRegexMatching3

scenario:SamplingRuleMatchingBench/benchRegexMatching4

scenario:SpanBench/benchOpenTelemetryAPI

scenario:SpanBench/benchOpenTelemetryAPI-opcache

Uh oh!

Uh oh!

Uh oh!

mabdinur commented Mar 6, 2024

Uh oh!

bwoebi commented Mar 6, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mabdinur commented Mar 19, 2024

Uh oh!

PROFeNoM left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

codecov-commenter commented Feb 29, 2024 •

edited

Loading

pr-commenter Bot commented Mar 1, 2024 •

edited

Loading