fix(client): attach logs to OTel spans in RemoteExecutor - #28792
Closed
dahiya001rohit wants to merge 4 commits into
Closed
fix(client): attach logs to OTel spans in RemoteExecutor#28792dahiya001rohit wants to merge 4 commits into
dahiya001rohit wants to merge 4 commits into
Conversation
- Update `TracingHelper` interface to accept logs in `dispatchEngineSpans`. - Update `ActiveTracingHelper` to attach logs as events to spans. - Pass logs from `RemoteExecutor` to `dispatchEngineSpans`. - Add unit test for `ActiveTracingHelper`.
|
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug where logs were not being attached to OpenTelemetry spans in the RemoteExecutor, bringing its behavior in line with the standard Client Engine. The fix updates the TracingHelper interface to accept logs as an optional parameter and modifies the implementation to attach these logs as events to the appropriate spans.
- Updated
TracingHelper.dispatchEngineSpansinterface to accept an optionallogsparameter - Modified
ActiveTracingHelperto process and attach logs as events to their corresponding spans - Updated
RemoteExecutorto pass logs when dispatching engine spans - Added unit test to verify logs are correctly attached to spans
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/internals/src/tracing/types.ts | Added optional logs parameter to dispatchEngineSpans method in TracingHelper interface |
| packages/instrumentation/src/tests/ActiveTracingHelper.test.ts | Added unit test verifying logs are attached to spans as events |
| packages/instrumentation/src/ActiveTracingHelper.ts | Implemented log grouping by span ID and event attachment logic in dispatchEngineSpans |
| packages/client/src/runtime/core/tracing/TracingHelper.ts | Updated DynamicTracingHelper to forward logs parameter to underlying tracing helper |
| packages/client/src/runtime/core/engines/client/RemoteExecutor.ts | Removed outdated FIXME comment and now passes extensions.logs to dispatchEngineSpans |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+9
to
+17
| const mockSpan = { | ||
| spanContext: () => ({ | ||
| traceId: 'trace-id', | ||
| spanId: 'span-id', | ||
| traceFlags: 0, | ||
| }), | ||
| addEvent: vi.fn(), | ||
| end: vi.fn(), | ||
| } |
There was a problem hiding this comment.
The mock span is missing the addLinks method which is called in the implementation when engineSpan.links exists (line 141 in ActiveTracingHelper.ts). While this test doesn't trigger that code path, consider adding addLinks: vi.fn() to the mock for completeness and to prevent potential issues if the test is extended.
tensordreams
pushed a commit
that referenced
this pull request
Jul 21, 2026
Log events reported by Accelerate were emitted before the spans they belong to were dispatched, so `$on` handlers observed no active span, unlike the equivalent `LocalExecutor` path. `TracingHelper.dispatchEngineSpans` now receives the log events recorded during the dispatched spans along with a callback to emit them. `ActiveTracingHelper` emits each event while its span is active and also records it on the span, mapping `error` events to `recordException`. Events whose span was filtered out by `ignoreSpanTypes`, or that have no matching span at all, are still emitted, and the disabled tracing helper emits all of them directly, so no log is lost when tracing is off. This is a breaking change to the `TracingHelper` interface exported from `@prisma/instrumentation-contract`: custom implementations must accept the two new parameters and pass every event to `emitLogEvent`. Closes: #28792 Closes: #28806
tensordreams
pushed a commit
that referenced
this pull request
Jul 21, 2026
Log events reported by Accelerate were emitted before the spans they belong to were dispatched, so `$on` handlers observed no active span, unlike the equivalent `LocalExecutor` path. `TracingHelper.dispatchEngineSpans` now receives the log events recorded during the dispatched spans along with a callback to emit them. `ActiveTracingHelper` emits each event while its span is active and also records it on the span, mapping `error` events to `recordException`. Events whose span was filtered out by `ignoreSpanTypes`, or that have no matching span at all, are still emitted, and the disabled tracing helper emits all of them directly, so no log is lost when tracing is off. This is a breaking change to the `TracingHelper` interface exported from `@prisma/instrumentation-contract`: custom implementations must accept the two new parameters and pass every event to `emitLogEvent`. Closes: #28792 Closes: #28806
tensordreams
pushed a commit
that referenced
this pull request
Jul 21, 2026
Log events reported by Accelerate were emitted before the spans they belong to were dispatched, so `$on` handlers observed no active span, unlike the equivalent `LocalExecutor` path. `TracingHelper.dispatchEngineSpans` now receives the log events recorded during the dispatched spans along with a callback to emit them. `ActiveTracingHelper` emits each event while its span is active and also records it on the span, mapping `error` events to `recordException`. Events whose span was filtered out by `ignoreSpanTypes`, or that have no matching span at all, are still emitted, and the disabled tracing helper emits all of them directly, so no log is lost when tracing is off. This is a breaking change to the `TracingHelper` interface exported from `@prisma/instrumentation-contract`: custom implementations must accept the two new parameters and pass every event to `emitLogEvent`. Closes: #28792 Closes: #28806
aqrln
added a commit
that referenced
this pull request
Jul 21, 2026
Logs should be emitted while within the corresponding span. This was not the case for `RemoteExecutor` and there was a FIXME comment about this bug: log events reported by Accelerate were emitted before the spans they belong to were dispatched, so `$on` handlers observed no active span, unlike the equivalent `LocalExecutor` path. ## Changes - `TracingHelper.dispatchEngineSpans` now receives the log events recorded during the dispatched spans, along with a callback to emit them. - `ActiveTracingHelper` emits each event while its span is active, and also records it on the span, mapping `error` events to `recordException`. - Events whose span was filtered out by `ignoreSpanTypes`, or that have no matching span at all, are still emitted, so no log is silently dropped. - `disabledTracingHelper` emits all events directly. Logging is configured independently of tracing, and tracing can be disabled mid-request via `PrismaInstrumentation.disable()` after the server was already asked for spans. ## Breaking change `TracingHelper` is exported from the published `@prisma/instrumentation-contract` package. Custom implementations must accept the two new parameters and pass every event to `emitLogEvent`, otherwise logs the user asked for are dropped. The interface doc comment and the package README now state this obligation. ## Tests - `packages/instrumentation/src/ActiveTracingHelper.test.ts` covers span-scoped emission, events recorded on their span, `recordException` for `error` events, and both leftover paths (ignored span, absent span). - `packages/client/tests/functional/tracing-event-context` covers plain queries, batch transactions, and interactive transactions end to end. CI exercises the `RemoteExecutor` path via the `client-query-compiler-accelerate` job. Verified by temporarily reverting the fix: three of the four functional tests fail against `provider=postgresql, qpe=remote` without it and pass with it. The five pre-existing tracing suites pass in both executor modes. Closes: #28792 Closes: #28806
lh0x00
pushed a commit
to lh0x00/prisma
that referenced
this pull request
Aug 9, 2026
Logs should be emitted while within the corresponding span. This was not the case for `RemoteExecutor` and there was a FIXME comment about this bug: log events reported by Accelerate were emitted before the spans they belong to were dispatched, so `$on` handlers observed no active span, unlike the equivalent `LocalExecutor` path. ## Changes - `TracingHelper.dispatchEngineSpans` now receives the log events recorded during the dispatched spans, along with a callback to emit them. - `ActiveTracingHelper` emits each event while its span is active, and also records it on the span, mapping `error` events to `recordException`. - Events whose span was filtered out by `ignoreSpanTypes`, or that have no matching span at all, are still emitted, so no log is silently dropped. - `disabledTracingHelper` emits all events directly. Logging is configured independently of tracing, and tracing can be disabled mid-request via `PrismaInstrumentation.disable()` after the server was already asked for spans. ## Breaking change `TracingHelper` is exported from the published `@prisma/instrumentation-contract` package. Custom implementations must accept the two new parameters and pass every event to `emitLogEvent`, otherwise logs the user asked for are dropped. The interface doc comment and the package README now state this obligation. ## Tests - `packages/instrumentation/src/ActiveTracingHelper.test.ts` covers span-scoped emission, events recorded on their span, `recordException` for `error` events, and both leftover paths (ignored span, absent span). - `packages/client/tests/functional/tracing-event-context` covers plain queries, batch transactions, and interactive transactions end to end. CI exercises the `RemoteExecutor` path via the `client-query-compiler-accelerate` job. Verified by temporarily reverting the fix: three of the four functional tests fail against `provider=postgresql, qpe=remote` without it and pass with it. The five pre-existing tracing suites pass in both executor modes. Closes: prisma#28792 Closes: prisma#28806
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
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.
Fixed a bug where logs were missing from OTel spans in the Remote Executor. I updated the tracing logic to properly attach these logs as events, making traces complete and consistent with the standard Client Engine. Included a unit test to verify the fix.
TracingHelperinterface to accept logs indispatchEngineSpans.ActiveTracingHelperto attach logs as events to spans.RemoteExecutortodispatchEngineSpans.ActiveTracingHelper.