opentelemetry-js-contrib
opentelemetry-js-contrib copied to clipboard
`instrumentation-knex` doesn't handle errors raised by `better-sqlite3`
What version of OpenTelemetry are you using?
"@opentelemetry/api": "^1.9.0",
"@opentelemetry/auto-instrumentations-node": "^0.47.1",
"@opentelemetry/exporter-prometheus": "^0.52.1",
"@opentelemetry/exporter-trace-otlp-grpc": "^0.52.1",
"@opentelemetry/sdk-node": "^0.52.0",
What version of Node are you using?
v18.18.2
What did you do?
Instrument any application that uses knex and better-sqlite3 and trigger any SQL error.
What did you expect to see?
A span or trace should be correctly created and emitted.
What did you see instead?
A TypeError: Expected second argument to be a string is thrown instead and the application probably stops. Either way not span gets created.
Additional context
Looking at the stackstrace:
cause: TypeError: Expected second argument to be a string
at new SqliteError (###/node_modules/better-sqlite3/lib/sqlite-error.js:9:9)
at Object.cloneErrorWithNewMessage (###/node_modules/@opentelemetry/instrumentation-knex/src/utils.ts:48:25)
at <anonymous> (###/node_modules/@opentelemetry/instrumentation-knex/src/instrumentation.ts:179:39)
at async Runner.ensureConnection (###/node_modules/knex/lib/execution/runner.js:318:14)
at async Runner.run (###/node_modules/knex/lib/execution/runner.js:30:19)
at async ###/node_modules/knex/lib/execution/batch-insert.js:31:30
The problem seems to be between https://github.com/open-telemetry/opentelemetry-js-contrib/blob/main/plugins/node/opentelemetry-instrumentation-knex/src/utils.ts#L48 that creates a new instance of the underlying error and https://github.com/WiseLibs/better-sqlite3/blob/master/lib/sqlite-error.js#L9 which requires callers to provide 2 arguments. The 2nd argument (code) is not being passed by the knex instrumenter.
I opened https://github.com/open-telemetry/opentelemetry-js-contrib/pull/2298 to address this! Reviews appreciated :)
@AbhiPrasad thanks for working on the fix, would you mind if I assign you? :slightly_smiling_face:
For sure!