Add diagnostics_channel TracingChannel support by dapirian · Pull Request #47 · review-bench/brianc_node-postgres

dapirian · 2026-10-17T00:58:06Z

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.

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

🟡 Changes recommended

Published installations would fail to load, and tracing has unresolved completion, context-propagation, and sensitive-payload issues.

8 open issues

Include diagnostics.js in the published package · New
Preserve pool fast path without aggregate subscriber getter · New
Run pool acquisition inside the traced function · New
Handle multi-statement query results · New
Preserve the query callback receiver · New
Sanitize callback values before tracing · New
Route query timeouts through the traced callback · New
Preserve query fast path without aggregate subscriber getter · New

What changed in this PR

Adds diagnostics-channel instrumentation to pg and pg-pool, enabling lifecycle tracing without monkey-patching.

Changes:

Instruments client queries, connections, and pool lifecycle events.
Adds channel initialization, compatibility fallbacks, and subscriber checks.
Adds client and pool diagnostics tests.

File	Description
`packages/pg/test/unit/client/diagnostics-tests.js`	Tests query and connection tracing.
`packages/pg/lib/diagnostics.js`	Initializes client tracing channels and guards.
`packages/pg/lib/client.js`	Instruments query and connection callbacks.
`packages/pg-pool/test/diagnostics.js`	Tests acquisition, release, and removal events.
`packages/pg-pool/index.js`	Publishes pool lifecycle diagnostics.
`packages/pg-pool/diagnostics.js`	Initializes pool channels and compatibility guards.

_{Review effort: Max}

@@ -1,5 +1,6 @@
 'use strict'
 const EventEmitter = require('events').EventEmitter
+const { poolConnectChannel, poolReleaseChannel, poolRemoveChannel, shouldTrace } = require('./diagnostics')


+// Check explicitly for `false` rather than truthiness because the aggregated
+// `hasSubscribers` getter on TracingChannel is `undefined` on Node 18 (which
+// backported TracingChannel but not the getter). When `undefined`, we assume
+// there may be subscribers and trace unconditionally.
+function shouldTrace(channel) {
+  return channel.hasSubscribers !== false
+}


+        (tracedCb) => {
+          response.callback = tracedCb


+      }
+      const origCb = query.callback
+      const enrichedCb = (err, res) => {
+        if (res) context.result = { rowCount: res.rowCount, command: res.command }


+      const enrichedCb = (err, res) => {
+        if (res) context.result = { rowCount: res.rowCount, command: res.command }
+        return origCb(err, res)
+      }


+      }
+      queryChannel.traceCallback(
+        (tracedCb) => {
+          query.callback = tracedCb


+      const origCb = query.callback
+      const enrichedCb = (err, res) => {
+        if (res) context.result = { rowCount: res.rowCount, command: res.command }
+        return origCb(err, res)
+      }
+      queryChannel.traceCallback(
+        (tracedCb) => {
+          query.callback = tracedCb
+          enqueue()
+        },
+        0,
+        context,
+        null,
+        enrichedCb
+      )


+// Check explicitly for `false` rather than truthiness because the aggregated
+// `hasSubscribers` getter on TracingChannel is `undefined` on Node 18 (which
+// backported TracingChannel but not the getter). When `undefined`, we assume
+// there may be subscribers and trace unconditionally.
+function shouldTrace(channel) {
+  return channel.hasSubscribers !== false
+}


logaretm and others added 7 commits March 5, 2026 12:40

Skip TracingChannel tests on Node < 19.9

91ace72

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>

fix: format

5b50561

fix: format

3957e3c

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Copilot AI max review requested due to automatic review settings October 17, 2026 00:58

Copilot started reviewing on behalf of dapirian October 17, 2026 00:58

View session

Copilot AI reviewed Oct 17, 2026

View reviewed changes

dapirian closed this Oct 17, 2026

dapirian deleted the eval/3650/ccr-high/2026-10-16.2/round-2 branch October 17, 2026 01:16

Navigation Menu

Repository navigation

Add diagnostics_channel TracingChannel support - #47

Add diagnostics_channel TracingChannel support#47
dapirian wants to merge 7 commits into
eval/3650/basefrom
eval/3650/ccr-high/2026-10-16.2/round-2

dapirian commented Oct 17, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

dapirian commented Oct 17, 2026

Usage

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants