`instrumentation-knex` doesn't handle errors raised by `better-sqlite3`

jmadureira commented 3 days ago

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.

AbhiPrasad commented 3 days ago

I opened https://github.com/open-telemetry/opentelemetry-js-contrib/pull/2298 to address this! Reviews appreciated :)

pichlermarc commented 2 days ago

@AbhiPrasad thanks for working on the fix, would you mind if I assign you? :slightly_smiling_face:

AbhiPrasad commented 2 days ago

For sure!

open-telemetry / opentelemetry-js-contrib