Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extract Upper64 bit trace ID from extension response (#6041 => v2) #6181

Merged
merged 3 commits into from
Oct 22, 2024

Conversation

agocs
Copy link
Contributor

@agocs agocs commented Oct 22, 2024

Summary of changes

I've updated the Lambda extension so it is capable of returning a 128 bit trace ID when a tracer calls the /lambda/start-invocation endpoint in this PR

As per the
RFC
, the

lower 64 bits in x-datadog-trace-id (decimal) and the higher 64 bits
in x-datadog-tags header under _dd.p.tid (hex) tag.

This change modifies the function that calls /lambda/start-invocation, allowing it to pick out the upper 64 bits of the trace ID and set the resulting 128-bit trace ID in the extracted context.

Reason for change

The Lambda Extension may now return a 128 bit trace ID when a Step Function invokes a Lambda Function.

Implementation details

I rewrote LambdaCommon's CreatePlaceholderScope so it uses SpanContextPropagator.Instance.Extract rather than extracting trace context elements one by one.

Test coverage

Added a unit test for 128 bit trace IDs. Fixed existing unit tests so they pass a dictionary of headers to CreatePlaceholderScope. Removed a unit test that only passes SamplingPriority, since a distributed trace with only a sampling priority is hardly a distributed trace at all.

Other details

Backport to 2.x of #6041

## Summary of changes

I've updated the Lambda extension so it is capable of returning a 128
bit trace ID when a tracer calls the `/lambda/start-invocation` endpoint
[in this PR](DataDog/datadog-agent#27988)

As per [the
RFC](https://datadoghq.atlassian.net/wiki/spaces/RUMP/pages/3545630931/RFC+Support+128+bit+trace+IDs+in+RUM+SDKs#:~:text=For%20Datadog%20headers%2C%20the%20128%20bit%20trace%20id%20is%20sent%20in%20two%20parts%2C%20lower%2064%20bits%20in%20x%2Ddatadog%2Dtrace%2Did%20(decimal)%20and%20the%20higher%2064%20bits%20in%20x%2Ddatadog%2Dtags%20header%20under%20_dd.p.tid%20(hex)%20tag),
the

> lower 64 bits in `x-datadog-trace-id` (decimal) and the higher 64 bits
in `x-datadog-tags` header under `_dd.p.tid` (hex) tag.

This change modifies the function that calls `/lambda/start-invocation`,
allowing it to pick out the upper 64 bits of the trace ID and set the
resulting 128-bit trace ID in the extracted context.


## Reason for change

The Lambda Extension may now return a 128 bit trace ID when a Step
Function invokes a Lambda Function.

## Implementation details

I rewrote LambdaCommon's `CreatePlaceholderScope` so it uses
`SpanContextPropagator.Instance.Extract` rather than extracting trace
context elements one by one.

## Test coverage

Added a unit test for 128 bit trace IDs. Fixed existing unit tests so
they pass a dictionary of headers to CreatePlaceholderScope. Removed a
unit test that only passes SamplingPriority, since a distributed trace
with only a sampling priority is hardly a distributed trace at all.

## Other details
Backported to 2.x in (TODO)
<!-- Fixes #{issue} -->

<!-- ⚠️ Note: where possible, please obtain 2 approvals prior to
merging. Unless CODEOWNERS specifies otherwise, for external teams it is
typically best to have one review from a team member, and one review
from apm-dotnet. Trivial changes do not require 2 reviews. -->

---------

Co-authored-by: Lucas Pimentel <[email protected]>
Co-authored-by: Andrew Lock <[email protected]>
Co-authored-by: Daniel Romano <[email protected]>
Co-authored-by: Steven Bouwkamp <[email protected]>
Co-authored-by: Anna <[email protected]>
Co-authored-by: NachoEchevarria <[email protected]>
Co-authored-by: William Conti <[email protected]>
Co-authored-by: Kevin Gosse <[email protected]>
Co-authored-by: Tony Redondo <[email protected]>
Co-authored-by: Gregory LEOCADIE <[email protected]>
@agocs agocs marked this pull request as ready for review October 22, 2024 20:23
@agocs agocs requested review from a team as code owners October 22, 2024 20:23
@lucaspimentel lucaspimentel added type:enhancement Improvement to an existing feature area:serverless labels Oct 22, 2024
@andrewlock
Copy link
Member

Execution-Time Benchmarks Report ⏱️

Execution-time results for samples comparing the following branches/commits:

Execution-time benchmarks measure the whole time it takes to execute a program. And are intended to measure the one-off costs. Cases where the execution time results for the PR are worse than latest master results are shown in red. The following thresholds were used for comparing the execution times:

  • Welch test with statistical test for significance of 5%
  • Only results indicating a difference greater than 5% and 5 ms are considered.

Note that these results are based on a single point-in-time result for each branch. For full results, see the dashboard.

Graphs show the p99 interval based on the mean and StdDev of the test run, as well as the mean value of the run (shown as a diamond below the graph).

gantt
    title Execution time (ms) FakeDbCommand (.NET Framework 4.6.2) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6181) - mean (75ms)  : 63, 86
     .   : milestone, 75,
    master - mean (71ms)  : 67, 75
     .   : milestone, 71,

    section CallTarget+Inlining+NGEN
    This PR (6181) - mean (1,022ms)  : 1006, 1037
     .   : milestone, 1022,
    master - mean (1,126ms)  : 1071, 1181
     .   : milestone, 1126,

Loading
gantt
    title Execution time (ms) FakeDbCommand (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6181) - mean (110ms)  : 107, 113
     .   : milestone, 110,
    master - mean (109ms)  : 106, 112
     .   : milestone, 109,

    section CallTarget+Inlining+NGEN
    This PR (6181) - mean (701ms)  : 685, 717
     .   : milestone, 701,
    master - mean (789ms)  : 711, 867
     .   : milestone, 789,

Loading
gantt
    title Execution time (ms) FakeDbCommand (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6181) - mean (93ms)  : 89, 96
     .   : milestone, 93,
    master - mean (92ms)  : 89, 95
     .   : milestone, 92,

    section CallTarget+Inlining+NGEN
    This PR (6181) - mean (665ms)  : 644, 687
     .   : milestone, 665,
    master - mean (732ms)  : 717, 748
     .   : milestone, 732,

Loading
gantt
    title Execution time (ms) HttpMessageHandler (.NET Framework 4.6.2) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6181) - mean (191ms)  : 187, 194
     .   : milestone, 191,
    master - mean (190ms)  : 186, 195
     .   : milestone, 190,

    section CallTarget+Inlining+NGEN
    This PR (6181) - mean (1,108ms)  : 1084, 1133
     .   : milestone, 1108,
    master - mean (1,198ms)  : 1174, 1222
     .   : milestone, 1198,

Loading
gantt
    title Execution time (ms) HttpMessageHandler (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6181) - mean (276ms)  : 273, 280
     .   : milestone, 276,
    master - mean (277ms)  : 272, 282
     .   : milestone, 277,

    section CallTarget+Inlining+NGEN
    This PR (6181) - mean (870ms)  : 848, 891
     .   : milestone, 870,
    master - mean (943ms)  : 927, 959
     .   : milestone, 943,

Loading
gantt
    title Execution time (ms) HttpMessageHandler (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6181) - mean (265ms)  : 260, 269
     .   : milestone, 265,
    master - mean (265ms)  : 261, 269
     .   : milestone, 265,

    section CallTarget+Inlining+NGEN
    This PR (6181) - mean (858ms)  : 829, 887
     .   : milestone, 858,
    master - mean (928ms)  : 907, 948
     .   : milestone, 928,

Loading

@datadog-ddstaging
Copy link

datadog-ddstaging bot commented Oct 22, 2024

Datadog Report

Branch report: chris.agocs/backport_6401
Commit report: 7beb1b2
Test service: dd-trace-dotnet

✅ 0 Failed, 342004 Passed, 1778 Skipped, 14h 21m 45.14s Total Time

@bouwkast bouwkast merged commit b29d984 into release/2.x Oct 22, 2024
71 of 74 checks passed
@bouwkast bouwkast deleted the chris.agocs/backport_6401 branch October 22, 2024 22:08
@github-actions github-actions bot added this to the vNext-v2 milestone Oct 22, 2024
@andrewlock andrewlock added the type:backport Feature backport to old version label Oct 23, 2024
@andrewlock andrewlock changed the title Backport 6401: Extract Upper64 bit trace ID from extension response Extract Upper64 bit trace ID from extension response (#6041 => v2) Oct 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:serverless type:backport Feature backport to old version type:enhancement Improvement to an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants