Skip to content

feat(core/client): propagate w3c trace headers#8156

Merged
kuhe merged 2 commits into
mainfrom
kuhe/chore/w3c
Jul 8, 2026
Merged

feat(core/client): propagate w3c trace headers#8156
kuhe merged 2 commits into
mainfrom
kuhe/chore/w3c

Conversation

@kuhe

@kuhe kuhe commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Issue

modified from #8147

Description

propagate traceparent tracestate baggage headers from Lambda invoke store.

Testing

CI, new unit tests

Checklist

  • If the PR is a feature, add integration tests (*.integ.spec.ts) or E2E tests.
    • It's not a feature.
  • My E2E tests are resilient to concurrent i/o.
    • I didn't write any E2E tests.
  • I added access level annotations e.g. @public, @internal tags and enabled doc generation on the package. Remember that access level annotations go below the description, not above.
    • I didn't add any public functions.
  • Streams - how do they work?? My WebStream readers/locks are properly lifecycled. Node.js stream backpressure is handled. Error handling.
    • No streams here.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@kuhe kuhe requested a review from a team as a code owner July 6, 2026 16:37
@kuhe kuhe force-pushed the kuhe/chore/w3c branch from 58488bb to 49b7c06 Compare July 6, 2026 16:39
const invokeStore = await InvokeStore.getInstanceAsync();
const traceIdFromInvokeStore = invokeStore?.getXRayTraceId();
const traceId = traceIdFromInvokeStore ?? traceIdFromEnv;
{

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When traceparent is already set in request headers (ie: by user), the current logic still unconditionally injects tracestate and baggage from InvokeStore. Do we want this? From the spec, I understand it differently

Section 3.4: "If the value of the traceparent field wasn't changed before propagation, tracestate MUST NOT be modified as well."

WDYT?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't read the spec initially, only copied the code function.

I think this is a good catch, I've updated this in the 2nd commit to only take the secondary headers from invokeStore if the traceparent was from the invokeStore.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@darklight3it darklight3it Jul 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a note. Sanitization is the only thing we should do regardless.

3.2.1 Header Name

Header name: traceparent

In order to increase interoperability across multiple protocols and encourage successful integration, by default vendors SHOULD keep the header name lowercase. The header name is a single word without any delimiters, for example, a hyphen (-).

Vendors MUST expect the header name in any case (upper, lower, mixed), and SHOULD send the header name in lowercase.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@darklight3it oh, I see. I converted the code incorrectly, thanks.

@kuhe kuhe force-pushed the kuhe/chore/w3c branch from 1ba0630 to 1e8ed04 Compare July 7, 2026 15:08

@maxday maxday left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@kuhe kuhe merged commit e5f383c into main Jul 8, 2026
7 checks passed
@kuhe kuhe deleted the kuhe/chore/w3c branch July 8, 2026 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants