feat(node): Add `knex` integration #13526

Zen-cronic · 2024-08-29T15:26:53Z

Resolves #13311

Implement Knex OTL instrumentation in packages/node.

This integration is not enabled by default.

Set up initial code and dependencies. Enable integration by default. Signed-off-by: Kaung Zin Hein <kaungzinhein113@gmail.com>

Zen-cronic · 2024-08-29T15:28:04Z

dev-packages/node-integration-tests/package.json

@@ -47,6 +47,7 @@
    "graphql": "^16.3.0",
    "http-terminator": "^3.2.0",
    "ioredis": "^5.4.1",
+    "knex": "^2.5.1",


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>

Zen-cronic · 2024-08-30T01:08:50Z

packages/node/src/integrations/tracing/knex.ts

+        const spanJSON = spanToJSON(span);
+        const spanData = spanJSON.data;
+
+        if (spanData && 'knex.version' in spanData) {


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 · 2024-08-30T19:26:56Z

I've fixed all the failing CI builds/tests. The current fail is unrelated to the added integration.

AbhiPrasad

Hey thanks for adding the instrumentation!

In general looks good to me. After we resolve the db.system concern we are good to merge.

If possible, could you share a screenshot of an example trace with knex instrumentation spans?

AbhiPrasad · 2024-09-18T13:11:48Z

packages/node/src/integrations/tracing/knex.ts

+
+        if (spanData && 'knex.version' in spanData) {
+          span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, 'auto.db.otel.knex');
+          span.setAttribute('db.system', 'knex');


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

Zen-cronic · 2024-09-14T00:44:53Z

packages/node/src/integrations/tracing/knex.ts

+
+export const instrumentKnex = generateInstrumentOnce(
+  INTEGRATION_NAME,
+  () => new KnexInstrumentation({ requireParentSpan: true }),


Here, requireParentSpan is set to true by default. So, we'd have to expose the config. Similar discussion.

Zen-cronic · 2024-09-30T00:28:05Z

packages/node/src/integrations/tracing/knex.ts

+
+        if (spanData && 'knex.version' in spanData) {
+          span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, 'auto.db.otel.knex');
+          span.setAttribute('db.system', 'knex');


Zen-cronic · 2024-11-11T15:02:56Z

Failing tests due to changes after this commit , I'll tweak the tests :)

Signed-off-by: Kaung Zin Hein <kaungzinhein113@gmail.com>

Zen-cronic · 2024-11-11T16:07:42Z

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>

This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [@sentry/node](https://github.com/getsentry/sentry-javascript/tree/master/packages/node) ([source](https://github.com/getsentry/sentry-javascript)) | dependencies | minor | [`8.37.1` -> `8.40.0`](https://renovatebot.com/diffs/npm/@sentry%2fnode/8.37.1/8.40.0) | | [@sentry/react](https://github.com/getsentry/sentry-javascript/tree/master/packages/react) ([source](https://github.com/getsentry/sentry-javascript)) | dependencies | minor | [`8.37.1` -> `8.40.0`](https://renovatebot.com/diffs/npm/@sentry%2freact/8.37.1/8.40.0) | --- ### Release Notes <details> <summary>getsentry/sentry-javascript (@sentry/node)</summary> ### [`v8.40.0`](https://github.com/getsentry/sentry-javascript/blob/HEAD/CHANGELOG.md#8400) [Compare Source](getsentry/sentry-javascript@8.39.0...8.40.0) ##### Important Changes - **feat(angular): Support Angular 19 ([#14398](getsentry/sentry-javascript#14398 The `@sentry/angular` SDK can now be used with Angular 19. If you're upgrading to the new Angular version, you might want to migrate from the now deprecated `APP_INITIALIZER` token to `provideAppInitializer`. In this case, change the Sentry `TraceService` initialization in `app.config.ts`: ```ts // Angular 18 export const appConfig: ApplicationConfig = { providers: [ // other providers { provide: TraceService, deps: [Router], }, { provide: APP_INITIALIZER, useFactory: () => () => {}, deps: [TraceService], multi: true, }, ], }; // Angular 19 export const appConfig: ApplicationConfig = { providers: [ // other providers { provide: TraceService, deps: [Router], }, provideAppInitializer(() => { inject(TraceService); }), ], }; ``` - **feat(core): Deprecate `debugIntegration` and `sessionTimingIntegration` ([#14363](getsentry/sentry-javascript#14363 The `debugIntegration` was deprecated and will be removed in the next major version of the SDK. To log outgoing events, use [Hook Options](https://docs.sentry.io/platforms/javascript/configuration/options/#hooks) (`beforeSend`, `beforeSendTransaction`, ...). The `sessionTimingIntegration` was deprecated and will be removed in the next major version of the SDK. To capture session durations alongside events, use [Context](https://docs.sentry.io/platforms/javascript/enriching-events/context/) (`Sentry.setContext()`). - **feat(nestjs): Deprecate `@WithSentry` in favor of `@SentryExceptionCaptured` ([#14323](getsentry/sentry-javascript#14323 The `@WithSentry` decorator was deprecated. Use `@SentryExceptionCaptured` instead. This is a simple renaming and functionality stays identical. - **feat(nestjs): Deprecate `SentryTracingInterceptor`, `SentryService`, `SentryGlobalGenericFilter`, `SentryGlobalGraphQLFilter` ([#14371](getsentry/sentry-javascript#14371 The `SentryTracingInterceptor` was deprecated. If you are using `@sentry/nestjs` you can safely remove any references to the `SentryTracingInterceptor`. If you are using another package migrate to `@sentry/nestjs` and remove the `SentryTracingInterceptor` afterwards. The `SentryService` was deprecated and its functionality was added to `Sentry.init`. If you are using `@sentry/nestjs` you can safely remove any references to the `SentryService`. If you are using another package migrate to `@sentry/nestjs` and remove the `SentryService` afterwards. The `SentryGlobalGenericFilter` was deprecated. Use the `SentryGlobalFilter` instead which is a drop-in replacement. The `SentryGlobalGraphQLFilter` was deprecated. Use the `SentryGlobalFilter` instead which is a drop-in replacement. - **feat(node): Deprecate `nestIntegration` and `setupNestErrorHandler` in favor of using `@sentry/nestjs` ([#14374](getsentry/sentry-javascript#14374 The `nestIntegration` and `setupNestErrorHandler` functions from `@sentry/node` were deprecated and will be removed in the next major version of the SDK. If you're using `@sentry/node` in a NestJS application, we recommend switching to our new dedicated `@sentry/nestjs` package. ##### Other Changes - feat(browser): Send additional LCP timing info ([#14372](getsentry/sentry-javascript#14372)) - feat(replay): Clear event buffer when full and in buffer mode ([#14078](getsentry/sentry-javascript#14078)) - feat(core): Ensure `normalizedRequest` on `sdkProcessingMetadata` is merged ([#14315](getsentry/sentry-javascript#14315)) - feat(core): Hoist everything from `@sentry/utils` into `@sentry/core` ([#14382](getsentry/sentry-javascript#14382)) - fix(core): Do not throw when trying to fill readonly properties ([#14402](getsentry/sentry-javascript#14402)) - fix(feedback): Fix `__self` and `__source` attributes on feedback nodes ([#14356](getsentry/sentry-javascript#14356)) - fix(feedback): Fix non-wrapping form title ([#14355](getsentry/sentry-javascript#14355)) - fix(nextjs): Update check for not found navigation error ([#14378](getsentry/sentry-javascript#14378)) ### [`v8.39.0`](https://github.com/getsentry/sentry-javascript/blob/HEAD/CHANGELOG.md#8390) [Compare Source](getsentry/sentry-javascript@8.38.0...8.39.0) ##### Important Changes - **feat(nestjs): Instrument event handlers ([#14307](getsentry/sentry-javascript#14307 The `@sentry/nestjs` SDK will now capture performance data for [NestJS Events (`@nestjs/event-emitter`)](https://docs.nestjs.com/techniques/events) ##### Other Changes - feat(nestjs): Add alias `@SentryExceptionCaptured` for `@WithSentry` ([#14322](getsentry/sentry-javascript#14322)) - feat(nestjs): Duplicate `SentryService` behaviour into `@sentry/nestjs` SDK `init()` ([#14321](getsentry/sentry-javascript#14321)) - feat(nestjs): Handle GraphQL contexts in `SentryGlobalFilter` ([#14320](getsentry/sentry-javascript#14320)) - feat(node): Add alias `childProcessIntegration` for `processThreadBreadcrumbIntegration` and deprecate it ([#14334](getsentry/sentry-javascript#14334)) - feat(node): Ensure request bodies are reliably captured for http requests ([#13746](getsentry/sentry-javascript#13746)) - feat(replay): Upgrade rrweb packages to 2.29.0 ([#14160](getsentry/sentry-javascript#14160)) - fix(cdn): Ensure `_sentryModuleMetadata` is not mangled ([#14344](getsentry/sentry-javascript#14344)) - fix(core): Set `sentry.source` attribute to `custom` when calling `span.updateName` on `SentrySpan` ([#14251](getsentry/sentry-javascript#14251)) - fix(mongo): rewrite Buffer as ? during serialization ([#14071](getsentry/sentry-javascript#14071)) - fix(replay): Remove replay id from DSC on expired sessions ([#14342](getsentry/sentry-javascript#14342)) - ref(profiling) Fix electron crash ([#14216](getsentry/sentry-javascript#14216)) - ref(types): Deprecate `Request` type in favor of `RequestEventData` ([#14317](getsentry/sentry-javascript#14317)) - ref(utils): Stop setting `transaction` in `requestDataIntegration` ([#14306](getsentry/sentry-javascript#14306)) - ref(vue): Reduce bundle size for starting application render span ([#14275](getsentry/sentry-javascript#14275)) ### [`v8.38.0`](https://github.com/getsentry/sentry-javascript/blob/HEAD/CHANGELOG.md#8380) [Compare Source](getsentry/sentry-javascript@8.37.1...8.38.0) - docs: Improve docstrings for node otel integrations ([#14217](getsentry/sentry-javascript#14217)) - feat(browser): Add moduleMetadataIntegration lazy loading support ([#13817](getsentry/sentry-javascript#13817)) - feat(core): Add trpc path to context in trpcMiddleware ([#14218](getsentry/sentry-javascript#14218)) - feat(deps): Bump [@opentelemetry/instrumentation-amqplib](https://github.com/opentelemetry/instrumentation-amqplib) from 0.42.0 to 0.43.0 ([#14230](getsentry/sentry-javascript#14230)) - feat(deps): Bump [@sentry/cli](https://github.com/sentry/cli) from 2.37.0 to 2.38.2 ([#14232](getsentry/sentry-javascript#14232)) - feat(node): Add `knex` integration ([#13526](getsentry/sentry-javascript#13526)) - feat(node): Add `tedious` integration ([#13486](getsentry/sentry-javascript#13486)) - feat(utils): Single implementation to fetch debug ids ([#14199](getsentry/sentry-javascript#14199)) - fix(browser): Avoid recording long animation frame spans starting before their parent span ([#14186](getsentry/sentry-javascript#14186)) - fix(node): Include `debug_meta` with ANR events ([#14203](getsentry/sentry-javascript#14203)) - fix(nuxt): Fix dynamic import rollup plugin to work with latest nitro ([#14243](getsentry/sentry-javascript#14243)) - fix(react): Support wildcard routes on React Router 6 ([#14205](getsentry/sentry-javascript#14205)) - fix(spotlight): Export spotlightBrowserIntegration from the main browser package ([#14208](getsentry/sentry-javascript#14208)) - ref(browser): Ensure start time of interaction root and child span is aligned ([#14188](getsentry/sentry-javascript#14188)) - ref(nextjs): Make build-time value injection turbopack compatible ([#14081](getsentry/sentry-javascript#14081)) Work in this release was contributed by [@grahamhency](https://github.com/grahamhency), [@Zen-cronic](https://github.com/Zen-cronic), [@gilisho](https://github.com/gilisho) and [@phuctm97](https://github.com/phuctm97). Thank you for your contributions! </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about these updates again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).  Reviewed-on: https://git.tristess.app/alexandresoro/ouca/pulls/317 Reviewed-by: Alexandre Soro <code@soro.dev> Co-authored-by: renovate <renovate@git.tristess.app> Co-committed-by: renovate <renovate@git.tristess.app>

feat(node): Add knexIntegration to Node

b3ba484

Set up initial code and dependencies. Enable integration by default. Signed-off-by: Kaung Zin Hein <kaungzinhein113@gmail.com>

Zen-cronic commented Aug 29, 2024

View reviewed changes

test(node): Add tests for knexIntegration

e679a21

Set up postgres for testing. Update span attributes to be named under 'knex'. Signed-off-by: Kaung Zin Hein <kaungzinhein113@gmail.com>

Zen-cronic marked this pull request as ready for review August 29, 2024 17:17

fix(node): Update integration to identify knex spans

36d425f

Differentiate knex spans from other database spans. Signed-off-by: Kaung Zin Hein <kaungzinhein113@gmail.com>

Zen-cronic commented Aug 30, 2024

View reviewed changes

Zen-cronic added 2 commits August 29, 2024 22:59

fix(e2e): Update exports for node-exports-test-app

b40cdad

Signed-off-by: Kaung Zin Hein <kaungzinhein113@gmail.com>

test(node): Add more db client tests

08a6d6d

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 ~~feat(node): Add knexIntegration to Node~~ feat(node): Add knex integration Aug 30, 2024

AbhiPrasad requested changes Sep 18, 2024

View reviewed changes

Zen-cronic commented Oct 20, 2024

View reviewed changes

AbhiPrasad added 3 commits November 7, 2024 10:16

Merge branch 'develop' into feat/knexIntegration-node

de2277a

Merge branch 'develop' into feat/knexIntegration-node

60b6132

clean up integration

8c04ae8

AbhiPrasad self-requested a review November 7, 2024 15:23

AbhiPrasad approved these changes Nov 7, 2024

View reviewed changes

AbhiPrasad self-assigned this Nov 7, 2024

fix merge conflict in yarn.lock

eb3902d

test(node): Fix tests after latest change

a943bbe

Signed-off-by: Kaung Zin Hein <kaungzinhein113@gmail.com>

AbhiPrasad enabled auto-merge (squash) November 11, 2024 17:23

AbhiPrasad merged commit 1f56ffa into getsentry:develop Nov 11, 2024
131 checks passed

HazAT mentioned this pull request Nov 11, 2024

chore: Add external contributor to CHANGELOG.md #14235

Merged

github-actions bot mentioned this pull request Nov 12, 2024

Add knexIntegration to Node #13311

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(node): Add `knex` integration #13526

feat(node): Add `knex` integration #13526

Zen-cronic commented Aug 29, 2024 •

edited by AbhiPrasad

Loading

Zen-cronic Aug 29, 2024

Zen-cronic Aug 30, 2024

Zen-cronic commented Aug 30, 2024 •

edited

Loading

AbhiPrasad left a comment

AbhiPrasad Sep 18, 2024 •

edited

Loading

Zen-cronic Sep 30, 2024

Zen-cronic Sep 14, 2024

Zen-cronic Sep 30, 2024

Zen-cronic commented Nov 11, 2024

Zen-cronic commented Nov 11, 2024

feat(node): Add knex integration #13526

feat(node): Add knex integration #13526

Conversation

Zen-cronic commented Aug 29, 2024 • edited by AbhiPrasad Loading

Zen-cronic Aug 29, 2024

Choose a reason for hiding this comment

Zen-cronic Aug 30, 2024

Choose a reason for hiding this comment

Zen-cronic commented Aug 30, 2024 • edited Loading

AbhiPrasad left a comment

Choose a reason for hiding this comment

AbhiPrasad Sep 18, 2024 • edited Loading

Choose a reason for hiding this comment

Zen-cronic Sep 30, 2024

Choose a reason for hiding this comment

Zen-cronic Sep 14, 2024

Choose a reason for hiding this comment

Zen-cronic Sep 30, 2024

Choose a reason for hiding this comment

Zen-cronic commented Nov 11, 2024

Zen-cronic commented Nov 11, 2024

feat(node): Add `knex` integration #13526

feat(node): Add `knex` integration #13526

Zen-cronic commented Aug 29, 2024 •

edited by AbhiPrasad

Loading

Zen-cronic commented Aug 30, 2024 •

edited

Loading

AbhiPrasad Sep 18, 2024 •

edited

Loading