Skip to content

fix(node): Ensure startNewTrace propagates traceId in OTel environments - #19963

Merged
logaretm merged 4 commits into
developfrom
awad/js-1994-startnewtrace-does-not-use-same-trace-id-for-spans-in
Mar 26, 2026
Merged

fix(node): Ensure startNewTrace propagates traceId in OTel environments#19963
logaretm merged 4 commits into
developfrom
awad/js-1994-startnewtrace-does-not-use-same-trace-id-for-spans-in

Conversation

@logaretm

@logaretmlogaretm commented Mar 24, 2026

Copy link
Copy Markdown
Member

Summary

  • Add OTel-aware startNewTrace implementation that injects the new traceId as a remote span context into the OTel context
  • Add startNewTrace to the AsyncContextStrategy interface so OTel can override the default behavior
  • Register the new implementation in the OTel async context strategy

Root Cause

startNewTrace set a new traceId on the Sentry scope's propagation context but only called withActiveSpan(null, callback), which in OTel translates to trace.deleteSpan(context.active()). This removed the active span but did not inject the new traceId into the OTel context. Each subsequent startInactiveSpan call created a root span with a fresh random traceId from OTel's tracer.

The fix follows the same pattern as continueTrace — injecting the traceId as a remote span context via trace.setSpanContext() so all spans in the callback inherit it.

Closes#19952

CopilotAI review requested due to automatic review settings March 24, 2026 19:33
@linear-code

Copy link
Copy Markdown

@github-actions

github-actionsBot commented Mar 24, 2026

Copy link
Copy Markdown
Contributor

Semver Impact of This PR

🟢 Patch (bug fixes)

📋 Changelog Preview

This is how your changes will appear in the changelog.
Entries from this PR are highlighted with a left border (blockquote style).


New Features ✨

  • (node) Add nodeRuntimeMetricsIntegration by chargome in #19923
  • (nuxt) Support parametrized SSR routes in Nuxt 5 by s1gr1d in #19977

Bug Fixes 🐛

  • (e2e) Pin @opentelemetry/api to 1.9.0 in ts3.8 test app by logaretm in #19992
  • (node) Ensure startNewTrace propagates traceId in OTel environments by logaretm in #19963
  • (opentelemetry) Convert seconds timestamps in span.end() to milliseconds by logaretm in #19958

Documentation 📚

  • (release) Update publishing-a-release.md by nicohrubec in #19982

Internal Changes 🔧

  • (core) Consolidate getOperationName into one shared utility by nicohrubec in #19971
  • (deno) Expand Deno E2E test coverage by chargome in #19957

🤖 This preview updates automatically when you update the PR.

Comment threadpackages/opentelemetry/src/trace.ts Outdated

@cursorcursorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Comment threadpackages/opentelemetry/src/trace.ts Outdated

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds an OpenTelemetry-aware startNewTrace pathway so that, in OTEL-powered environments, spans created within the callback consistently inherit the newly generated traceId (matching the behavior of continueTrace).

Changes:

  • Implement OTEL-specific startNewTrace that injects a remote SpanContext into the active OTEL context.
  • Extend AsyncContextStrategy to allow OTEL to override core startNewTrace behavior, and register the override in the OTEL strategy.
  • Add regression tests covering startNewTrace traceId propagation in OTEL environments.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
FileDescription
packages/opentelemetry/src/trace.tsAdds OTEL-specific startNewTrace which sets a new trace context via OTEL context propagation.
packages/opentelemetry/src/asyncContextStrategy.tsRegisters startNewTrace on the OTEL async context strategy so core can delegate to it.
packages/core/src/tracing/trace.tsDelegates startNewTrace to the active async context strategy when available.
packages/core/src/asyncContext/types.tsExtends AsyncContextStrategy interface with optional startNewTrace.
packages/opentelemetry/test/trace.test.tsAdds regression tests to validate traceId behavior within startNewTrace.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment threadpackages/opentelemetry/src/trace.ts Outdated
Comment threadpackages/opentelemetry/test/trace.test.ts
@github-actions

github-actionsBot commented Mar 24, 2026

Copy link
Copy Markdown
Contributor

size-limit report 📦

