feat(node): Add knex integration
#13526
Conversation
Set up initial code and dependencies. Enable integration by default. Signed-off-by: Kaung Zin Hein <kaungzinhein113@gmail.com>
| @@ -47,6 +47,7 @@ | |||
| "graphql": "^16.3.0", | |||
| "http-terminator": "^3.2.0", | |||
| "ioredis": "^5.4.1", | |||
| "knex": "^2.5.1", | |||
There was a problem hiding this comment.
Breaking change in major version 3 drops support for node < 16. See here.
Set up postgres for testing. Update span attributes to be named under 'knex'. Signed-off-by: Kaung Zin Hein <kaungzinhein113@gmail.com>
Differentiate knex spans from other database spans. Signed-off-by: Kaung Zin Hein <kaungzinhein113@gmail.com>
| const spanJSON = spanToJSON(span); | ||
| const spanData = spanJSON.data; | ||
|
|
||
| if (spanData && 'knex.version' in spanData) { |
There was a problem hiding this comment.
In knex instr, the prop knex.version is added to the span attribute.
Signed-off-by: Kaung Zin Hein <kaungzinhein113@gmail.com>
Add mysql2 client tests using knex. Add `knexIntegration` exports in several packages. Signed-off-by: Kaung Zin Hein <kaungzinhein113@gmail.com>
Zen-cronic
changed the title
knex integration
|
I've fixed all the failing CI builds/tests. The current fail is unrelated to the added integration. |
|
|
||
| if (spanData && 'knex.version' in spanData) { | ||
| span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, 'auto.db.otel.knex'); | ||
| span.setAttribute('db.system', 'knex'); |
There was a problem hiding this comment.
We shouldn't override db.system here, it's already being set by the instrumentation: https://github.com/open-telemetry/opentelemetry-js-contrib/blob/19454560b49b669c8f9c0cca9fcc3308b415f2b1/plugins/node/opentelemetry-instrumentation-knex/src/instrumentation.ts#L138
|
|
||
| export const instrumentKnex = generateInstrumentOnce( | ||
| INTEGRATION_NAME, | ||
| () => new KnexInstrumentation({ requireParentSpan: true }), |
There was a problem hiding this comment.
Here, requireParentSpan is set to true by default. So, we'd have to expose the config. Similar discussion.
|
|
||
| if (spanData && 'knex.version' in spanData) { | ||
| span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, 'auto.db.otel.knex'); | ||
| span.setAttribute('db.system', 'knex'); |
|
Failing tests due to changes after this commit , I'll tweak the tests :) |
Signed-off-by: Kaung Zin Hein <kaungzinhein113@gmail.com>
|
All the integration tests pass! Thanks for helping out @AbhiPrasad 🚀 |
This PR adds the external contributor to the CHANGELOG.md file, so that they are credited for their contribution. See #13526 Co-authored-by: AbhiPrasad <18689448+AbhiPrasad@users.noreply.github.com>
Resolves #13311
Implement Knex OTL instrumentation in
packages/node.This integration is not enabled by default.