Skip to content

Add diagnostics_channel TracingChannel support - #18

Closed
jeremyhayes wants to merge 7 commits into
eval/3650/basefrom
eval/3650/ccr-medium/round-3
Closed

jeremyhayes wants to merge 7 commits into
eval/3650/basefrom
eval/3650/ccr-medium/round-3

Conversation

@jeremyhayes

@jeremyhayes jeremyhayes commented Jul 21, 2026 •

Copy link
Copy Markdown

This PR introduces tracing channels support to pg-pool and pg querying methods as discussed in #3619

I 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 added the following channels:

Channel Type Package Description
pg:query TracingChannel pg Query lifecycle (start, end, error with async context)
pg:connection TracingChannel pg Client connection lifecycle
pg:pool:connect TracingChannel pg-pool Pool connection acquisition lifecycle
pg:pool:release Channel pg-pool Client released back to pool
pg:pool:remove Channel pg-pool Client removed from pool

I understand there may be hesitation given that AI tooling was used in the implementation. To be clear, this isn't a drive-by contribution. I designed the channel layout, wrote the proposal document, and iterated through multiple rounds to handle real edge cases (Node 18 compat, preserving custom Promise types, hasSubscribers guards). The AI assisted with the code, but the approach and decisions are mine.

Usage

Instrumentations will only need to subscribe to tracing channels to create traces, logs or metrics:

import { tracingChannel } from 'diagnostics_channel'

tracingChannel('pg:query').subscribe({
  start({ query, client }) {
    // start span
    // query: { text, name, rowMode }
    // client: { database, host, port, user, processID, ssl }
  },
  asyncEnd({ query, result }) {
    // end span
    // result: { rowCount, command }
  },
  error({ query, error }) {
    // record error on span
  },
})

This is part of a broader initiative to bring TracingChannel support to the most widely used Node.js database and cache libraries. The same pattern has already been merged and shipped in:

I have been directly involved in those prior implementations and I'm happy to own this through to release, address any feedback, and provide whatever support is needed. Would love to hear your thoughts.

Supersedes #3624.

Review in cubic

logaretm and others added 7 commits March 5, 2026 12:40
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 #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>
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>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds native diagnostics-channel tracing to pg and pg-pool.

Changes:

  • Instruments query, connection, and pool acquisition lifecycles.
  • Publishes pool release and removal events.
  • Adds diagnostics tests and compatibility fallbacks.
Show a summary per file
File Description
packages/pg/lib/diagnostics.js Defines pg tracing channels.
packages/pg/lib/client.js Instruments connections and queries.
packages/pg/test/unit/client/diagnostics-tests.js Tests client diagnostics.
packages/pg-pool/diagnostics.js Defines pool diagnostics channels.
packages/pg-pool/index.js Instruments pool lifecycle events.
packages/pg-pool/test/diagnostics.js Tests pool diagnostics.

Review details

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comments suppressed due to low confidence (1)

packages/pg/lib/client.js:231

  • The promise branch has the same raw-payload issue: _connect passes the Client as its second callback argument, and traceCallback publishes it as context.result. The tracing context should receive only the documented connection metadata while the internal resolver continues to resolve with this.
        connectionChannel.traceCallback((tracedCb) => this._connect(tracedCb), 0, context, null, callback)
  • Files reviewed: 6/6 changed files
  • Comments generated: 7
  • Review effort level: Medium

Comment thread packages/pg-pool/index.js
@@ -1,5 +1,6 @@
'use strict'
const EventEmitter = require('events').EventEmitter
const { poolConnectChannel, poolReleaseChannel, poolRemoveChannel, shouldTrace } = require('./diagnostics')
Comment on lines +27 to +29
function shouldTrace(channel) {
return channel.hasSubscribers !== false
}
Comment on lines +31 to +33
function shouldTrace(channel) {
return channel.hasSubscribers !== false
}
Comment thread packages/pg/lib/client.js
Comment on lines +726 to +729
queryChannel.traceCallback(
(tracedCb) => {
query.callback = tracedCb
enqueue()
Comment thread packages/pg/lib/client.js
Comment on lines +726 to +729
queryChannel.traceCallback(
(tracedCb) => {
query.callback = tracedCb
enqueue()
Comment thread packages/pg/lib/client.js
const context = {
connection: { database: this.database, host: this.host, port: this.port, user: this.user, ssl: !!this.ssl },
}
connectionChannel.traceCallback((tracedCb) => this._connect(tracedCb), 0, context, null, callback)
Comment thread packages/pg-pool/index.js
Comment on lines +214 to +218
const enrichedCb = (err, client, done) => {
if (client) context.client = { processID: client.processID, reused: !!client._poolUseCount }
return origCb(err, client, done)
}
poolConnectChannel.traceCallback(
@jeremyhayes
jeremyhayes deleted the eval/3650/ccr-medium/round-3 branch July 21, 2026 01:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants