dd-trace-js icon indicating copy to clipboard operation
dd-trace-js copied to clipboard

feat: add aws xray header context extraction

Open wconti27 opened this issue 2 years ago • 7 comments

What does this PR do?

Add code to extract Trace Context Propagation from AWSTraceHeader, which is used by dd-trace-java to propagate context for some AWS services.

Motivation

Improving AWS propagation & dd-trace-js / dd-trace-java compatability.

Plugin Checklist

Additional Notes

Security

Datadog employees:

  • [ ] If this PR touches code that signs or publishes builds or packages, or handles credentials of any kind, I've requested a review from @DataDog/security-design-and-guidance.
  • [ ] This PR doesn't touch any of that.

Unsure? Have a question? Request a review!

wconti27 avatar Dec 20 '23 18:12 wconti27

Overall package size

Self size: 6.26 MB Deduped: 60.76 MB No deduping: 61.04 MB

Dependency sizes

name version self size total size
@datadog/native-iast-taint-tracking 1.7.0 16.71 MB 16.72 MB
@datadog/native-appsec 7.1.1 14.39 MB 14.4 MB
@datadog/pprof 5.2.0 8.84 MB 9.21 MB
protobufjs 7.2.5 2.77 MB 6.56 MB
@datadog/native-iast-rewriter 2.3.0 2.15 MB 2.24 MB
@opentelemetry/core 1.14.0 872.87 kB 1.47 MB
@datadog/native-metrics 2.0.0 898.77 kB 1.3 MB
@opentelemetry/api 1.4.1 780.32 kB 780.32 kB
import-in-the-middle 1.7.3 67.62 kB 731.01 kB
msgpack-lite 0.1.26 201.16 kB 281.59 kB
opentracing 0.14.7 194.81 kB 194.81 kB
semver 7.5.4 93.4 kB 123.8 kB
pprof-format 2.1.0 111.69 kB 111.69 kB
@datadog/sketches-js 2.1.0 109.9 kB 109.9 kB
lodash.sortby 4.7.0 75.76 kB 75.76 kB
lru-cache 7.14.0 74.95 kB 74.95 kB
ipaddr.js 2.1.0 60.23 kB 60.23 kB
ignore 5.2.4 51.22 kB 51.22 kB
int64-buffer 0.1.10 49.18 kB 49.18 kB
shell-quote 1.8.1 44.96 kB 44.96 kB
istanbul-lib-coverage 3.2.0 29.34 kB 29.34 kB
tlhunter-sorted-set 0.1.0 24.94 kB 24.94 kB
limiter 1.1.5 23.17 kB 23.17 kB
dc-polyfill 0.1.4 23.1 kB 23.1 kB
retry 0.13.1 18.85 kB 18.85 kB
node-abort-controller 3.1.1 16.89 kB 16.89 kB
jest-docblock 29.7.0 8.99 kB 12.76 kB
crypto-randomuuid 1.0.0 11.18 kB 11.18 kB
path-to-regexp 0.1.7 6.78 kB 6.78 kB
koalas 1.0.2 6.47 kB 6.47 kB
methods 1.1.2 5.29 kB 5.29 kB
module-details-from-path 1.0.3 4.47 kB 4.47 kB

🤖 This report was automatically generated by heaviest-objects-in-the-universe

github-actions[bot] avatar Dec 20 '23 18:12 github-actions[bot]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 83.67%. Comparing base (de47a4b) to head (7d99016). Report is 2 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3898      +/-   ##
==========================================
- Coverage   85.23%   83.67%   -1.57%     
==========================================
  Files         247      112     -135     
  Lines       10961     4563    -6398     
  Branches       33       33              
==========================================
- Hits         9343     3818    -5525     
+ Misses       1618      745     -873     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Dec 20 '23 18:12 codecov[bot]

Benchmarks

Benchmark execution time: 2024-04-05 15:32:00

Comparing candidate commit 7d99016a495cac4d6737e74466f6939f7c50894b in PR branch conti/add-aws-xray-header-context-extraction with baseline commit de47a4b7a1dc3a4d846d3a7c356504f508fe5ca1 in branch master.

Found 2 performance improvements and 0 performance regressions! Performance is the same for 261 metrics, 3 unstable metrics.

scenario:plugin-graphql-with-depth-and-collapse-on-18

  • 🟩 max_rss_usage [-122.619MB; -101.609MB] or [-12.981%; -10.757%]

scenario:plugin-graphql-with-depth-off-18

  • 🟩 max_rss_usage [-125.707MB; -124.109MB] or [-13.060%; -12.894%]

pr-commenter[bot] avatar Dec 20 '23 18:12 pr-commenter[bot]

Needs a test to verify the injection works correctly. Also, is support for this something we actually have consensus on and is expected to exist across all languages, or are we just implementing this because Java happened to have it?

There's no consensus on which should be used by default at the moment. This is mainly being added since Java is the most popular language for APM, and if we want propagation between a Java producer, we need to be able to extract context using the AwsTraceHeader

wconti27 avatar Dec 27 '23 16:12 wconti27

I've been searching for this solution for a while, and anxiously await the merge and resolution to this! This will help many of us to implement proper tracing for our headless NodeJS services which use SQS!

rbonestell avatar Apr 04 '24 19:04 rbonestell

I've similarly been waiting for this to land as it would clean up disconnected spans in DataDog. We have a polyglot environment and the proposed changes above work great for Java. It would be helpful to have the same behavior across languages.

@Qard @rochdev are there any blockers to validating or merging the proposed changes? I only see linting issues in the diff. I'd love to contribute to get this over the line.

jeebay avatar Apr 16 '24 20:04 jeebay

Is this still alive? We're really looking forward to this implementation. Its a bit glaring that the lambdas support this feature but containerized services don't.

ConnorSinnott avatar May 24 '24 00:05 ConnorSinnott