latest @opentelemetry packages and correlate external traces #2334
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
🦋 Changeset detectedLatest commit: 54a3f2a The changes in this PR will be included in the next version bump. 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 set of changes introduces a comprehensive overhaul of trace context management and OpenTelemetry (OTEL) integration across the codebase. A new trace context API is implemented, featuring a Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Complexity label: Complex
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (7)
📒 Files selected for processing (40)
💤 Files with no reviewable changes (3)
✅ Files skipped from review due to trivial changes (5)
🚧 Files skipped from review as they are similar to previous changes (31)
🧰 Additional context used📓 Path-based instructions (2)**/*.{ts,tsx}📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
Files:
{packages/core,apps/webapp}/**/*.{ts,tsx}📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
Files:
🧠 Learnings (14)📓 Common learnings📚 Learning: applies to **/trigger/**/*.{ts,tsx,js,jsx} : always generate trigger.dev tasks using the `task` func...Applied to files:
📚 Learning: applies to **/trigger/**/*.{ts,tsx,js,jsx} : you must use `@trigger.dev/sdk/v3` when writing trigger...Applied to files:
📚 Learning: applies to trigger.config.ts : global lifecycle hooks, telemetry, runtime, machine settings, log lev...Applied to files:
📚 Learning: before generating any code for trigger.dev tasks, verify: (1) are you importing from `@trigger.dev/s...Applied to files:
📚 Learning: applies to apps/webapp/**/*.{ts,tsx} : in the webapp, all environment variables must be accessed thr...Applied to files:
📚 Learning: applies to **/trigger/**/*.{ts,tsx,js,jsx} : when implementing scheduled (cron) tasks, use `schedule...Applied to files:
📚 Learning: applies to **/trigger/**/*.{ts,tsx,js,jsx} : never generate deprecated code patterns using `client.d...Applied to files:
📚 Learning: applies to **/trigger/**/*.{ts,tsx,js,jsx} : when triggering a task from backend code, use `tasks.tr...Applied to files:
📚 Learning: applies to **/trigger/**/*.{ts,tsx,js,jsx} : when logging in tasks, use the `logger` api (`logger.de...Applied to files:
📚 Learning: in the trigger.dev codebase, the supervisor component uses docker_enforce_machine_presets while the ...Applied to files:
📚 Learning: in the trigger.dev project, .env.example files should contain actual example secret values rather th...Applied to files:
📚 Learning: applies to trigger.config.ts : the `trigger.config.ts` file must use `defineconfig` from `@trigger.d...Applied to files:
📚 Learning: applies to trigger.config.ts : build extensions such as `additionalfiles`, `additionalpackages`, `ap...Applied to files:
🧬 Code Graph Analysis (1)packages/core/src/v3/otel/tracingSDK.ts (6)
⏰ 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)
🔇 Additional comments (6)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://app.smartcontext.ai/default/https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (4)
packages/trigger-sdk/package.json (1)
53-53: Consider loosening the sem-ver pin on@opentelemetry/semantic-conventions.Pinning to the exact version
1.36.0means we won’t receive patch-level fixes (including potential security fixes) unless we bump manually. Unless you have a reproducibility requirement, prefer a caret range:- "@opentelemetry/semantic-conventions": "1.36.0", + "@opentelemetry/semantic-conventions": "^1.36.0",This stays within the same major line while allowing non-breaking updates.
apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam.spans.$spanParam/route.tsx (1)
741-746: UI addition LGTM – one UX micro-nit.Displaying the external trace ID is valuable. The ID can be long; consider wrapping with
break-all(as done for other long strings above) to avoid horizontal scroll on narrow viewports:- <Property.Value>{run.externalTraceId}</Property.Value> + <Property.Value className="break-all"> + {run.externalTraceId} + </Property.Value>apps/webapp/app/routes/api.v1.tasks.$taskId.trigger.ts (1)
113-119: Consider consolidating debug logs.This debug log duplicates most of the information from the log immediately above (lines 102-111). Consider either:
- Removing this duplicate log
- Consolidating both into a single log entry
- Adding a comment explaining why separate logging is needed for OpenTelemetry context
- logger.debug("Triggering task", { - taskId: params.taskId, - idempotencyKey, - idempotencyKeyTTL, - triggerVersion, - headers, - options: body.options, - isFromWorker, - traceContext, - }); - - logger.debug("[otelContext]", { - taskId: params.taskId, - headers, - options: body.options, - isFromWorker, - traceContext, - }); + logger.debug("Triggering task", { + taskId: params.taskId, + idempotencyKey, + idempotencyKeyTTL, + triggerVersion, + headers, + options: body.options, + isFromWorker, + traceContext, + otelContext: true, // Flag for OpenTelemetry context debugging + });packages/core/src/v3/traceContext/manager.ts (1)
20-20: Remove unnecessary null coalescing operatorSince
traceContextis initialized to{}in the constructor, the null coalescing operator is redundant.- return propagation.extract(context.active(), this.traceContext ?? {}); + return propagation.extract(context.active(), this.traceContext);
ericallam
force-pushed
the
opentelemetry-deps-update
branch
from
de3d6ac to
54a3f2a
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (7)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlreferences/d3-chat/package.jsonis excluded by!references/**references/d3-chat/src/app/api/demo-batch-trigger/route.tsis excluded by!references/**references/d3-chat/src/app/api/demo-call-from-trigger/route.tsis excluded by!references/**references/d3-chat/src/app/api/demo-trigger/route.tsis excluded by!references/**references/d3-chat/src/instrumentation.tsis excluded by!references/**references/d3-chat/src/trigger/chat.tsis excluded by!references/**
📒 Files selected for processing (41)
.changeset/five-nails-whisper.md(1 hunks)apps/webapp/app/presenters/v3/SpanPresenter.server.ts(5 hunks)apps/webapp/app/routes/api.v1.tasks.$taskId.trigger.ts(2 hunks)apps/webapp/app/routes/api.v2.tasks.batch.ts(1 hunks)apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam.spans.$spanParam/route.tsx(1 hunks)apps/webapp/app/runEngine/services/batchTrigger.server.ts(1 hunks)apps/webapp/app/runEngine/services/triggerTask.server.ts(3 hunks)apps/webapp/app/runEngine/types.ts(2 hunks)apps/webapp/app/v3/environmentVariableRules.server.ts(0 hunks)apps/webapp/app/v3/environmentVariables/environmentVariablesRepository.server.ts(6 hunks)apps/webapp/app/v3/eventRepository.server.ts(8 hunks)apps/webapp/app/v3/services/triggerTask.server.ts(1 hunks)apps/webapp/test/environmentVariableRules.test.ts(1 hunks)internal-packages/run-engine/src/engine/index.ts(1 hunks)internal-packages/run-engine/src/engine/types.ts(1 hunks)packages/cli-v3/package.json(1 hunks)packages/cli-v3/src/cli/common.ts(2 hunks)packages/cli-v3/src/entryPoints/dev-run-worker.ts(9 hunks)packages/cli-v3/src/entryPoints/managed-run-worker.ts(9 hunks)packages/cli-v3/src/telemetry/tracing.ts(0 hunks)packages/cli-v3/src/utilities/session.ts(1 hunks)packages/core/package.json(1 hunks)packages/core/src/v3/apiClient/core.ts(1 hunks)packages/core/src/v3/index.ts(1 hunks)packages/core/src/v3/isomorphic/index.ts(1 hunks)packages/core/src/v3/isomorphic/traceContext.ts(1 hunks)packages/core/src/v3/otel/tracingSDK.ts(10 hunks)packages/core/src/v3/schemas/schemas.ts(1 hunks)packages/core/src/v3/taskContext/otelProcessors.ts(2 hunks)packages/core/src/v3/trace-context-api.ts(1 hunks)packages/core/src/v3/traceContext/api.ts(1 hunks)packages/core/src/v3/traceContext/manager.ts(1 hunks)packages/core/src/v3/traceContext/types.ts(1 hunks)packages/core/src/v3/tracer.ts(0 hunks)packages/core/src/v3/utils/globals.ts(2 hunks)packages/core/src/v3/workers/index.ts(1 hunks)packages/core/src/v3/workers/taskExecutor.ts(4 hunks)packages/core/test/taskExecutor.test.ts(1 hunks)packages/trigger-sdk/package.json(1 hunks)packages/trigger-sdk/src/v3/index.ts(1 hunks)packages/trigger-sdk/src/v3/otel.ts(1 hunks)
💤 Files with no reviewable changes (3)
- apps/webapp/app/v3/environmentVariableRules.server.ts
- packages/core/src/v3/tracer.ts
- packages/cli-v3/src/telemetry/tracing.ts
✅ Files skipped from review due to trivial changes (11)
- apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam.spans.$spanParam/route.tsx
- internal-packages/run-engine/src/engine/types.ts
- internal-packages/run-engine/src/engine/index.ts
- packages/trigger-sdk/src/v3/otel.ts
- packages/core/src/v3/index.ts
- apps/webapp/app/runEngine/services/batchTrigger.server.ts
- packages/core/src/v3/workers/index.ts
- apps/webapp/test/environmentVariableRules.test.ts
- packages/core/src/v3/schemas/schemas.ts
- packages/core/package.json
- packages/core/src/v3/workers/taskExecutor.ts
🚧 Files skipped from review as they are similar to previous changes (25)
- packages/trigger-sdk/package.json
- packages/core/src/v3/trace-context-api.ts
- apps/webapp/app/v3/services/triggerTask.server.ts
- apps/webapp/app/routes/api.v2.tasks.batch.ts
- packages/core/src/v3/utils/globals.ts
- apps/webapp/app/routes/api.v1.tasks.$taskId.trigger.ts
- packages/trigger-sdk/src/v3/index.ts
- packages/cli-v3/src/utilities/session.ts
- apps/webapp/app/runEngine/types.ts
- packages/core/src/v3/isomorphic/index.ts
- packages/core/src/v3/apiClient/core.ts
- packages/cli-v3/src/cli/common.ts
- packages/cli-v3/src/entryPoints/dev-run-worker.ts
- packages/cli-v3/src/entryPoints/managed-run-worker.ts
- packages/core/test/taskExecutor.test.ts
- apps/webapp/app/runEngine/services/triggerTask.server.ts
- apps/webapp/app/v3/environmentVariables/environmentVariablesRepository.server.ts
- packages/core/src/v3/traceContext/types.ts
- packages/core/src/v3/isomorphic/traceContext.ts
- packages/cli-v3/package.json
- packages/core/src/v3/traceContext/manager.ts
- packages/core/src/v3/traceContext/api.ts
- packages/core/src/v3/taskContext/otelProcessors.ts
- apps/webapp/app/v3/eventRepository.server.ts
- apps/webapp/app/presenters/v3/SpanPresenter.server.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Always prefer using isomorphic code like fetch, ReadableStream, etc. instead of Node.js specific code
For TypeScript, we usually use types over interfaces
Avoid enums
No default exports, use function declarations
Files:
packages/core/src/v3/otel/tracingSDK.ts
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
We use zod a lot in packages/core and in the webapp
Files:
packages/core/src/v3/otel/tracingSDK.ts
🧠 Learnings (14)
📓 Common learnings
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to trigger.config.ts : Global lifecycle hooks, telemetry, runtime, machine settings, log level, max duration, and build configuration must be set in `trigger.config.ts` as shown.
📚 Learning: applies to **/trigger/**/*.{ts,tsx,js,jsx} : always generate trigger.dev tasks using the `task` func...
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : ALWAYS generate Trigger.dev tasks using the `task` function from `@trigger.dev/sdk/v3` and export them as shown in the correct pattern.
Applied to files:
.changeset/five-nails-whisper.mdpackages/core/src/v3/otel/tracingSDK.ts
📚 Learning: applies to **/trigger/**/*.{ts,tsx,js,jsx} : you must use `@trigger.dev/sdk/v3` when writing trigger...
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : You MUST use `@trigger.dev/sdk/v3` when writing Trigger.dev tasks.
Applied to files:
.changeset/five-nails-whisper.mdpackages/core/src/v3/otel/tracingSDK.ts
📚 Learning: applies to **/trigger/**/*.{ts,tsx,js,jsx} : when implementing schema tasks, use `schematask` from `...
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : When implementing schema tasks, use `schemaTask` from `@trigger.dev/sdk/v3` and validate payloads as shown.
Applied to files:
.changeset/five-nails-whisper.md
📚 Learning: applies to **/trigger/**/*.{ts,tsx,js,jsx} : when implementing scheduled (cron) tasks, use `schedule...
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : When implementing scheduled (cron) tasks, use `schedules.task` from `@trigger.dev/sdk/v3` and follow the shown patterns.
Applied to files:
.changeset/five-nails-whisper.md
📚 Learning: before generating any code for trigger.dev tasks, verify: (1) are you importing from `@trigger.dev/s...
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Before generating any code for Trigger.dev tasks, verify: (1) Are you importing from `@trigger.dev/sdk/v3`? (2) Have you exported every task? (3) Have you generated any deprecated code patterns?
Applied to files:
.changeset/five-nails-whisper.md
📚 Learning: applies to trigger.config.ts : global lifecycle hooks, telemetry, runtime, machine settings, log lev...
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to trigger.config.ts : Global lifecycle hooks, telemetry, runtime, machine settings, log level, max duration, and build configuration must be set in `trigger.config.ts` as shown.
Applied to files:
.changeset/five-nails-whisper.mdpackages/core/src/v3/otel/tracingSDK.ts
📚 Learning: applies to trigger.config.ts : build extensions such as `additionalfiles`, `additionalpackages`, `ap...
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to trigger.config.ts : Build extensions such as `additionalFiles`, `additionalPackages`, `aptGet`, `emitDecoratorMetadata`, `prismaExtension`, `syncEnvVars`, `puppeteer`, `ffmpeg`, and `esbuildPlugin` must be configured in `trigger.config.ts` as shown.
Applied to files:
.changeset/five-nails-whisper.mdpackages/core/src/v3/otel/tracingSDK.ts
📚 Learning: applies to **/trigger/**/*.{ts,tsx,js,jsx} : when triggering a task from inside another task, use `y...
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : When triggering a task from inside another task, use `yourTask.trigger`, `yourTask.batchTrigger`, `yourTask.triggerAndWait`, `yourTask.batchTriggerAndWait`, `batch.triggerAndWait`, `batch.triggerByTask`, or `batch.triggerByTaskAndWait` as shown.
Applied to files:
.changeset/five-nails-whisper.md
📚 Learning: applies to **/trigger/**/*.{ts,tsx,js,jsx} : when triggering a task from backend code, use `tasks.tr...
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : When triggering a task from backend code, use `tasks.trigger`, `tasks.batchTrigger`, or `tasks.triggerAndPoll` as shown in the examples.
Applied to files:
.changeset/five-nails-whisper.md
📚 Learning: applies to **/trigger/**/*.{ts,tsx,js,jsx} : you must `export` every task, including subtasks, in tr...
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : You MUST `export` every task, including subtasks, in Trigger.dev task files.
Applied to files:
.changeset/five-nails-whisper.md
📚 Learning: applies to apps/webapp/**/*.{ts,tsx} : when importing from `@trigger.dev/core` in the webapp, never ...
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-07-18T17:49:47.180Z
Learning: Applies to apps/webapp/**/*.{ts,tsx} : When importing from `@trigger.dev/core` in the webapp, never import from the root `@trigger.dev/core` path; always use one of the subpath exports as defined in the package's package.json.
Applied to files:
.changeset/five-nails-whisper.md
📚 Learning: applies to **/trigger/**/*.{ts,tsx,js,jsx} : when logging in tasks, use the `logger` api (`logger.de...
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : When logging in tasks, use the `logger` API (`logger.debug`, `logger.log`, `logger.info`, `logger.warn`, `logger.error`) as shown.
Applied to files:
packages/core/src/v3/otel/tracingSDK.ts
📚 Learning: applies to trigger.config.ts : the `trigger.config.ts` file must use `defineconfig` from `@trigger.d...
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to trigger.config.ts : The `trigger.config.ts` file must use `defineConfig` from `@trigger.dev/sdk/v3` and follow the configuration structure shown.
Applied to files:
packages/core/src/v3/otel/tracingSDK.ts
🧬 Code Graph Analysis (1)
packages/core/src/v3/otel/tracingSDK.ts (5)
packages/core/src/v3/utils/getEnv.ts (1)
getEnvVar(11-13)packages/core/src/v3/semanticInternalAttributes.ts (1)
SemanticInternalAttributes(1-64)packages/core/src/v3/task-context-api.ts (1)
taskContext(5-5)packages/core/src/v3/taskContext/otelProcessors.ts (1)
TaskContextSpanProcessor(8-46)packages/core/src/v3/trace-context-api.ts (1)
traceContext(5-5)
⏰ 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). (25)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 10)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (10, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 10)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (9, 10)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 10)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 10)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (9)
packages/core/src/v3/otel/tracingSDK.ts (6)
103-123: Span processor array approach improves maintainabilityThe refactor to collect span processors in an array before passing to
NodeTracerProviderconstructor is cleaner than the previous approach of adding processors post-creation. The configuration logic for batch vs simple processors is consistent.Also applies to: 129-150
125-126: External trace context integration implemented correctlyThe integration of external trace context via
traceContext.getExternalTraceContext()and its propagation to wrapper classes is well-designed. The fallback to generatedexternalTraceIdwhen no external context exists maintains backward compatibility.Also applies to: 132-133, 147-148, 194-198, 211-215
152-164: Trace provider instantiation refactor improves clarityMoving span processor configuration to the constructor via the
spanProcessorsarray eliminates the need for post-creation processor registration, making the initialization flow more explicit and easier to follow.
220-228: Log provider instantiation follows same patternThe log provider refactor mirrors the span provider approach consistently, using constructor-based processor injection rather than post-creation addition.
300-322: Attempt span logic is complex but correctThe special handling for attempt spans to update parent span context with external trace context details (including tracestate) is sophisticated. The logic correctly preserves trace flags and handles optional trace state.
284-288: External span wrapper type safety needs improvementThe
externalTraceContextparameter allowsundefinedbut the logic assumes certain properties exist. The type definition should be more specific to prevent runtime errors.Improve type safety for external trace context:
- private externalTraceContext: - | { traceId: string; spanId: string; tracestate?: string } - | undefined + private externalTraceContext: { + traceId: string; + spanId: string; + tracestate?: string; + } | nullAdd proper null checks in the
transformSpanmethod:- if (isAttemptSpan && this.externalTraceContext) { + if (isAttemptSpan && this.externalTraceContext !== null) {Also applies to: 296-322
⛔ Skipped due to learnings
Learnt from: nicktrn PR: triggerdotdev/trigger.dev#1389 File: apps/coordinator/src/index.ts:403-0 Timestamp: 2024-10-08T15:31:34.807Z Learning: In `apps/coordinator/src/index.ts`, moving the `exitRun` and `crashRun` functions outside the `onConnection` method is not feasible because of TypeScript type issues with `socket`. Keeping these functions inside `onConnection` is preferred.Learnt from: nicktrn PR: triggerdotdev/trigger.dev#1389 File: apps/coordinator/src/index.ts:403-0 Timestamp: 2024-10-08T13:39:26.553Z Learning: In `apps/coordinator/src/index.ts`, moving the `exitRun` and `crashRun` functions outside the `onConnection` method is not feasible because of TypeScript type issues with `socket`. Keeping these functions inside `onConnection` is preferred.Learnt from: nicktrn PR: triggerdotdev/trigger.dev#1389 File: apps/coordinator/src/index.ts:403-0 Timestamp: 2024-10-16T01:08:01.788Z Learning: In `apps/coordinator/src/index.ts`, moving the `exitRun` and `crashRun` functions outside the `onConnection` method is not feasible because of TypeScript type issues with `socket`. Keeping these functions inside `onConnection` is preferred.Learnt from: CR PR: triggerdotdev/trigger.dev#0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-07-18T17:49:24.468Z Learning: Applies to **/tsconfig.json : Use strict modeLearnt from: CR PR: triggerdotdev/trigger.dev#0 File: .cursor/rules/writing-tasks.mdc:0-0 Timestamp: 2025-07-18T17:50:25.014Z Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : NEVER generate deprecated code patterns using `client.defineJob` and related deprecated APIs, as shown in the prohibited code block..changeset/five-nails-whisper.md (3)
7-22: Package version table is comprehensive and well-formattedThe tabular format clearly shows the version changes across all OpenTelemetry packages. The distinction between major updates, minor updates, new dependencies, and removals is helpful for understanding the scope of changes.
27-57: Configuration example demonstrates integration correctlyThe
defineConfigexample shows proper setup of external OTLP exporters with authentication headers. The Axiom integration example is practical and demonstrates real-world usage.
5-6: Documentation title and summary are clearThe title "External Trace Correlation & OpenTelemetry Package Updates" accurately describes the changes, and the introduction properly sets expectations for what users can expect.
Also applies to: 23-25
The opentelemetry dependencies in the packages 'cli-v3' and 'core' were outdated and needed updating to the latest versions. This ensures compatibility with the latest features and fixes provided by opentelemetry. - Updated '@opentelemetry/api-logs' to version '0.203.0'. - Updated '@opentelemetry/core' to version '2.0.1'. - Updated '@opentelemetry/exporter-trace-otlp-http' to version '0.203.0'. - Updated other relevant opentelemetry packages to the latest stable versions available.
ericallam
force-pushed
the
opentelemetry-deps-update
branch
from
328996a to
a720f37
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
External Trace Correlation & OpenTelemetry Package Updates
Overview
This PR introduces external trace correlation capabilities to Trigger.dev, enabling seamless distributed tracing across system boundaries. Additionally, we've updated all OpenTelemetry packages to their latest stable versions, bringing significant performance improvements and new features.
Key Changes
1. External Trace Correlation
We've implemented a comprehensive trace context management system that allows Trigger.dev to:
Architecture
The implementation introduces a new
TraceContextManagerthat:2. OpenTelemetry Package Updates
@opentelemetry/api@opentelemetry/api-logs@opentelemetry/core@opentelemetry/exporter-logs-otlp-http@opentelemetry/exporter-trace-otlp-http@opentelemetry/instrumentation@opentelemetry/instrumentation-fetch@opentelemetry/resources@opentelemetry/sdk-logs@opentelemetry/sdk-node@opentelemetry/sdk-trace-base@opentelemetry/sdk-trace-node@opentelemetry/semantic-conventions3. Environment Variable Improvements
TRIGGER_prefix for all Trigger.dev-specific OpenTelemetry environment variablesTechnical Implementation Details
Trace Context Propagation
We will now correlate your external traces with trigger.dev traces and logs when using our external exporters:
You can also now propagate your external trace context when calling back into your own backend infra from inside a trigger.dev task:
Key Files Modified
Core Changes:
packages/core/src/v3/traceContext/*: New trace context management systempackages/core/src/v3/otel/tracingSDK.ts: Updated SDK initialization with new packagesapps/webapp/app/runEngine/services/triggerTask.server.ts: External trace propagation logicAPI Endpoints:
apps/webapp/app/routes/api.v1.tasks.$taskId.trigger.ts: Accept external trace headersapps/webapp/app/routes/api.v2.tasks.batch.ts: Batch trigger trace propagationBenefits
Migration Notes
Testing
Future Considerations
This foundation enables future enhancements such as: