feat: add diagnostics_channel support#1846
Merged
dhensby merged 17 commits intotediousjs:masterfrom Apr 22, 2026
Merged
Conversation
90eb10f to
a84b5b6
Compare
Document the new diagnostics_channel integration covering TracingChannel and point-event channels, payload schemas, security considerations, and an OpenTelemetry span example. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Create lib/diagnostics.js with pre-created TracingChannel and point-event channel instances, CHANNELS constant map, and trace/publish helpers guarded by hasSubscribers checks for zero-cost when unused. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Wrap query, batch, execute, bulk, connect, prepare, and prepared-execute operations with TracingChannel tracing on the promise API path. Add point-event publishes for: - connection acquire/release (with timing data) - pool close - transaction begin/commit/rollback - request cancel - prepared statement unprepare Internal requests (sp_prepare, sp_execute, sp_unprepare) are marked _internal and excluded from tracing to avoid framework noise. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Publish connection:create and connection:destroy point events in both tedious and msnodesqlv8 driver connection pool implementations. Also publish transaction:rollback for server-side XACT_ABORT auto-rollbacks in the tedious driver, which bypass the base class rollback() method. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add CHANNELS to the base exports so it is available via
require('mssql').CHANNELS. Consumers use these string constants
to subscribe to diagnostics channels.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Accept both plain objects and factory functions as the context parameter in tracePromise(). This ensures zero context allocation overhead when no subscribers are active. Also update README to clarify that TracingChannel events fire on the promise API only, and that connectionId is available through connection:acquire/release point events rather than request traces. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Required for diagnostics_channel.tracingChannel() support which was added in Node.js 18.19.0 (LTS maintenance release). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Test coverage for:
- CHANNELS constant exports (all 18 names, frozen)
- TracingChannel instance creation
- tracePromise zero-overhead when no subscribers
- tracePromise context factory lazy evaluation
- tracePromise lifecycle events (start, asyncEnd, error)
- publish zero-overhead when no subscribers
- publish message delivery with subscribers
- CHANNELS accessible via require('mssql')
- Request._internal flag default value
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- tracePromise always requires a context factory (no dual mode) - Remove traceCallback (unused) - Remove _poolAcquiredAt timing from connection objects - Simplify connection:acquire/release to publish facts only - Don't publish connection:destroy when connection already closed - Update README and tests accordingly Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The new tests in test/common/diagnostics.js were never actually executed because npm test names test/common/unit.js explicitly (the --recursive flag in mocha.opts is ignored when specific files are passed). Require the diagnostics spec from unit.js so all channel tests run under npm test. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three identical copies of _getPoolId lived on Request, Transaction, and PreparedStatement. Extract to lib/utils.js so the parent-chain walk is defined once and all three call sites share it. No behavioural change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous implementation did an O(n) Object.entries scan of ISOLATION_LEVEL on every transaction begin to resolve the name, and returned 'UNKNOWN' for any value not in the map — which silently dropped the caller's actual value. Build a reverse-lookup table once at module load and emit the raw numeric isolationLevel alongside the string isolationLevelName. Unknown values now surface as undefined for the name (so consumers can detect them) while the numeric value is preserved. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The POOL_CLOSE event previously only fired when pool.destroy() resolved.
If destroy rejected — which surfaces the error to close() callers —
observers got no event at all, leaving dashboards to infer closure from
the absence of connection activity.
Emit on both resolve and reject and add a reason field ('closed' vs
'error'), plus the error object on the error path, so subscribers can
distinguish clean shutdown from abnormal termination.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
mssql:prepare and mssql:prepared-execute broke the convention used everywhere else (mssql:connection:*, mssql:transaction:*, mssql:prepared-statement:unprepare). Rename before release so the namespace tree reads consistently: mssql:prepare -> mssql:prepared-statement:prepare mssql:prepared-execute -> mssql:prepared-statement:execute Constants follow the same shape (TRACE_PREPARED_STATEMENT_PREPARE / TRACE_PREPARED_STATEMENT_EXECUTE). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Document three things that only live in the code today: - SQL text in trace contexts is expected to be library-supplied or hard-coded; because node-mssql is parameterised-query-first, user values flow through parameters and never into command text. Warn consumers off hard-coding credentials as SQL literals instead of asking them to redact at the subscriber layer. - The connectionId / poolId / requestId / transactionId / preparedStatementId values are process-local integers, not UUIDs. Subscribers attempting cross-process correlation will otherwise make false assumptions. - POOL_CLOSE now carries a reason field, and TRANSACTION_BEGIN now emits both numeric and named isolation level — reflect both in the point-event table. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The existing diagnostics tests only exercised the helpers in isolation — they never drove a Request or Transaction through the real classes, so a regression in the tracePromise/publish call sites would pass silently. Stub the underlying driver callbacks (_query, _execute, _begin, etc.) and verify through real instances that: - request.query emits TRACE_QUERY start and asyncEnd with the command, parameter names, and requestId in context - request.query rejection propagates to TRACE_QUERY error with the original Error preserved on ctx.error - _internal requests are suppressed from TRACE_QUERY - request.execute populates procedure in context - request.cancel emits REQUEST_CANCEL for user requests and not for _internal ones - transaction begin/commit/rollback emit through their channels, with the new isolationLevel + isolationLevelName pair and the aborted flag Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The callback branches of query/batch/execute/bulk on Request, prepare and execute on PreparedStatement, and connect on ConnectionPool were previously invisible to TracingChannel subscribers, forcing consumers to migrate to the promise API just to get observability. Add a traceCallback helper alongside tracePromise in lib/diagnostics that wraps Node's TracingChannel#traceCallback with the same hasSubscribers fast path. Route each callback branch through it so subscribers see the same start / end / asyncStart / asyncEnd / error sub-events regardless of which API style the caller chose — no branching required on the subscriber side. Internal requests (sp_prepare, sp_execute, sp_unprepare) continue to be suppressed via _internal; Request exposes a _tracedCallback mirror of _tracedPromise to keep that gate symmetric. Stream-mode PreparedStatement.execute without a user callback falls through untraced because there is no async boundary for traceCallback to hook — streaming consumers get lifecycle via Request events instead. Update the README note that said the promise API was the only traced path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
efd6620 to
a3083c9
Compare
There was a problem hiding this comment.
Pull request overview
Adds diagnostics_channel instrumentation to node-mssql so external observability tooling can subscribe to query/pool/transaction/prepared-statement lifecycle events (TracingChannels + point events) with minimal overhead when unused, and bumps the minimum supported Node.js version to enable dc.tracingChannel().
Changes:
- Introduces
lib/diagnostics.js(CHANNELS constants, tracing/publish helpers) and exportsCHANNELSfrom the public module surface. - Instruments core public APIs (Request/ConnectionPool/Transaction/PreparedStatement) plus driver pools/transactions to emit tracing + point events.
- Adds unit tests and README documentation for the new channels and payloads; bumps
engines.nodeto>=18.19.0.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
lib/diagnostics.js |
New diagnostics_channel infrastructure (channels + helpers). |
lib/base/request.js |
Adds tracing/publish hooks around query/batch/execute/bulk and cancel; introduces _internal. |
lib/base/connection-pool.js |
Adds tracing for connect/acquire and point events for acquire/release/close. |
lib/base/transaction.js |
Publishes begin/commit/rollback point events incl. isolation level info. |
lib/base/prepared-statement.js |
Adds tracing for prepare/execute, point event for unprepare, marks internal requests. |
lib/tedious/connection-pool.js |
Publishes connection create/destroy events for tedious driver connections. |
lib/msnodesqlv8/connection-pool.js |
Publishes connection create/destroy events for msnodesqlv8 driver connections. |
lib/tedious/transaction.js |
Emits rollback event on XACT_ABORT auto-abort. |
lib/utils.js |
Adds getPoolId() helper for consistent pool identification in payloads. |
lib/base/index.js |
Exports CHANNELS from the main module surface. |
package.json |
Bumps Node engine requirement to >=18.19.0. |
README.md |
Documents Diagnostics Channel usage, channel list, and example OTel spans. |
test/common/diagnostics.js |
New unit tests covering CHANNELS, helpers, and integration wiring. |
test/common/unit.js |
Includes the new diagnostics unit test suite. |
|
🎉 This PR is included in version 12.4.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
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.
Summary
Adds
diagnostics_channelintegration tonode-mssql, enabling observability tooling (OpenTelemetry, custom metrics, logging) to subscribe to library events with near-zero overhead when not in use.Inspired by node-redis#3195.
TracingChannels (8)
Async operation tracing via
dc.tracingChannel()— provides start/end/asyncStart/asyncEnd/error lifecycle events for both the promise and callback APIs:mssql:query—request.query()mssql:batch—request.batch()mssql:execute—request.execute()mssql:bulk—request.bulk()mssql:connect—pool.connect()mssql:pool:acquire— connection acquisition from pool (measures wait/saturation)mssql:prepared-statement:prepare—ps.prepare()mssql:prepared-statement:execute—ps.execute()Point-event channels (10)
Single events at state transitions:
mssql:connection:acquire/releasemssql:connection:create/destroymssql:pool:closemssql:transaction:begin/commit/rollbackmssql:request:cancelmssql:prepared-statement:unprepareDesign decisions
>=18.19.0minimum — required fordc.tracingChannel(); includes polyfill forTracingChannel.hasSubscribers(added in Node 22)tracePromise()andtraceCallback()respectivelyhasSubscriberscheck gates all worksp_prepare,sp_execute,sp_unprepare) are marked_internal = trueand excluded from tracingtransaction:rollbackwithaborted: trueCHANNELSexported as a frozen object of string constants for minimal semver surfacegetPoolId()hoisted tolib/utils.jsas a shared utilityerrorin the payloadisolationLeveland human-readableisolationLevelNamemssql:prepared-statement:*for consistencyBreaking changes
>=18to>=18.19.0Files changed (14)
lib/diagnostics.jslib/base/request.js_internalflaglib/base/connection-pool.jslib/base/transaction.jslib/base/prepared-statement.jslib/tedious/connection-pool.jslib/msnodesqlv8/connection-pool.jslib/tedious/transaction.jslib/base/index.jslib/utils.jsgetPoolId()helperpackage.json>=18.19.0README.mdtest/common/diagnostics.jstest/common/unit.jsTest coverage
74 total unit tests passing (47 existing + 27 new diagnostics tests):
tracePromise/traceCallbackzero-overhead when no subscriberstraceCallbacklifecycle eventspublishzero-overhead and message deliveryrequire("mssql")_internalflag default value