Add external log exporters and fix missing external trace exporters in deployed tasks #2038
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: 83dd40e 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 |
## Walkthrough
This update introduces support for external log exporters in the telemetry configuration, allowing logs from deployed tasks to be exported to external systems. It also adds a runtime toggle to disable console interception. Changes span configuration types, tracing SDK initialization, console interception logic, and example usage in a reference project, ensuring both traces and logs can be externally exported for improved observability.
## Changes
| File(s) | Change Summary |
|-------------------------------------------------------------------------|---------------------------------------------------------------------------------------------------------------------|
| packages/core/src/v3/config.ts | Added `logExporters` property to the `telemetry` section of `TriggerConfig` type for OpenTelemetry log exporters; added `disableConsoleInterceptor` boolean property to `TriggerConfig`. |
| packages/core/src/v3/otel/tracingSDK.ts | Extended `TracingSDKConfig` with `logExporters`; added `ExternalLogRecordExporterWrapper` class; updated constructor to register external log exporters with conditional batch processing and trace ID proxying. |
| packages/core/src/v3/consoleInterceptor.ts | Added `interceptingDisabled` boolean parameter to `ConsoleInterceptor` constructor; modified `intercept` method to bypass interception when disabled. |
| packages/cli-v3/src/entryPoints/dev-run-worker.ts<br>packages/cli-v3/src/entryPoints/managed-run-worker.ts | Updated tracing SDK initialization to accept and pass `logExporters` and `exporters` from config; passed `disableConsoleInterceptor` flag to `ConsoleInterceptor`. |
| references/d3-chat/package.json | Added dependencies: `@opentelemetry/exporter-logs-otlp-http@0.52.1` and `@opentelemetry/exporter-trace-otlp-http@0.52.1`. |
| references/d3-chat/trigger.config.ts | Configured telemetry with OTLP log and trace exporters sending data to an external Axiom API endpoint with authorization headers. |
| references/d3-chat/src/trigger/chat.ts | Added an info log statement at the start of the `interruptibleChat` task's `run` function. |
| .changeset/cuddly-boats-press.md | Added a changeset entry summarizing the introduction of external log exporters and console interceptor toggle for improved observability. |
## Sequence Diagram(s)
```mermaid
sequenceDiagram
participant Task
participant TracingSDK
participant LoggerProvider
participant ExternalLogExporter
Task->>TracingSDK: Initialize with config (including logExporters)
TracingSDK->>LoggerProvider: Register log exporters from config
Task->>LoggerProvider: Emit log record
LoggerProvider->>ExternalLogExporter: Export log recordPossibly related PRs
Suggested reviewers
Poem
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
references/d3-chat/src/trigger/chat.ts (1)
230-230: Enhance startup log with additional context
Logging wheninterruptible-chatstarts is useful; consider including extra metadata (e.g., run ID or correlation ID) to improve traceability across logs and spans.packages/core/src/v3/config.ts (1)
83-90: AddlogExportersto telemetry config schema
The newlogExportersoption is well-documented in the JSDoc. Remember to update your public configuration documentation to include usage examples for this property.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
.changeset/cuddly-boats-press.md(1 hunks)packages/cli-v3/src/entryPoints/dev-run-worker.ts(1 hunks)packages/cli-v3/src/entryPoints/managed-run-worker.ts(1 hunks)packages/core/src/v3/config.ts(2 hunks)packages/core/src/v3/otel/tracingSDK.ts(3 hunks)references/d3-chat/package.json(1 hunks)references/d3-chat/src/trigger/chat.ts(1 hunks)references/d3-chat/trigger.config.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
references/d3-chat/src/trigger/chat.ts (1)
packages/core/src/v3/tracer.ts (1)
logger(56-64)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: typecheck / typecheck
- GitHub Check: units / 🧪 Unit Tests
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (9)
references/d3-chat/package.json (1)
30-30: Add OTLP HTTP log exporter dependency
Including@opentelemetry/exporter-logs-otlp-httpenables the OTLP HTTP log exporter in the reference project. Ensure this version is compatible with other OpenTelemetry packages used across the repo..changeset/cuddly-boats-press.md (1)
5-5: Changelog entry accurately reflects feature changes
The description clearly states the addition of external log exporters and the fix for missing external trace exporters, matching the PR objectives.packages/cli-v3/src/entryPoints/dev-run-worker.ts (1)
167-167: IncludelogExportersin TracingSDK initialization
Correctly wires the newlogExportersconfiguration into theTracingSDK, enabling external log export in the development worker.packages/core/src/v3/config.ts (1)
14-14: ImportLogRecordExporterfor new config property
This import is required to type thetelemetry.logExportersarray. Confirm that the@opentelemetry/sdk-logsversion used here matches your other OTel dependencies.references/d3-chat/trigger.config.ts (1)
4-4: Good addition of the OpenTelemetry HTTP logs exporter.The import of
OTLPLogExporterfrom the OpenTelemetry HTTP logs exporter package is appropriate for the new telemetry configuration being added.packages/cli-v3/src/entryPoints/managed-run-worker.ts (1)
166-167: Properly added support for external exporters in TracingSDK initialization.Good implementation that passes both trace exporters and log exporters from the configuration to the TracingSDK. The code correctly handles undefined values by defaulting to empty arrays, which prevents potential runtime errors.
This change directly addresses the PR objectives to add external log exporters and fix missing external trace exporters in deployed tasks.
packages/core/src/v3/otel/tracingSDK.ts (3)
18-18: Appropriately imported LogRecordExporter.The explicit import of
LogRecordExporterfrom@opentelemetry/sdk-logsis needed for the type definition in the configuration.
91-91: Good extension of TracingSDKConfig.The TracingSDKConfig type has been properly extended to include the optional logExporters array, maintaining consistency with the existing exporters property pattern.
215-226: Well-implemented external log exporter processing.The implementation for processing external log exporters follows the same pattern used for trace exporters, ensuring consistency in the codebase. It correctly:
- Iterates through all provided log exporters
- Adds appropriate processors based on environment configuration
- Uses the same environment variables for batch processing configuration as the main exporter
This change properly fulfills the PR objective to support external log exporters in deployed tasks.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
references/d3-chat/trigger.config.ts (1)
15-16: Document AXIOM_TOKEN in project documentationThe telemetry setup looks solid, but there's a need to document the required
AXIOM_TOKENenvironment variable in the repo's documentation.#!/bin/bash # Check if the AXIOM_TOKEN environment variable is documented somewhere rg -i "AXIOM_TOKEN" --type mdAlso applies to: 24-25
🧹 Nitpick comments (1)
references/d3-chat/trigger.config.ts (1)
10-29: Well-structured telemetry configurationThe telemetry configuration is properly implemented with both log and trace exporters pointing to Axiom's API. The configuration correctly:
- Uses environment variables for sensitive data (
AXIOM_TOKEN)- Specifies the appropriate dataset identifier
- Sets the correct Axiom API endpoints
Consider extracting the dataset name to an environment variable for better configurability across environments.
- "X-Axiom-Dataset": "d3-chat-tester", + "X-Axiom-Dataset": process.env.AXIOM_DATASET || "d3-chat-tester",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
.changeset/cuddly-boats-press.md(1 hunks)packages/cli-v3/src/entryPoints/dev-run-worker.ts(2 hunks)packages/cli-v3/src/entryPoints/managed-run-worker.ts(2 hunks)packages/core/src/v3/config.ts(3 hunks)packages/core/src/v3/consoleInterceptor.ts(1 hunks)packages/core/src/v3/otel/tracingSDK.ts(6 hunks)references/d3-chat/package.json(1 hunks)references/d3-chat/src/trigger/chat.ts(1 hunks)references/d3-chat/trigger.config.ts(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- references/d3-chat/src/trigger/chat.ts
- references/d3-chat/package.json
🚧 Files skipped from review as they are similar to previous changes (5)
- .changeset/cuddly-boats-press.md
- packages/core/src/v3/config.ts
- packages/cli-v3/src/entryPoints/managed-run-worker.ts
- packages/cli-v3/src/entryPoints/dev-run-worker.ts
- packages/core/src/v3/otel/tracingSDK.ts
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: typecheck / typecheck
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: units / 🧪 Unit Tests
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (3)
packages/core/src/v3/consoleInterceptor.ts (2)
13-14: Well-implemented console interceptor toggleGood addition of the
interceptingDisabledparameter to the constructor. This enables the runtime toggle mentioned in the PR objectives.
20-22: Clean implementation of early return patternThe early return pattern when intercepting is disabled is a clean and efficient approach. This ensures that when interception is disabled, we simply pass through to the callback without any overhead.
references/d3-chat/trigger.config.ts (1)
4-5: Good OpenTelemetry exporter importsThese imports correctly add the necessary dependencies for both log and trace exporters, aligning with the PR objective to support external telemetry.
…ith the external traceID as well
matt-aitken
approved these changes
May 13, 2025
nicktrn
approved these changes
May 13, 2025
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.
This PR adds support for 3rd party log exporters (alongside the existing 3rd party trace exporters). This also fixes an issue where 3rd party trace exporters were not being registered in deployed tasks, and we've also fixed traceID generation in case 3rd parties expected the traceIDs to be in the exact format of the otel spec.
Summary by CodeRabbit
New Features
Bug Fixes
Chores