fix(opentelemetry): make sdk-trace-node and sdk-trace-web required peer deps #5952
+5
−23
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
clayroach
requested review from
IMax153,
gcanti,
mattiamanzati,
mikearnaldi and
tim-smart
as code owners
🦋 Changeset detectedLatest commit: ec4fb58 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
clayroach
force-pushed
the
fix/opentelemetry-optional-peer-deps
branch
2 times, most recently
from
ac92bde to
9fe789e
Compare
Member
|
we should make them peer dependencies instead of optional, cc @tim-smart |
clayroach
force-pushed
the
fix/opentelemetry-optional-peer-deps
branch
from
9fe789e to
b713b28
Compare
clayroach
changed the title
Contributor
Author
|
Done |
clayroach
added a commit
to atrim-ai/instrumentation
that referenced
this pull request
Jan 10, 2026
Contributor
Author
|
@tim-smart looks like this gets addressed in 4.0.0? https://github.com/Effect-TS/effect-smol/blob/main/packages/effect/src/unstable/observability/OtlpTracer.ts. Can you confirm? |
clayroach
added a commit
to atrim-ai/instrumentation
that referenced
this pull request
Jan 18, 2026
…n tests (#149) * feat(effect): add YAML-driven auto-instrumentation via Supervisor API Implements Phase 1 of true auto-instrumentation for Effect-TS applications. Users can now automatically trace all Effect fibers without manual Effect.withSpan() calls - just provide the AutoTracingLive layer. Key features: - AutoTracingSupervisor using Effect's Supervisor API - Intelligent span naming with template variables ({fiber_id}, {function}, {module}, {line}) - Pattern-based naming rules matching files/functions - Include/exclude filter patterns - Performance controls (sampling_rate, min_duration, max_concurrent) - Opt-out utilities: withoutAutoTracing() and setSpanName() New export path: @atrim/instrument-node/effect/auto Closes #130 * feat(effect): add auto-tracing supervisor with OTLP export tests - Add withAutoTracing() function using Effect.supervised() for fiber tracing - Create main span for root effect + supervise all forked child fibers - Use defaultResource() for rich SDK/process resource attributes - Add integration tests for OTLP export to Atrim platform - Add examples for different exporter configurations - Export config types from @atrim/instrument-core Known limitations to address in follow-up: - Source location attributes (code.function, code.filepath) not reliably captured due to stack trace timing during fiber supervision - Tests currently use single-level span hierarchies * test(effect): add deep hierarchy and mixed tracing tests - Add 3-level deep fiber hierarchy test - Add parallel sibling spans test (parent with children and grandchildren) - Add mixed manual OTEL + auto-traced Effect spans test - Add Effect.withSpan + auto-tracing coexistence test These tests validate: - Deep span parent-child relationships - Parallel execution with proper span hierarchy - Interoperability with manual OTEL spans - Coexistence with Effects built-in withSpan Follow-up items identified during implementation: - Source location attributes (code.function, code.filepath, code.lineno) not reliably captured due to stack trace timing in fiber supervision - Consider alternative approaches for source capture - Resource attributes now include SDK info via defaultResource() * fix(effect): propagate root span context to forked child fibers - Add setRootSpan() method to AutoTracingSupervisor - Use root span as parent context for fibers without fiber parent - Ensures forked fibers have correct parentSpanId in traces * fix(effect): use ROOT_CONTEXT for proper trace ID propagation - Use OtelApi.ROOT_CONTEXT instead of context.active() as base - Effect fiber scheduler runs in different async context, losing OTel context - Explicitly set parent span on ROOT_CONTEXT ensures trace ID propagates correctly - Child spans now share same traceId as root and have correct parentSpanId * fix(effect): use single global TracerProvider for CombinedTracingLive This fixes the dual TracerProvider issue where HTTP spans (from @effect/opentelemetry's NodeSdk) and fiber spans (from our Supervisor) were going to different exporters. Root cause: NodeSdk.layer creates a scoped TracerProvider that doesn't set the global provider. The Supervisor uses OtelApi.trace.getTracer() which gets the global provider, resulting in spans being exported to different destinations. Solution: CombinedTracingLive now uses a single global BasicTracerProvider that both Effect's Tracer (via layerGlobal) and our Supervisor use. This ensures all spans go to the same exporter. Changes: - Add span_relationships config to instrumentation schema (parent-child, span-links, both) - Create new effect-tracing.ts with CombinedTracingLive and EffectTracingLive layers - Update Supervisor to support span links per OTel spec for forked fibers - Add integration test for HTTP server with combined tracing - Update example config with span_relationships documentation The span_relationships config allows users to choose: - parent-child: Traditional parent-child relationships (default) - span-links: Use span links per OTel spec for async operations - both: Create parent-child AND add span links Test confirms both HTTP and fiber spans now export to the same destination. * feat(examples): update effect-platform to use CombinedTracingLive This updates the effect-platform example to showcase CombinedTracingLive, which provides automatic HTTP + fiber-level tracing without manual Effect.withSpan() calls. Changes: - Replace manual Effect.withSpan() calls with CombinedTracingLive layer - Add forked background fibers to demonstrate automatic fiber tracing - Update instrumentation.yaml with auto_instrumentation config - Simplify business logic by removing manual span annotations - Update README to explain CombinedTracingLive benefits and usage The example now demonstrates: - Automatic HTTP request span creation - Automatic fiber span creation for forked background tasks - Parent-child relationships between HTTP and fiber spans - Zero-config tracing via YAML * add @opentelemetry/sdk-trace-web until Effect-TS/effect#5952 is resolved * feat(effect): add tracedFork for call-site capture in forked fibers Adds tracedFork() wrapper that captures the call site BEFORE entering Effect runtime, enabling meaningful span names instead of "effect.anonymous". Why this is needed: - Effect.fork() returns an Effect that creates a fiber when run - By the time the Supervisor's onStart hook is called, user code is off the stack - only Effect internals are visible - tracedFork() captures the call site while user code IS on stack Usage: import { tracedFork } from '@atrim/instrument-node/effect/auto' // Instead of: yield* Effect.fork(myTask) yield* tracedFork(myTask) // Span: "effect.myFile:42" Also adds: - tracedForkDaemon() for daemon fibers - patch-fork.ts documenting ESM limitation (runtime patching not possible) - CapturedSourceLocation FiberRef for supervisor to read - Module:line fallback naming for anonymous functions * feat(effect): add native source capture POC with Effect.withSpan support Implements POC for issue #145 using Effect's native source capture feature from the custom Effect build (@clayroach/effect with source-capture). Key changes: - Add SourceCaptureTracingLive layer that enables native source capture - Use layerWithoutOtelTracer to directly provide our tracer to @effect/opentelemetry - Set up global TracerProvider at module load time for correct layer ordering - Add flushAndShutdown() for explicit span export before process exit - Add loadFullConfigSync() for synchronous config loading The POC demonstrates: - Effect.fork() calls automatically get meaningful span names - Effect.withSpan() creates root spans that child fiber spans link to - No wrapper functions needed - just use standard Effect.fork() Next step: Auto-instrument without requiring explicit Effect.withSpan() * feat(effect): add rootSpan support for auto-instrumentation without withSpan - Add rootSpan getter/setter to SourceCaptureSupervisor - Enables fibers without ParentSpan in context to use a default root span - Foundation for OpSupervision-based operation tracing * feat(effect): add YAML-driven operation tracing with OpSupervision Adds automatic tracing for high-level Effect operations (Effect.all, Effect.forEach) using OpSupervision to hook into every operation. Operations are configured via instrumentation.yaml with zero-config defaults. Key features: - Auto-trace Effect.all, Effect.forEach without manual Effect.withSpan() calls - YAML-driven config: control which operations to trace, span names, metadata - Captures source location (file, line, column) at operation call site - Proper parent span context for unified traces - Zero-config defaults (traces all/forEach when config missing) Implementation: - OperationTracingConfigSchema: YAML schema for operation_tracing section - OperationTracingSupervisor: Reads OperationMeta from Effect trace field - Uses OpSupervision runtime flag to intercept all operations - Integrates with SourceCaptureTracingLive for unified trace context - Uses span.setAttribute() for proper type handling (integers vs strings) Example added at examples/effect-op-tracing/ demonstrating: - YAML configuration with console and OTLP exporters - Automatic span creation with proper parent relationships - Clean source location metadata (no confusing stack traces) Requires @clayroach/effect@3.19.14-source-capture.3 with OperationMeta support. Co-Authored-By: Claude Sonnet 4.5 (1M context) <noreply@anthropic.com> * changeset: add operation tracing feature * chore: version bump to 0.8.0 for operation tracing release * feat(effect): add UnifiedTracingSupervisor with correct fork span hierarchy Combines operation tracing and fiber tracing into a single supervisor that: - Creates effect.fork spans for fork operations - Creates effect.fiber spans for fiber executions - Correctly sets fork spans as parents of their resulting fiber spans - Uses source location correlation to match fork operations with child fibers - Integrates with Effect Platform HTTP tracing via layerGlobal Before: fork and fiber spans were siblings under HTTP span (wrong) After: fiber spans are children of their fork spans (correct) Trace hierarchy: http.server GET ├── effect.fork (index.ts:15) │ └── effect.fiber (index.ts:15) ← child of fork └── effect.all (index.ts:18) * feat(effect): enable OpSupervision at layer level Users no longer need withUnifiedTracing wrapper - just provide UnifiedTracingLive layer and get everything: - HTTP tracing (via Effect Platform) - Operation tracing (Effect.fork, Effect.all, Effect.forEach) - Fiber tracing - Correct fork span hierarchy Uses Effect.patchRuntimeFlags in Layer.effectDiscard to enable OpSupervision when the layer is built. This closes #146. Before: effect.pipe(withUnifiedTracing) // wrapper needed After: Layer.provide(UnifiedTracingLive) // just the layer! * fix(test): add tsx loader for @clayroach/effect TypeScript source files The pnpm override maps `effect` to `@clayroach/effect` which exports .ts source files directly. This causes "Unknown file extension .ts" errors when running tests with vitest. Fixed by adding NODE_OPTIONS='--import tsx' to test scripts in: - packages/node/package.json - packages/web/package.json Also added changeset for OpSupervision feature. * fix(test): re-enable skipped integration tests with proper fixes - Make TracerProvider initialization lazy in source-capture-supervisor.ts and unified-tracing-supervisor.ts to avoid module-level side effects that overwrote test-configured providers with default endpoints - Update test imports to use supervisor.js directly to avoid triggering the module-level side effect in source-capture-supervisor.js - Remove hardcoded endpoint from examples/effect-platform/instrumentation.yaml so tests can pass OTEL_EXPORTER_OTLP_ENDPOINT env var - Fix examples/effect-platform/index.ts pipe syntax for Layer.launch - Update effect-platform test assertions to match actual HTTP span attributes instead of outdated manual span names All 4 previously skipped test files now pass: - auto-tracing-export.integration.test.ts - mixed-tracing.integration.test.ts - effect-express.integration.test.ts - effect-platform.integration.test.ts * chore: update pnpm-lock.yaml * fix: update effect override to match lockfile (source-capture.8) * fix(test): increase forceFlush timing threshold to avoid CI flakiness --------- Co-authored-by: Claude Sonnet 4.5 (1M context) <noreply@anthropic.com>
Contributor
No, that is the effect native tracer which doesn't depend on the otel libraries. |
tim-smart
reviewed
Jan 19, 2026
tim-smart
reviewed
Jan 19, 2026
…er deps Remove optional flag from @opentelemetry/sdk-trace-node and @opentelemetry/sdk-trace-web peer dependencies. This fixes module resolution errors when importing from the main entry point, since ESM static imports require these packages to be installed.
Per review feedback - since the main entry point imports these packages statically, marking them as optional creates a false contract.
clayroach
force-pushed
the
fix/opentelemetry-optional-peer-deps
branch
from
0dafb14 to
d0f2e40
Compare
tim-smart
reviewed
Jan 21, 2026
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Problem
@effect/opentelemetrymarks@opentelemetry/sdk-trace-nodeand@opentelemetry/sdk-trace-webas optional peer dependencies. However, both are exported from the mainindex.ts, which creates static ESM imports. This causes module resolution errors when users don't have these packages installed:Solution
Make
@opentelemetry/sdk-trace-nodeand@opentelemetry/sdk-trace-webrequired peer dependencies by removing them frompeerDependenciesMeta.Changes
optional: truefor@opentelemetry/sdk-trace-nodeoptional: truefor@opentelemetry/sdk-trace-webTesting