Skip to content

fix(core): external trace context leaks across warm-started runs#3768

Merged
nicktrn merged 2 commits into
mainfrom
fix/external-trace-context-warm-start-tri-10151
May 28, 2026
Merged

fix(core): external trace context leaks across warm-started runs#3768
nicktrn merged 2 commits into
mainfrom
fix/external-trace-context-warm-start-tri-10151

Conversation

@nicktrn
Copy link
Copy Markdown
Collaborator

@nicktrn nicktrn commented May 28, 2026

On warm-started workers with processKeepAlive enabled, every run's attempt span was exported with the first run's traceId and parentSpanId. ExternalSpanExporterWrapper and ExternalLogRecordExporterWrapper captured externalTraceContext at TracingSDK construction, and the SDK is memoized for the worker's lifetime - so the per-run reset of StandardTraceContextManager.traceContext never reached the wrappers.

Reported by a customer running v4.4.x: 33 distinct runs on the same host/pid showed up in their APM as siblings of one parent span.

Fix: the wrappers now read external context live from the trace context manager per export. Runs without an external trace context fall through to the unchanged externalTraceId fallback - no behaviour change for them.

Regression test in packages/core/test/externalSpanExporterWrapper.test.ts asserts that reassigning the manager between exports produces correctly-parented spans.

@nicktrn nicktrn self-assigned this May 28, 2026
@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented May 28, 2026

🦋 Changeset detected

Latest commit: 4dc2da5

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 32 packages
Name Type
@trigger.dev/core Patch
@trigger.dev/build Patch
trigger.dev Patch
@trigger.dev/plugins Patch
@trigger.dev/python Patch
@trigger.dev/redis-worker Patch
@trigger.dev/schema-to-json Patch
@trigger.dev/sdk Patch
@internal/cache Patch
@internal/clickhouse Patch
@internal/llm-model-catalog Patch
@trigger.dev/rbac Patch
@internal/redis Patch
@internal/replication Patch
@internal/run-engine Patch
@internal/schedule-engine Patch
@internal/testcontainers Patch
@internal/tracing Patch
@internal/tsql Patch
@internal/zod-worker Patch
references-ai-chat Patch
d3-chat Patch
references-d3-openai-agents Patch
references-nextjs-realtime Patch
references-realtime-hooks-test Patch
references-realtime-streams Patch
references-telemetry Patch
@internal/sdk-compat-tests Patch
@trigger.dev/react-hooks Patch
@trigger.dev/rsc Patch
@trigger.dev/database Patch
@trigger.dev/otlp-importer Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 28, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 68a3e128-ac7f-461d-a87c-a242518df908

📥 Commits

Reviewing files that changed from the base of the PR and between 7f42d79 and 4dc2da5.

📒 Files selected for processing (1)
  • packages/core/test/externalSpanExporterWrapper.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/core/test/externalSpanExporterWrapper.test.ts
📜 Recent review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (28)
  • GitHub Check: internal / 🧪 Unit Tests: Internal (5, 8)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (2, 8)
  • GitHub Check: internal / 🧪 Unit Tests: Internal (7, 8)
  • GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (7, 8)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (4, 8)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (1, 8)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (5, 8)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (8, 8)
  • GitHub Check: internal / 🧪 Unit Tests: Internal (8, 8)
  • GitHub Check: internal / 🧪 Unit Tests: Internal (3, 8)
  • GitHub Check: internal / 🧪 Unit Tests: Internal (6, 8)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (6, 8)
  • GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
  • GitHub Check: sdk-compat / Node.js 20.20 (ubuntu-latest)
  • GitHub Check: sdk-compat / Node.js 22.12 (ubuntu-latest)
  • GitHub Check: sdk-compat / Bun Runtime
  • GitHub Check: internal / 🧪 Unit Tests: Internal (2, 8)
  • GitHub Check: internal / 🧪 Unit Tests: Internal (1, 8)
  • GitHub Check: packages / 🧪 Unit Tests: Packages (1, 1)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (3, 8)
  • GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
  • GitHub Check: internal / 🧪 Unit Tests: Internal (4, 8)
  • GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
  • GitHub Check: sdk-compat / Cloudflare Workers
  • GitHub Check: typecheck / typecheck
  • GitHub Check: e2e-webapp / 🧪 E2E Tests: Webapp
  • GitHub Check: sdk-compat / Deno Runtime

Walkthrough

This PR fixes a trace context leak in warm-started workers with processKeepAlive enabled. The core change refactors ExternalSpanExporterWrapper and ExternalLogRecordExporterWrapper to read external trace context live from StandardTraceContextManager at export/transform time instead of capturing it eagerly at construction. This prevents subsequent runs from incorrectly reusing the first run's traceId and parentSpanId in exported attempt spans. The fix includes wrapper instantiation updates in TracingSDK, live context reading logic in span and log transformation methods, a regression test validating the behavior, and a changeset documenting the patch.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description provides a thorough explanation of the problem, root cause, and solution, but is missing the required template sections: Testing, Changelog, Screenshots, and Checklist. Add the required template sections including Testing (steps taken to verify), Changelog (summary of changes), Screenshots (if applicable), and complete the Checklist before merging.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix(core): external trace context leaks across warm-started runs' directly and clearly summarizes the main fix being implemented—preventing external trace context leaks in warm-started workers.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/external-trace-context-warm-start-tri-10151

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install failed: dependency version conflict. Check your lock file or package.json.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 4 additional findings.

Open in Devin Review

coderabbitai[bot]

This comment was marked as resolved.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 28, 2026

Actionable comments posted: 0

@nicktrn nicktrn enabled auto-merge (squash) May 28, 2026 10:37
@nicktrn nicktrn merged commit c043c4a into main May 28, 2026
50 checks passed
@nicktrn nicktrn deleted the fix/external-trace-context-warm-start-tri-10151 branch May 28, 2026 10:43
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.

2 participants