Add diagnostics_channel TracingChannel support#3624
Open
logaretm wants to merge 7 commits intobrianc:masterfrom
Open
Add diagnostics_channel TracingChannel support#3624logaretm wants to merge 7 commits intobrianc:masterfrom
logaretm wants to merge 7 commits intobrianc:masterfrom
Conversation
Enables instrumentation libraries (OpenTelemetry, etc.) to subscribe to structured events without monkey-patching. Uses TracingChannel for async context propagation and plain channels for simple events. Channels: - pg:query (TracingChannel) — query lifecycle with result enrichment - pg:connection (TracingChannel) — client connect lifecycle - pg:pool:connect (TracingChannel) — pool checkout lifecycle - pg:pool:release (plain) — client released back to pool - pg:pool:remove (plain) — client removed from pool All instrumentation is guarded by hasSubscribers for zero overhead when unused. Gracefully degrades to no-ops on Node < 19.9 or non-Node environments. Closes brianc#3619 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
TracingChannel is not available on Node 18 LTS. Skip the tracing-dependent tests gracefully instead of failing. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds diagnostics_channel TracingChannel instrumentation for pg and pg-pool so observability tooling can subscribe to structured lifecycle events without monkey-patching, with zero overhead when no subscribers exist and graceful degradation when unsupported.
Changes:
- Introduces diagnostics channel loaders for
pgandpg-pool(TracingChannels + plain channels where appropriate) - Instruments
pgclient connect + query lifecycle andpg-poolconnect/release/remove events - Adds new unit tests validating published diagnostics events and context enrichment
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/pg/lib/diagnostics.js | Adds lazy/guarded creation of pg:query and pg:connection tracing channels |
| packages/pg/lib/client.js | Instruments connect() and query enqueue/completion with diagnostics tracing |
| packages/pg/test/unit/client/diagnostics-tests.js | Adds unit tests for pg query + connection diagnostics events |
| packages/pg-pool/diagnostics.js | Adds lazy/guarded creation of pool connect tracing channel and release/remove channels |
| packages/pg-pool/index.js | Publishes pool remove/release events and traces pool connect lifecycle |
| packages/pg-pool/test/diagnostics.js | Adds unit tests for pg-pool diagnostics channels |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+21
to
+36
| const subs = { | ||
| start: (ctx) => events.push({ type: 'start', context: ctx }), | ||
| end: () => {}, | ||
| asyncStart: () => {}, | ||
| asyncEnd: (ctx) => { | ||
| events.push({ type: 'asyncEnd', context: ctx }) | ||
|
|
||
| // asyncEnd fires after the callback, so check everything here | ||
| assert.equal(events.length, 2) | ||
| assert.equal(events[0].type, 'start') | ||
| assert.equal(events[0].context.query.text, 'SELECT 1') | ||
| assert.equal(events[0].context.client.database, client.database) | ||
|
|
||
| assert.equal(events[1].type, 'asyncEnd') | ||
| assert.equal(events[1].context.result.command, 'SELECT') | ||
| assert.equal(events[1].context.result.rowCount, 1) |
There was a problem hiding this comment.
Unsubscribing is done only at the end of the success path inside callbacks (e.g., asyncEnd). If an assertion throws before unsubscribe (or the test times out), the subscription can leak into later tests and cause cross-test interference. Prefer ensuring channel.unsubscribe(subs) runs via a try/finally around assertions or a suite-level teardown/afterEach hook.
Comment on lines
+5
to
+26
| let poolConnectChannel = noopChannel | ||
| let poolReleaseChannel = noopChannel | ||
| let poolRemoveChannel = noopChannel | ||
|
|
||
| try { | ||
| let dc | ||
| if (typeof process.getBuiltInModule === 'function') { | ||
| dc = process.getBuiltInModule('diagnostics_channel') | ||
| } else { | ||
| dc = require('diagnostics_channel') | ||
| } | ||
| if (typeof dc.tracingChannel === 'function') { | ||
| poolConnectChannel = dc.tracingChannel('pg:pool:connect') | ||
| } | ||
| if (typeof dc.channel === 'function') { | ||
| poolReleaseChannel = dc.channel('pg:pool:release') | ||
| poolRemoveChannel = dc.channel('pg:pool:remove') | ||
| } | ||
| } catch (e) { | ||
| // diagnostics_channel not available (non-Node environment) | ||
| } | ||
|
|
There was a problem hiding this comment.
The diagnostics bootstrap logic here is very similar to packages/pg/lib/diagnostics.js (built-in-module fallback, feature detection, noop channels). To reduce duplication and avoid the two implementations drifting over time, consider extracting a small shared helper (even internal) for: loading diagnostics_channel safely, creating tracing channels, and creating plain channels.
Suggested change
| let poolConnectChannel = noopChannel | |
| let poolReleaseChannel = noopChannel | |
| let poolRemoveChannel = noopChannel | |
| try { | |
| let dc | |
| if (typeof process.getBuiltInModule === 'function') { | |
| dc = process.getBuiltInModule('diagnostics_channel') | |
| } else { | |
| dc = require('diagnostics_channel') | |
| } | |
| if (typeof dc.tracingChannel === 'function') { | |
| poolConnectChannel = dc.tracingChannel('pg:pool:connect') | |
| } | |
| if (typeof dc.channel === 'function') { | |
| poolReleaseChannel = dc.channel('pg:pool:release') | |
| poolRemoveChannel = dc.channel('pg:pool:remove') | |
| } | |
| } catch (e) { | |
| // diagnostics_channel not available (non-Node environment) | |
| } | |
| function loadDiagnosticsChannel() { | |
| try { | |
| if (typeof process.getBuiltInModule === 'function') { | |
| return process.getBuiltInModule('diagnostics_channel') | |
| } | |
| return require('diagnostics_channel') | |
| } catch (e) { | |
| // diagnostics_channel not available (non-Node environment) | |
| return null | |
| } | |
| } | |
| function createTracingChannel(dc, name) { | |
| if (dc && typeof dc.tracingChannel === 'function') { | |
| return dc.tracingChannel(name) | |
| } | |
| return noopChannel | |
| } | |
| function createChannel(dc, name) { | |
| if (dc && typeof dc.channel === 'function') { | |
| return dc.channel(name) | |
| } | |
| return noopChannel | |
| } | |
| const diagnosticsChannel = loadDiagnosticsChannel() | |
| const poolConnectChannel = createTracingChannel(diagnosticsChannel, 'pg:pool:connect') | |
| const poolReleaseChannel = createChannel(diagnosticsChannel, 'pg:pool:release') | |
| const poolRemoveChannel = createChannel(diagnosticsChannel, 'pg:pool:remove') |
This was referenced Mar 12, 2026
Node 18 backported TracingChannel but without the aggregated `hasSubscribers` getter (it returns `undefined` instead of a boolean). Raw truthiness checks treat `undefined` as "no subscribers" which silently disables tracing on Node 18. Replace all `channel.hasSubscribers` guards with `shouldTrace(channel)` which checks `hasSubscribers !== false` — treating `undefined` (Node 18) as "might have subscribers, trace unconditionally" and `false` (Node 20+) as "definitely no subscribers, skip". Also removes the now-unnecessary test skip logic since TracingChannel does exist on Node 18. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Node 18 backported TracingChannel but with a buggy implementation — unsubscribing and resubscribing to the same channel crashes internally (`_subscribers` becomes undefined). Node 16 has no TracingChannel at all. Gate tests on `hasStableTracingChannel` which checks both that `dc.tracingChannel` exists AND that the aggregated `hasSubscribers` getter returns a boolean (only true on Node 19.9+/20.5+). TracingChannel tests: skipped on Node 16/18, run on Node 20+ Plain channel tests (release/remove): run on all versions Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…omise type tracePromise wraps the result in a native Promise, which breaks clients configured with a custom Promise implementation (e.g. bluebird). Switch to traceCallback inside the user's this._Promise constructor so the returned promise type is always correct. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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.
This PR introduces tracing channels support to
pg-poolandpgquerying methods as discussed in #3619I haven't yet run a benchmark, but without active tracing or subscribers the overhead is non-existent so existing users shouldn't be affected. The overhead would come from the consumers of the diag channels published here which is done anyways via monkey patching.
I planned a draft document and had claude do the implementation, I adjust the approach a few times to ensure we don't introduce a lot of code and redundancies.
Summary
diagnostics_channelTracingChannelsupport topgandpg-pool, enabling instrumentation libraries (OpenTelemetry, etc.) to subscribe to structured events without monkey-patchingpg:query,pg:connection(TracingChannels with full async lifecycle),pg:pool:connect(TracingChannel),pg:pool:release, andpg:pool:remove(plain channels)hasSubscribers— zero overhead when no subscribers are attachedCloses #3619
🤖 Generated with Claude Code