Skip to content

Add diagnostics_channel TracingChannel support - #29

Closed
MiichelleZhou wants to merge 7 commits into
eval/3650/basefrom
eval/3650/cursor/round-4
Closed

MiichelleZhou wants to merge 7 commits into
eval/3650/basefrom
eval/3650/cursor/round-4

Conversation

@MiichelleZhou

@MiichelleZhou MiichelleZhou commented Sep 28, 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.


Note

Medium Risk
Instrumentation wraps connect/query/pool hot paths via callback substitution; behavior should be unchanged when no subscribers, but regressions in async completion or promise paths are possible.

Overview
Adds Node.js diagnostics_channel instrumentation to pg and pg-pool so APM/tracing tools can subscribe without monkey-patching.

pg exposes pg:query and pg:connection TracingChannels, wrapping Client.query (callback-based paths) and Client.connect with traceCallback and attaching query/connection metadata (and query rowCount/command on completion). pg-pool adds pg:pool:connect (TracingChannel with pool stats and client processID/reused), plus pg:pool:release and pg:pool:remove publish channels on release and removal (including release errors).

Shared diagnostics.js helpers load diagnostics_channel safely (including getBuiltInModule when available), use noop channels when missing, and shouldTrace only skips work when hasSubscribers === false (Node 18 compat). New unit tests cover channel payloads; TracingChannel tests are skipped on Node versions without a stable hasSubscribers getter.

Reviewed by Cursor Bugbot for commit b922f0d. Configure here.

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>
@MiichelleZhou

Copy link
Copy Markdown
Author

cursor review

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Add diagnostics channels for pg queries, connections, and pool events

✨ Enhancement 🧪 Tests 🕐 40+ Minutes

Grey Divider

AI Description

• Expose query, connection, and pool lifecycles to instrumentation without monkey-patching.
• Provide tracing context for asynchronous operations and events for pool release and removal.
• Guard instrumentation when unavailable and test channel payloads and lifecycle events.
Diagram

graph TD
  Caller["Application"] --> Pool["Pool checkout"] --> PoolDiag["Pool channels"] --> DC["Node diagnostics"] --> Sub["Trace subscribers"]
  Caller --> Client["PG client"] --> PgDiag["Client channels"] --> DC
  Pool --> Client
Loading
High-Level Assessment

Per-package diagnostics channels fit the goal better than existing EventEmitter hooks, which do not provide query-wide asynchronous tracing context. Before publishing, include the new pg-pool/diagnostics.js helper in pg-pool's package file list so its index.js import resolves.

Files changed (6) +557 / -11

Enhancement (4) +155 / -11
diagnostics.jsDefine pool diagnostics channels and compatibility guard +35/-0

Define pool diagnostics channels and compatibility guard

• Creates a tracing channel for pool checkout and plain channels for release and removal. Falls back when diagnostics_channel is unavailable and accommodates Node 18's undefined aggregate subscriber flag.

packages/pg-pool/diagnostics.js

index.jsInstrument pool checkout, release, and removal +34/-0

Instrument pool checkout, release, and removal

• Traces checkout through its completion callback, including pool counts and acquired-client information. Publishes guarded release and removal events with client identifiers and release errors.

packages/pg-pool/index.js

client.jsTrace client connections and callback-backed queries +55/-11

Trace client connections and callback-backed queries

• Wraps callback and promise connection paths in connection tracing. Traces queued queries with query and client metadata, enriching successful completions with result details while retaining an untraced path.

packages/pg/lib/client.js

diagnostics.jsDefine query and connection tracing channels +31/-0

Define query and connection tracing channels

• Registers pg:query and pg:connection when tracing channels are available. Supplies no-op fallbacks and a subscriber guard compatible with Node 18's undefined aggregate flag.

packages/pg/lib/diagnostics.js

Tests (2) +402 / -0
diagnostics.jsTest pool lifecycle diagnostics +197/-0

Test pool lifecycle diagnostics

• Tests checkout context enrichment, release payloads and errors, and removal publication using mock clients. Skips tracing tests on runtimes without a stable TracingChannel implementation.

packages/pg-pool/test/diagnostics.js

diagnostics-tests.jsTest query and connection tracing events +205/-0

Test query and connection tracing events

• Exercises query success, failure, promise usage, context fields, and callback-based connection tracing. Skips tracing cases when the runtime lacks a stable TracingChannel implementation.

packages/pg/test/unit/client/diagnostics-tests.js

@cursor cursor Bot 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.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit b922f0d. Configure here.

Comment thread packages/pg/lib/client.js
context,
null,
enrichedCb
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Query timeout leaves traces unfinished

Medium Severity

pg:query tracing wraps query.callback via traceCallback, but query_timeout invokes the saved pre-wrap callback and then replaces query.callback with a no-op. After start is published, error and asyncEnd never fire, so subscribers can leak spans when a query times out.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit b922f0d. Configure here.

@MiichelleZhou
MiichelleZhou deleted the eval/3650/cursor/round-4 branch September 28, 2026 03:12
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