Skip to content

Add diagnostics_channel TracingChannel support#3650

Open
logaretm wants to merge 10 commits into
brianc:masterfrom
logaretm:feat/diagnostics-channel
Open

Add diagnostics_channel TracingChannel support#3650
logaretm wants to merge 10 commits into
brianc:masterfrom
logaretm:feat/diagnostics-channel

Conversation

@logaretm
Copy link
Copy Markdown

@logaretm logaretm commented Apr 7, 2026

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

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.

Copilot AI review requested due to automatic review settings April 7, 2026 19:14
@logaretm
Copy link
Copy Markdown
Author

logaretm commented Apr 7, 2026

@charmander @brianc I closed the previous PR to clean up a few things. I would love to get feedback on this. I got a few points worth noting/discussing:

Argument sanitization

APMs usually sanitize arguments, the redis implementation for tracing channels actually takes this into account but I don't think this is needed for SQL since parameterized queries already separate the statement from the values. This PR emits query.text but not query.values, which is the same approach taken by OTel.

https://github.com/open-telemetry/opentelemetry-js-contrib/blob/main/packages/instrumentation-pg/src/utils.ts#L268-L271

Documentation

I haven't yet documented any of this, would something like this work here? Happy to add a doc covering channel names, payload shapes, and usage examples once the API is agreed on.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds first-class diagnostics_channel / TracingChannel instrumentation to pg and pg-pool so query/connection/pool lifecycles can be observed via subscribers instead of monkey-patching.

Changes:

  • Introduces pg:query and pg:connection TracingChannels and wires them into Client#query and Client#connect.
  • Introduces pg:pool:connect TracingChannel plus pg:pool:release / pg:pool:remove channels and wires them into pool lifecycle paths.
  • Adds unit tests validating emitted events and context payloads (skipping unstable Node versions).

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
packages/pg/lib/diagnostics.js Creates/query/connection tracing channels and shouldTrace() helper.
packages/pg/lib/client.js Wraps connect() / query() callbacks with TracingChannel#traceCallback.
packages/pg/test/unit/client/diagnostics-tests.js Adds unit tests for pg:query and pg:connection tracing.
packages/pg-pool/diagnostics.js Creates pool diagnostics channels and shouldTrace() helper.
packages/pg-pool/index.js Publishes pool connect/release/remove lifecycle events.
packages/pg-pool/test/diagnostics.js Adds tests for pool tracing + publish-only channels.

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

Comment thread packages/pg/lib/diagnostics.js
Comment thread packages/pg/lib/client.js
Comment thread packages/pg-pool/diagnostics.js
logaretm added 7 commits June 2, 2026 15:19
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
TracingChannel is not available on Node 18 LTS. Skip the tracing-dependent
tests gracefully instead of failing.
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.
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
…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.
@logaretm logaretm force-pushed the feat/diagnostics-channel branch from ce2eaaf to f722fef Compare June 2, 2026 19:19
@logaretm
Copy link
Copy Markdown
Author

logaretm commented Jun 2, 2026

@brianc friendly ping. I added docs and adjusted some areas for OTel alignment.

Since last change here we shipped this in GraphQL and Nitro with more libraries in the ecosystem adopting tracing channels. is there anything I can help with or clarify?

@logaretm logaretm force-pushed the feat/diagnostics-channel branch from fed4ead to cf9ce13 Compare June 2, 2026 19:51
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.

2 participants