-
-
Notifications
You must be signed in to change notification settings - Fork 900
fix(otel): exported logs and spans will now have matching trace IDs #2724
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
When using custom OTLP exporters via `telemetry.exporters` and
This occurred when tasks were triggered **without** a parent trace
context (e.g., via API or dashboard). In this scenario: - Spans were
correctly rewritten to use the generated `externalTraceId` - Logs kept
their original internal trace ID due to a bug in the early return logic
### Root Cause
In `ExternalLogRecordExporterWrapper.transformLogRecord()`, the early
return condition incorrectly included `!this.externalTraceContext`:
```typescript
if (!logRecord.spanContext || !this.externalTraceId ||
!this.externalTraceContext) { return logRecord; // Bug: Returns early
when externalTraceContext is undefined }
// This fallback logic was never reached:
const externalTraceId = this.externalTraceContext
? this.externalTraceContext.traceId
: this.externalTraceId;
```
### Fix
1. **Reordered logic in `transformLogRecord()`**: Move the
1. `externalTraceId` calculation before the early return, and check the
1. culated value instead of `this.externalTraceContext`:
```typescript
const externalTraceId = this.externalTraceContext
? this.externalTraceContext.traceId
: this.externalTraceId;
if (!logRecord.spanContext || !externalTraceId) {
return logRecord;
}
```
2. **Clarified `_isExternallySampled` logic**: Updated both
2. `ExternalSpanExporterWrapper` and `ExternalLogRecordExporterWrapper`
2. explicitly handle the case where there's no external trace context
2. a generated `externalTraceId` exists:
```typescript
this._isExternallySampled = externalTraceContext
? isTraceFlagSampled(externalTraceContext.traceFlags)
: !!externalTraceId;
```
### Impact
Logs and spans from the same task run will now have matching trace IDs
when exported to external observability tools, enabling proper trace correlation regardless of whether the task was triggered with or without a parent trace context.
`telemetry.logExporters` in `trigger.config.ts`, logs and spans were
exported with **different trace IDs**, breaking trace correlation in
external observability tools like Datadog.
🦋 Changeset detectedLatest commit: ee457aa The changes in this PR will be included in the next version bump. This PR includes changesets to release 26 packages
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 |
WalkthroughThis pull request adds a changeset entry for a patch release documenting a fix to OpenTelemetry integration in Trigger.dev. The corresponding code changes in Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/core/src/v3/otel/tracingSDK.ts (3)
317-320: Sampling fallback behavior looks correct when no externalTraceContextUsing
externalTraceContext ? isTraceFlagSampled(...) : !!externalTraceIdmakes the wrapper treat traces as sampled whenever there’s a generatedexternalTraceIdbut no upstream context, which matches the intent for internally-started tasks. The logic is sound; the!!externalTraceIdbit is slightly redundant given it’s typed asstring, but that’s purely cosmetic.
401-404: Consistent sampling semantics between span and log exportersMirroring the
_isExternallySampledcomputation fromExternalSpanExporterWrapperintoExternalLogRecordExporterWrapperkeeps span/log behavior aligned when there’s noexternalTraceContextbut a generatedexternalTraceId. This is important for ensuring both spans and logs are either exported or dropped together. Same minor note as above that!!externalTraceIdis redundant but harmless.
429-433: Guarding on spanContext prevents unnecessary proxyingEarly‑returning when
!logRecord.spanContext(and, defensively,!externalTraceId) makes sense: logs without a span context are passed through untouched, and the proxy is only used when there’s something to rewrite. GivenexternalTraceIdis always a non‑empty string in current usage, the second part of the condition is mostly defensive, but it doesn’t hurt readability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.changeset/brown-pots-beg.md(1 hunks)packages/core/src/v3/otel/tracingSDK.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
Files:
packages/core/src/v3/otel/tracingSDK.ts
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use zod for validation in packages/core and apps/webapp
Files:
packages/core/src/v3/otel/tracingSDK.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Files:
packages/core/src/v3/otel/tracingSDK.ts
**/*.{js,ts,jsx,tsx,json,md,css,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier
Files:
packages/core/src/v3/otel/tracingSDK.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger.config.ts : Configure OpenTelemetry instrumentations and exporters in trigger.config.ts for enhanced logging
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger.config.ts : Configure OpenTelemetry instrumentations and exporters in trigger.config.ts for enhanced logging
Applied to files:
packages/core/src/v3/otel/tracingSDK.ts.changeset/brown-pots-beg.md
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Export tasks with unique IDs within the project to enable proper task discovery and execution
Applied to files:
.changeset/brown-pots-beg.md
⏰ 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). (23)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (2)
.changeset/brown-pots-beg.md (1)
1-5: Changeset entry looks good.The format is correct and the summary accurately describes the bug fix. The message is clear enough for users consuming release notes, and the patch bump type is appropriate for this bug fix.
packages/core/src/v3/otel/tracingSDK.ts (1)
425-427: Deriving externalTraceId before guard fixes the mismatch bugComputing
externalTraceIdfromexternalTraceContext.traceIdwith a fallback to the generatedexternalTraceIdbefore any early return is exactly what’s needed so logs from tasks without a parent context still get rewritten to the same external trace ID as spans. This directly addresses the original bug around mismatched trace IDs.
When using custom OTLP exporters via
telemetry.exportersandThis occurred when tasks were triggered without a parent trace
context (e.g., via API or dashboard). In this scenario: - Spans were
correctly rewritten to use the generated
externalTraceId- Logs kepttheir original internal trace ID due to a bug in the early return logic
Root Cause
In
ExternalLogRecordExporterWrapper.transformLogRecord(), the earlyreturn condition incorrectly included
!this.externalTraceContext:Fix
transformLogRecord(): Move theexternalTraceIdcalculation before the early return, and check thethis.externalTraceContext:_isExternallySampledlogic: Updated bothExternalSpanExporterWrapperandExternalLogRecordExporterWrapperexternalTraceIdexists:Impact
Logs and spans from the same task run will now have matching trace IDs
when exported to external observability tools, enabling proper trace
correlation regardless of whether the task was triggered with or without a parent trace context.
telemetry.logExportersintrigger.config.ts, logs and spans wereexported with different trace IDs, breaking trace correlation in
external observability tools like Datadog.