feat(instrumentation-oracledb): add database client metrics - #3657
feat(instrumentation-oracledb): add database client metrics#3657prajalg wants to merge 19 commits into
Conversation
|
Welcome, contributor! Thank you for your contribution to opentelemetry-js-contrib. Important reminders:
|
Pull request dashboard statusWaiting on reviewers · refreshed 2026-08-13 20:47 UTC Review the latest changes. Status above doesn't look right?
|
| "version": "6.5.2", | ||
| "resolved": "https://registry.npmjs.org/@types/oracledb/-/oracledb-6.5.2.tgz", | ||
| "integrity": "sha512-kK1eBS/Adeyis+3OlBDMeQQuasIDLUYXsi2T15ccNJ0iyUpQ4xDF7svFu3+bGVrI0CMBUclPciz+lsQR3JX3TQ==", | ||
| "version": "6.10.4", |
There was a problem hiding this comment.
Should this be 6.10.4 or the 7.0.x version
There was a problem hiding this comment.
I tested with @types/oracledb 7.0.2, but encountered an error:
Cannot find lib definition for 'esnext.disposable', seems it is not in TS 5.0.4 which the root repository uses.
Looks like it needs some typescript update, so decided to avoid updating to latest types.
| "@opentelemetry/instrumentation": "^0.221.0", | ||
| "@opentelemetry/semantic-conventions": "^1.34.0", | ||
| "@types/oracledb": "6.5.2" | ||
| "@types/oracledb": "^6.10.4" |
There was a problem hiding this comment.
Should this version be greater than 7.0.x?
There was a problem hiding this comment.
same reason of use of esnext.disposable in newer types.
Repository TS version should be updated first to support 7.0.2 types oracledb version
| // calculate the correct delta on every pool event. On the next pool event, this | ||
| // also reconciles any pool changes that were not recorded, such as changes made | ||
| // while instrumentation was disabled, ensuring metric values remain accurate. | ||
| const connectionsCounterState: Record<string, PoolConnectionsCounter> = {}; |
There was a problem hiding this comment.
a weak map probably may be better to automatically cleanup as the pools are closed/freed without receiving a callback (like when tracing is disabled after enable) into this module.
const connectionsCounterState = new WeakMap<oracleDBTypes.Pool, PoolConnectionsCounter>();
| pool.getConnection((err, conn) => { | ||
| pool.getConnection((...args) => { | ||
| const [err, conn] = args; |
There was a problem hiding this comment.
@types/oracledb@6.10.4 defines ResultCallback<T> as (...args: [DBError] | [null, T]) => void. A callback therefore receives either one error argument or two arguments for success. Our tests previously used (err, result), which assumes there are always two arguments. Using (...args) and destructuring the values works for both cases.
|
@maryliag Can you help us with the review here? |
maryliag
left a comment
maryliag
left a comment
There was a problem hiding this comment.
Thank you for working on this, I added just small nits
|
Hi @prajalg — just a friendly reminder that this pull request is waiting on you. There are still items that need your attention. See the dashboard status comment for the full list. You don't need to push a code change to hand it back — replying to move each discussion forward is enough, whether that's answering a question, explaining why no change is needed, or asking a follow-up. The dashboard then automatically routes it back to reviewers. If you believe this pull request is incorrectly routed as waiting on the author, comment |
|
/dashboard route:reviewers |
|
@prajalg routed this pull request to reviewers. |
4 participants
Which problem is this PR solving?
Adds metrics support for
@opentelemetry/instrumentation-oracledb.This introduces Oracle DB client metrics aligned with OpenTelemetry database semantic conventions:
db.client.connection.countdb.client.operation.durationdb.client.connection.pending_requestsdb.client.connection.timeoutsShort description of the changes
This PR adds metric collection to
instrumentation-oracledbusing node-oracledb trace handler callbacks.Main changes:
OracleTelemetryTraceMetricHandler, extending the existing trace handler behavior with metric callbacks.connectionsCounterStatemap which stores counts as {idle, pending, used, timeouts} for eachpoolNameso that it also supports instrumented application to have multiple pools (each with unique pool alias). Maintaining this map also reconciles if any pool changes that were not recorded, such as the changes made while instrumentation was disabled, ensuring metric values remain accurate after instrumentation is enabled again.oracledbtest dependency to^6.7.0and@types/oracledbto6.10.4.The pool metrics are recorded by reading the latest pool statistics and publishing deltas through OpenTelemetry metric instruments.