PathSize% ChangeChange
@sentry/browser25.69 kB--
@sentry/browser - with treeshaking flags24.17 kB--
@sentry/browser (incl. Tracing)42.67 kB--
@sentry/browser (incl. Tracing, Profiling)47.33 kB--
@sentry/browser (incl. Tracing, Replay)81.48 kB--
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags71.06 kB--
@sentry/browser (incl. Tracing, Replay with Canvas)86.17 kB--
@sentry/browser (incl. Tracing, Replay, Feedback)98.41 kB--
@sentry/browser (incl. Feedback)42.48 kB--
@sentry/browser (incl. sendFeedback)30.35 kB--
@sentry/browser (incl. FeedbackAsync)35.4 kB--
@sentry/browser (incl. Metrics)26.96 kB--
@sentry/browser (incl. Logs)27.1 kB--
@sentry/browser (incl. Metrics & Logs)27.78 kB--
@sentry/react27.45 kB--
@sentry/react (incl. Tracing)45.01 kB--
@sentry/vue30.13 kB--
@sentry/vue (incl. Tracing)44.52 kB--
@sentry/svelte25.7 kB--
CDN Bundle28.36 kB+0.04%+9 B 🔺
CDN Bundle (incl. Tracing)43.58 kB+0.04%+14 B 🔺
CDN Bundle (incl. Logs, Metrics)29.23 kB+0.04%+10 B 🔺
CDN Bundle (incl. Tracing, Logs, Metrics)44.45 kB+0.04%+16 B 🔺
CDN Bundle (incl. Replay, Logs, Metrics)68.31 kB+0.02%+10 B 🔺
CDN Bundle (incl. Tracing, Replay)80.42 kB+0.02%+13 B 🔺
CDN Bundle (incl. Tracing, Replay, Logs, Metrics)81.32 kB+0.02%+15 B 🔺
CDN Bundle (incl. Tracing, Replay, Feedback)85.99 kB+0.02%+14 B 🔺
CDN Bundle (incl. Tracing, Replay, Feedback, Logs, Metrics)86.87 kB+0.02%+13 B 🔺
CDN Bundle - uncompressed82.75 kB+0.06%+48 B 🔺
CDN Bundle (incl. Tracing) - uncompressed128.67 kB+0.04%+48 B 🔺
CDN Bundle (incl. Logs, Metrics) - uncompressed85.61 kB+0.06%+48 B 🔺
CDN Bundle (incl. Tracing, Logs, Metrics) - uncompressed131.54 kB+0.04%+48 B 🔺
CDN Bundle (incl. Replay, Logs, Metrics) - uncompressed209.27 kB+0.03%+48 B 🔺
CDN Bundle (incl. Tracing, Replay) - uncompressed245.55 kB+0.02%+48 B 🔺
CDN Bundle (incl. Tracing, Replay, Logs, Metrics) - uncompressed248.4 kB+0.02%+48 B 🔺
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed258.46 kB+0.02%+48 B 🔺
CDN Bundle (incl. Tracing, Replay, Feedback, Logs, Metrics) - uncompressed261.3 kB+0.02%+48 B 🔺
@sentry/nextjs (client)47.4 kB--
@sentry/sveltekit (client)43.12 kB--
@sentry/node-core56.51 kB+0.12%+65 B 🔺
@sentry/node173.55 kB+0.04%+64 B 🔺
@sentry/node - without tracing96.54 kB+0.08%+77 B 🔺
@sentry/aws-serverless113.54 kB+0.07%+72 B 🔺

View base workflow run

@github-actions

github-actionsBot commented Mar 24, 2026

Copy link
Copy Markdown
Contributor

node-overhead report 🧳

Note: This is a synthetic benchmark with a minimal express app and does not necessarily reflect the real-world performance impact in an application.
⚠️Warning: Base artifact is not the latest one, because the latest workflow run is not done yet. This may lead to incorrect results. Try to re-run all tests to get up to date results.

ScenarioRequests/s% of BaselinePrev. Requests/sChange %
GET Baseline8,687-9,043-4%
GET With Sentry1,66119%1,673-1%
GET With Sentry (error only)6,00369%6,094-1%
POST Baseline1,164-1,202-3%
POST With Sentry54647%588-7%
POST With Sentry (error only)1,02288%1,059-3%
MYSQL Baseline3,160-3,246-3%
MYSQL With Sentry50416%451+12%
MYSQL With Sentry (error only)2,56681%2,637-3%

View base workflow run

Comment threadpackages/opentelemetry/src/trace.ts
@logaretm
logaretmforce-pushed the awad/js-1994-startnewtrace-does-not-use-same-trace-id-for-spans-in branch from 9572217 to bf0ba37CompareMarch 26, 2026 01:09

@nicohrubecnicohrubec 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 looks all good to me. just one question did you verify that this also fixes #18401? seems like it might be a different issue

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

Looks good, but also not sure about the issue Nico mentioned above. It's Cloudflare-related

@logaretm
logaretmforce-pushed the awad/js-1994-startnewtrace-does-not-use-same-trace-id-for-spans-in branch from c57247f to 132b408CompareMarch 26, 2026 13:47
@logaretm

Copy link
Copy Markdown
MemberAuthor

@nicohrubec@s1gr1d You are both correct, doesn't seem related not sure how it made it into the description.

logaretmand others added 4 commits March 26, 2026 12:39
…ents
`startNewTrace` only set the traceId on the Sentry scope's propagation
context but did not inject it into the OTel context. This caused each
`startInactiveSpan` call within the callback to get a fresh random
traceId from OTel's tracer instead of sharing the one from `startNewTrace`.
Add an OTel-aware `startNewTrace` implementation that injects the new
traceId as a remote span context, following the same pattern as
`continueTrace`.
Closes#19952Closes#18401
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…scope leakage
Address review feedback: calling setPropagationContext before context.with
mutated the outer scope, leaking the new traceId after the callback returned.
Move it inside the context.with callback so it only affects the cloned scope.
Also add a regression test asserting the outer scope is not modified.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Ensures TraceFlags.NONE on the remote span context does not cause
the sampler to inherit a false sampling decision from the parent.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Use an explicit traceId in the inner scope instead of relying on
generateTraceId producing different values, which can be deterministic
in CI with mocked random.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@logaretm
logaretmforce-pushed the awad/js-1994-startnewtrace-does-not-use-same-trace-id-for-spans-in branch from 132b408 to ebd5c4cCompareMarch 26, 2026 16:39
@logaretm
logaretm merged commit c0d52df into developMar 26, 2026
1144 of 1156 checks passed
@logaretm
logaretm deleted the awad/js-1994-startnewtrace-does-not-use-same-trace-id-for-spans-in branch March 26, 2026 18:57
Sign up for freeto 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.

startNewTrace does not use same trace id for spans in callback

4 participants

@logaretm@nicohrubec@s1gr1d