From fb171b082e738a97e0df46b7bcb1b1a734b9a32c Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Wed, 11 Sep 2024 07:16:33 +0300 Subject: [PATCH 1/3] feat(node): Add `dataloader` instrumentation --- .../node-integration-tests/package.json | 1 + .../suites/tracing/dataloader/scenario.js | 33 ++++++++++++ .../suites/tracing/dataloader/test.ts | 40 +++++++++++++++ packages/node/package.json | 1 + packages/node/src/index.ts | 1 + .../src/integrations/tracing/dataloader.ts | 51 +++++++++++++++++++ .../node/src/integrations/tracing/index.ts | 3 ++ yarn.lock | 12 +++++ 8 files changed, 142 insertions(+) create mode 100644 dev-packages/node-integration-tests/suites/tracing/dataloader/scenario.js create mode 100644 dev-packages/node-integration-tests/suites/tracing/dataloader/test.ts create mode 100644 packages/node/src/integrations/tracing/dataloader.ts diff --git a/dev-packages/node-integration-tests/package.json b/dev-packages/node-integration-tests/package.json index 8c5a7dfe1bc3..e954fb631c97 100644 --- a/dev-packages/node-integration-tests/package.json +++ b/dev-packages/node-integration-tests/package.json @@ -43,6 +43,7 @@ "connect": "^3.7.0", "cors": "^2.8.5", "cron": "^3.1.6", + "dataloader": "2.2.2", "express": "^4.17.3", "generic-pool": "^3.9.0", "graphql": "^16.3.0", diff --git a/dev-packages/node-integration-tests/suites/tracing/dataloader/scenario.js b/dev-packages/node-integration-tests/suites/tracing/dataloader/scenario.js new file mode 100644 index 000000000000..569d23276f0b --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/dataloader/scenario.js @@ -0,0 +1,33 @@ +const { loggingTransport, startExpressServerAndSendPortToRunner } = require('@sentry-internal/node-integration-tests'); +const Sentry = require('@sentry/node'); + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + tracesSampleRate: 1.0, + transport: loggingTransport, +}); + +const PORT = 8008; + +// Stop the process from exiting before the transaction is sent +setInterval(() => {}, 1000); + +const run = async () => { + const express = require('express'); + const Dataloader = require('dataloader'); + + const app = express(); + const dataloader = new Dataloader(async keys => keys.map((_, idx) => idx), { + cache: false, + }); + + app.get('/', (req, res) => { + const user = dataloader.load('user-1'); + res.send(user); + }); + + startExpressServerAndSendPortToRunner(app, PORT); +}; + +run(); diff --git a/dev-packages/node-integration-tests/suites/tracing/dataloader/test.ts b/dev-packages/node-integration-tests/suites/tracing/dataloader/test.ts new file mode 100644 index 000000000000..27a2511f1a6e --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/dataloader/test.ts @@ -0,0 +1,40 @@ +import { cleanupChildProcesses, createRunner } from '../../../utils/runner'; + +describe('dataloader auto-instrumentation', () => { + afterAll(async () => { + cleanupChildProcesses(); + }); + + const EXPECTED_TRANSACTION = { + transaction: 'GET /', + spans: expect.arrayContaining([ + expect.objectContaining({ + data: expect.objectContaining({ + 'sentry.origin': 'auto.db.otel.dataloader', + 'sentry.op': 'cache.get', + }), + description: 'dataloader.load', + origin: 'auto.db.otel.dataloader', + op: 'cache.get', + status: 'ok', + }), + expect.objectContaining({ + data: expect.objectContaining({ + 'sentry.origin': 'auto.db.otel.dataloader', + 'sentry.op': 'cache.get', + }), + description: 'dataloader.batch', + origin: 'auto.db.otel.dataloader', + op: 'cache.get', + status: 'ok', + }), + ]), + }; + + test('should auto-instrument `dataloader` package.', done => { + createRunner(__dirname, 'scenario.js') + .expect({ transaction: EXPECTED_TRANSACTION }) + .start(done) + .makeRequest('get', '/'); + }); +}); diff --git a/packages/node/package.json b/packages/node/package.json index dace1125f2e4..d80869cd9251 100644 --- a/packages/node/package.json +++ b/packages/node/package.json @@ -70,6 +70,7 @@ "@opentelemetry/core": "^1.25.1", "@opentelemetry/instrumentation": "^0.53.0", "@opentelemetry/instrumentation-connect": "0.39.0", + "@opentelemetry/instrumentation-dataloader": "0.12.0", "@opentelemetry/instrumentation-express": "0.42.0", "@opentelemetry/instrumentation-fastify": "0.39.0", "@opentelemetry/instrumentation-fs": "0.15.0", diff --git a/packages/node/src/index.ts b/packages/node/src/index.ts index d4cbcb9544a9..f3c945f5316d 100644 --- a/packages/node/src/index.ts +++ b/packages/node/src/index.ts @@ -28,6 +28,7 @@ export { koaIntegration, setupKoaErrorHandler } from './integrations/tracing/koa export { connectIntegration, setupConnectErrorHandler } from './integrations/tracing/connect'; export { spotlightIntegration } from './integrations/spotlight'; export { genericPoolIntegration } from './integrations/tracing/genericPool'; +export { dataloaderIntegration } from './integrations/tracing/dataloader'; export { SentryContextManager } from './otel/contextManager'; export { generateInstrumentOnce } from './otel/instrument'; diff --git a/packages/node/src/integrations/tracing/dataloader.ts b/packages/node/src/integrations/tracing/dataloader.ts new file mode 100644 index 000000000000..38f9db100ca8 --- /dev/null +++ b/packages/node/src/integrations/tracing/dataloader.ts @@ -0,0 +1,51 @@ +import { DataloaderInstrumentation } from '@opentelemetry/instrumentation-dataloader'; +import { + SEMANTIC_ATTRIBUTE_SENTRY_OP, + SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, + defineIntegration, + spanToJSON, +} from '@sentry/core'; +import type { IntegrationFn } from '@sentry/types'; +import { generateInstrumentOnce } from '../../otel/instrument'; + +const INTEGRATION_NAME = 'Dataloader'; + +export const instrumentDataloader = generateInstrumentOnce(INTEGRATION_NAME, () => new DataloaderInstrumentation({})); + +const _dataloaderIntegration = (() => { + return { + name: INTEGRATION_NAME, + setupOnce() { + instrumentDataloader(); + }, + + setup(client) { + client.on('spanStart', span => { + const spanJSON = spanToJSON(span); + if (spanJSON.description?.startsWith('dataloader')) { + span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, 'auto.db.otel.dataloader'); + } + + // These are all possible dataloader span descriptions + // Still checking for the future versions + // in case they add support for `clear` and `prime` + if ( + spanJSON.description === 'dataloader.load' || + spanJSON.description === 'dataloader.loadMany' || + spanJSON.description === 'dataloader.batch' + ) { + span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_OP, 'cache.get'); + // TODO: We can try adding `key` to the `data` attribute upstream. + // Or alternatively, we can add `requestHook` to the dataloader instrumentation. + } + }); + }, + }; +}) satisfies IntegrationFn; + +/** + * Dataloader integration + * + * Capture tracing data for Dataloader. + */ +export const dataloaderIntegration = defineIntegration(_dataloaderIntegration); diff --git a/packages/node/src/integrations/tracing/index.ts b/packages/node/src/integrations/tracing/index.ts index 69ffc24a8be2..0248e3fbae21 100644 --- a/packages/node/src/integrations/tracing/index.ts +++ b/packages/node/src/integrations/tracing/index.ts @@ -2,6 +2,7 @@ import type { Integration } from '@sentry/types'; import { instrumentHttp } from '../http'; import { connectIntegration, instrumentConnect } from './connect'; +import { dataloaderIntegration, instrumentDataloader } from './dataloader'; import { expressIntegration, instrumentExpress } from './express'; import { fastifyIntegration, instrumentFastify } from './fastify'; import { genericPoolIntegration, instrumentGenericPool } from './genericPool'; @@ -41,6 +42,7 @@ export function getAutoPerformanceIntegrations(): Integration[] { connectIntegration(), genericPoolIntegration(), kafkaIntegration(), + dataloaderIntegration(), ]; } @@ -67,5 +69,6 @@ export function getOpenTelemetryInstrumentationToPreload(): (((options?: any) => instrumentGraphql, instrumentRedis, instrumentGenericPool, + instrumentDataloader, ]; } diff --git a/yarn.lock b/yarn.lock index a9e1df0710b6..692681043c70 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7101,6 +7101,13 @@ "@opentelemetry/semantic-conventions" "^1.27.0" "@types/connect" "3.4.36" +"@opentelemetry/instrumentation-dataloader@0.12.0": + version "0.12.0" + resolved "https://registry.yarnpkg.com/@opentelemetry/instrumentation-dataloader/-/instrumentation-dataloader-0.12.0.tgz#de03a3948dec4f15fed80aa424d6bd5d6a8d10c7" + integrity sha512-pnPxatoFE0OXIZDQhL2okF//dmbiWFzcSc8pUg9TqofCLYZySSxDCgQc69CJBo5JnI3Gz1KP+mOjS4WAeRIH4g== + dependencies: + "@opentelemetry/instrumentation" "^0.53.0" + "@opentelemetry/instrumentation-express@0.42.0": version "0.42.0" resolved "https://registry.yarnpkg.com/@opentelemetry/instrumentation-express/-/instrumentation-express-0.42.0.tgz#279f195aa66baee2b98623a16666c6229c8e7564" @@ -15235,6 +15242,11 @@ data-urls@^4.0.0: whatwg-mimetype "^3.0.0" whatwg-url "^12.0.0" +dataloader@2.2.2: + version "2.2.2" + resolved "https://registry.yarnpkg.com/dataloader/-/dataloader-2.2.2.tgz#216dc509b5abe39d43a9b9d97e6e5e473dfbe3e0" + integrity sha512-8YnDaaf7N3k/q5HnTJVuzSyLETjoZjVmHc4AeKAzOvKHEFQKcn64OKBfzHYtE9zGjctNM7V9I0MfnUVLpi7M5g== + date-fns@^2.29.2: version "2.29.3" resolved "https://registry.yarnpkg.com/date-fns/-/date-fns-2.29.3.tgz#27402d2fc67eb442b511b70bbdf98e6411cd68a8" From 0c3445d1b82521b407540ae99be557d325ab4ce3 Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Wed, 11 Sep 2024 13:11:08 +0300 Subject: [PATCH 2/3] Add missing exports. --- packages/astro/src/index.server.ts | 1 + packages/aws-serverless/src/index.ts | 1 + packages/bun/src/index.ts | 1 + packages/google-cloud-serverless/src/index.ts | 1 + 4 files changed, 4 insertions(+) diff --git a/packages/astro/src/index.server.ts b/packages/astro/src/index.server.ts index 2645151a9ede..0239e2a55798 100644 --- a/packages/astro/src/index.server.ts +++ b/packages/astro/src/index.server.ts @@ -29,6 +29,7 @@ export { createGetModuleFromFilename, createTransport, cron, + dataloaderIntegration, debugIntegration, dedupeIntegration, DEFAULT_USER_INCLUDES, diff --git a/packages/aws-serverless/src/index.ts b/packages/aws-serverless/src/index.ts index 19c90e3aef3f..44414824cdc1 100644 --- a/packages/aws-serverless/src/index.ts +++ b/packages/aws-serverless/src/index.ts @@ -78,6 +78,7 @@ export { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, + dataloaderIntegration, expressIntegration, expressErrorHandler, setupExpressErrorHandler, diff --git a/packages/bun/src/index.ts b/packages/bun/src/index.ts index fcb3d1331f46..267adda6fac4 100644 --- a/packages/bun/src/index.ts +++ b/packages/bun/src/index.ts @@ -99,6 +99,7 @@ export { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, + dataloaderIntegration, expressIntegration, expressErrorHandler, setupExpressErrorHandler, diff --git a/packages/google-cloud-serverless/src/index.ts b/packages/google-cloud-serverless/src/index.ts index 14aa0996cb7c..33fdc6ea314f 100644 --- a/packages/google-cloud-serverless/src/index.ts +++ b/packages/google-cloud-serverless/src/index.ts @@ -79,6 +79,7 @@ export { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, + dataloaderIntegration, expressIntegration, expressErrorHandler, setupExpressErrorHandler, From 214c71d60560bd6660eba8ebe6d3e07f18a3d759 Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Thu, 19 Sep 2024 11:49:23 +0300 Subject: [PATCH 3/3] Set `requireParentSpan` to `true`. --- packages/node/src/integrations/tracing/dataloader.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/node/src/integrations/tracing/dataloader.ts b/packages/node/src/integrations/tracing/dataloader.ts index 38f9db100ca8..d4567ea0dfbe 100644 --- a/packages/node/src/integrations/tracing/dataloader.ts +++ b/packages/node/src/integrations/tracing/dataloader.ts @@ -10,7 +10,13 @@ import { generateInstrumentOnce } from '../../otel/instrument'; const INTEGRATION_NAME = 'Dataloader'; -export const instrumentDataloader = generateInstrumentOnce(INTEGRATION_NAME, () => new DataloaderInstrumentation({})); +export const instrumentDataloader = generateInstrumentOnce( + INTEGRATION_NAME, + () => + new DataloaderInstrumentation({ + requireParentSpan: true, + }), +); const _dataloaderIntegration = (() => { return {