DataDog / datadog-lambda-js

The Datadog AWS Lambda Library for Node
Apache License 2.0
109 stars 35 forks source link

[w3c] Support W3C trace context propagation #429

Closed duncanista closed 9 months ago

duncanista commented 10 months ago

What does this PR do?

TLDR; Refactor code and delegate trace extraction to the tracer.

This PR does a couple things:

  1. Refactoring functions into classes to make trace context extraction simpler.
  2. Delegate the trace context extraction to the tracer, instead of doing it manually only for Datadog headers.
  3. Support W3C and Datadog headers extraction – actually, any supported by the current tracer.
  4. Add multiple new unit tests for every component.
  5. Separates Xray and Step Functions into services.

Motivation

Our library was extracting tracing context only for Datadog headers, the need to extract from W3C trace context is expected and already being done by the dd-trace package. Therefore, we should delegate our previous manual extraction to the already predefined functions.

Testing Guidelines

Services using W3C context in headers in AWS Lambda.

Screenshot 2023-11-07 at 8 26 07 PM

Screenshot 2023-11-07 at 8 36 32 PM

Additional Notes

Types of Changes

Check all that apply

codecov-commenter commented 10 months ago

Codecov Report

Attention: 66 lines in your changes are missing coverage. Please review.

Comparison is base (109a606) 81.19% compared to head (0e25e8c) 81.96%.

:exclamation: Current head 0e25e8c differs from pull request most recent head 1871ba7. Consider uploading reports for the commit 1871ba7 to get more accurate results

Files Patch % Lines
src/trace/span-context-wrapper.ts 56.25% 13 Missing and 1 partial :warning:
src/trace/xray-service.ts 91.26% 8 Missing and 1 partial :warning:
src/trace/step-function-service.ts 91.86% 6 Missing and 1 partial :warning:
src/trace/context/extractor.ts 90.56% 5 Missing :warning:
src/trace/context/extractors/sqs.ts 58.33% 4 Missing and 1 partial :warning:
src/trace/context/extractors/event-bridge.ts 66.66% 3 Missing and 1 partial :warning:
src/trace/context/extractors/http.ts 91.83% 3 Missing and 1 partial :warning:
src/trace/context/extractors/sns.ts 76.47% 3 Missing and 1 partial :warning:
src/trace/context/extractors/event-bridge-sqs.ts 80.00% 1 Missing and 2 partials :warning:
src/trace/context/extractors/kinesis.ts 81.25% 1 Missing and 2 partials :warning:
... and 5 more
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #429 +/- ## ========================================== + Coverage 81.19% 81.96% +0.76% ========================================== Files 50 54 +4 Lines 2042 2179 +137 Branches 467 506 +39 ========================================== + Hits 1658 1786 +128 - Misses 323 330 +7 - Partials 61 63 +2 ```

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

duncanista commented 10 months ago

It's normal for integration tests to not pass, since now, console patching only happens when dd-trace is available, which is a contradiction – and a breaking change. Will need to fix it either by doing a try-catch, but this might require to handle scenarios where we can get trace-context when there's no tracer: Xray, StepFunctionContext, and Custom extractor.

-- edit: fixed