From 2a33abf362b2484b5ecdd745b0fce14489f34e98 Mon Sep 17 00:00:00 2001 From: Francesco Gringl-Novy Date: Thu, 11 Jun 2026 16:49:50 +0200 Subject: [PATCH 1/5] ref(node): Streamline dataloader instrumentation --- .../suites/tracing/dataloader/scenario.mjs | 30 +- .../suites/tracing/dataloader/test.ts | 116 ++++++-- .../integrations/tracing/dataloader/index.ts | 47 +-- .../dataloader/vendored/dataloader-types.ts | 19 -- .../dataloader/vendored/instrumentation.ts | 269 +++++++----------- .../tracing/dataloader/vendored/types.ts | 26 +- 6 files changed, 239 insertions(+), 268 deletions(-) delete mode 100644 packages/node/src/integrations/tracing/dataloader/vendored/dataloader-types.ts diff --git a/dev-packages/node-integration-tests/suites/tracing/dataloader/scenario.mjs b/dev-packages/node-integration-tests/suites/tracing/dataloader/scenario.mjs index ac42c954f111..18c397d1e240 100644 --- a/dev-packages/node-integration-tests/suites/tracing/dataloader/scenario.mjs +++ b/dev-packages/node-integration-tests/suites/tracing/dataloader/scenario.mjs @@ -4,15 +4,35 @@ import express from 'express'; const PORT = 8008; +const batchLoadFn = async keys => keys.map((_, idx) => idx); + const run = async () => { const app = express(); - const dataloader = new Dataloader(async keys => keys.map((_, idx) => idx), { - cache: false, + + app.get('/load', async (_req, res) => { + const dataloader = new Dataloader(batchLoadFn, { cache: false }); + const user = await dataloader.load('user-1'); + res.send({ user }); + }); + + app.get('/load-many', async (_req, res) => { + const dataloader = new Dataloader(batchLoadFn, { cache: false }); + const users = await dataloader.loadMany(['user-1', 'user-2']); + res.send({ users }); + }); + + app.get('/cache-ops', async (_req, res) => { + const dataloader = new Dataloader(batchLoadFn); + dataloader.prime('user-1', 1); + dataloader.clear('user-1'); + dataloader.clearAll(); + res.send({}); }); - app.get('/', (req, res) => { - const user = dataloader.load('user-1'); - res.send(user); + app.get('/named', async (_req, res) => { + const dataloader = new Dataloader(batchLoadFn, { cache: false, name: 'usersLoader' }); + const user = await dataloader.load('user-1'); + res.send({ user }); }); startExpressServerAndSendPortToRunner(app, PORT); diff --git a/dev-packages/node-integration-tests/suites/tracing/dataloader/test.ts b/dev-packages/node-integration-tests/suites/tracing/dataloader/test.ts index 5bfb6ff72a39..90c3395f9dd9 100644 --- a/dev-packages/node-integration-tests/suites/tracing/dataloader/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/dataloader/test.ts @@ -1,42 +1,96 @@ import { afterAll, describe, expect } from 'vitest'; import { cleanupChildProcesses, createEsmAndCjsTests } from '../../../utils/runner'; +const ORIGIN = 'auto.db.otel.dataloader'; +const CACHE_GET_OP = 'cache.get'; + describe('dataloader auto-instrumentation', () => { - afterAll(async () => { + afterAll(() => { 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', - }), - ]), - }; - createEsmAndCjsTests(__dirname, 'scenario.mjs', 'instrument.mjs', (createRunner, test) => { - test('should auto-instrument `dataloader` package.', async () => { - const runner = createRunner().expect({ transaction: EXPECTED_TRANSACTION }).start(); - runner.makeRequest('get', '/'); + test('instruments load, loadMany, batch, prime, clear and clearAll', async () => { + const runner = createRunner() + .expect({ + transaction: event => { + expect(event.transaction).toBe('GET /load'); + + const spans = event.spans || []; + + const loadSpan = spans.find(span => span.description === 'dataloader.load'); + expect(loadSpan).toBeDefined(); + expect(loadSpan?.op).toBe(CACHE_GET_OP); + expect(loadSpan?.origin).toBe(ORIGIN); + expect(loadSpan?.status).toBe('ok'); + expect(loadSpan?.data?.['sentry.origin']).toBe(ORIGIN); + expect(loadSpan?.data?.['sentry.op']).toBe(CACHE_GET_OP); + + const batchSpan = spans.find(span => span.description === 'dataloader.batch'); + expect(batchSpan).toBeDefined(); + expect(batchSpan?.op).toBe(CACHE_GET_OP); + expect(batchSpan?.origin).toBe(ORIGIN); + expect(batchSpan?.status).toBe('ok'); + + // The batch span links back to the load span that triggered it + expect(batchSpan?.links).toEqual([ + expect.objectContaining({ + trace_id: loadSpan?.trace_id, + span_id: loadSpan?.span_id, + }), + ]); + }, + }) + .expect({ + transaction: event => { + expect(event.transaction).toBe('GET /load-many'); + + const loadManySpan = (event.spans || []).find(span => span.description === 'dataloader.loadMany'); + expect(loadManySpan).toBeDefined(); + expect(loadManySpan?.op).toBe(CACHE_GET_OP); + expect(loadManySpan?.origin).toBe(ORIGIN); + expect(loadManySpan?.status).toBe('ok'); + expect(loadManySpan?.data?.['sentry.origin']).toBe(ORIGIN); + expect(loadManySpan?.data?.['sentry.op']).toBe(CACHE_GET_OP); + }, + }) + .expect({ + transaction: event => { + expect(event.transaction).toBe('GET /cache-ops'); + + const spans = event.spans || []; + + // prime/clear/clearAll are not cache reads, so they get an origin but no `op` + for (const operation of ['prime', 'clear', 'clearAll']) { + const span = spans.find(s => s.description === `dataloader.${operation}`); + expect(span, `expected a dataloader.${operation} span`).toBeDefined(); + expect(span?.origin).toBe(ORIGIN); + expect(span?.status).toBe('ok'); + expect(span?.op).toBeUndefined(); + expect(span?.data?.['sentry.origin']).toBe(ORIGIN); + expect(span?.data?.['sentry.op']).toBeUndefined(); + } + }, + }) + .expect({ + transaction: event => { + expect(event.transaction).toBe('GET /named'); + + // A named dataloader includes its name in the span description + const namedLoadSpan = (event.spans || []).find(span => span.description === 'dataloader.load usersLoader'); + expect(namedLoadSpan).toBeDefined(); + expect(namedLoadSpan?.op).toBe(CACHE_GET_OP); + expect(namedLoadSpan?.origin).toBe(ORIGIN); + expect(namedLoadSpan?.status).toBe('ok'); + }, + }) + .start(); + + await runner.makeRequest('get', '/load'); + await runner.makeRequest('get', '/load-many'); + await runner.makeRequest('get', '/cache-ops'); + await runner.makeRequest('get', '/named'); await runner.completed(); - }); + }, 30_000); }); }); diff --git a/packages/node/src/integrations/tracing/dataloader/index.ts b/packages/node/src/integrations/tracing/dataloader/index.ts index c6024a48778e..c1930411d5e0 100644 --- a/packages/node/src/integrations/tracing/dataloader/index.ts +++ b/packages/node/src/integrations/tracing/dataloader/index.ts @@ -1,56 +1,17 @@ import { DataloaderInstrumentation } from './vendored/instrumentation'; import type { IntegrationFn } from '@sentry/core'; -import { - defineIntegration, - SEMANTIC_ATTRIBUTE_SENTRY_OP, - SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, - spanToJSON, -} from '@sentry/core'; -import { generateInstrumentOnce, instrumentWhenWrapped } from '@sentry/node-core'; +import { defineIntegration } from '@sentry/core'; +import { generateInstrumentOnce } from '@sentry/node-core'; const INTEGRATION_NAME = 'Dataloader'; -export const instrumentDataloader = generateInstrumentOnce( - INTEGRATION_NAME, - () => - new DataloaderInstrumentation({ - requireParentSpan: true, - }), -); +export const instrumentDataloader = generateInstrumentOnce(INTEGRATION_NAME, () => new DataloaderInstrumentation()); const _dataloaderIntegration = (() => { - let instrumentationWrappedCallback: undefined | ((callback: () => void) => void); - return { name: INTEGRATION_NAME, setupOnce() { - const instrumentation = instrumentDataloader(); - instrumentationWrappedCallback = instrumentWhenWrapped(instrumentation); - }, - - setup(client) { - // This is called either immediately or when the instrumentation is wrapped - instrumentationWrappedCallback?.(() => { - 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. - } - }); - }); + instrumentDataloader(); }, }; }) satisfies IntegrationFn; diff --git a/packages/node/src/integrations/tracing/dataloader/vendored/dataloader-types.ts b/packages/node/src/integrations/tracing/dataloader/vendored/dataloader-types.ts deleted file mode 100644 index 96d4ac9942c4..000000000000 --- a/packages/node/src/integrations/tracing/dataloader/vendored/dataloader-types.ts +++ /dev/null @@ -1,19 +0,0 @@ -/* - * Simplified types inlined from dataloader. - */ - -declare class DataLoader { - constructor(batchLoadFn: DataLoader.BatchLoadFn, options?: any); - load(key: K): Promise; - loadMany(keys: ArrayLike): Promise>; - prime(key: K, value: V | Error): this; - clear(key: K): this; - clearAll(): this; - [key: string]: any; -} - -declare namespace DataLoader { - type BatchLoadFn = (keys: ReadonlyArray) => PromiseLike>; -} - -export = DataLoader; diff --git a/packages/node/src/integrations/tracing/dataloader/vendored/instrumentation.ts b/packages/node/src/integrations/tracing/dataloader/vendored/instrumentation.ts index bfa8bb2b49f7..1fe52e92defd 100644 --- a/packages/node/src/integrations/tracing/dataloader/vendored/instrumentation.ts +++ b/packages/node/src/integrations/tracing/dataloader/vendored/instrumentation.ts @@ -18,20 +18,20 @@ * - Upstream version: @opentelemetry/instrumentation-dataloader@0.35.0 * - Minor TypeScript strictness adjustments for this repository's compiler settings */ -/* eslint-disable */ import { InstrumentationBase, InstrumentationNodeModuleDefinition, isWrapped } from '@opentelemetry/instrumentation'; -import { trace, context, Link, SpanStatusCode, SpanKind } from '@opentelemetry/api'; -import { DataloaderInstrumentationConfig } from './types'; -import { SDK_VERSION } from '@sentry/core'; -import type * as Dataloader from './dataloader-types'; +import { SpanKind } from '@opentelemetry/api'; +import type { DataLoader as Dataloader, DataloaderInstrumentationConfig } from './types'; +import { SDK_VERSION, SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, startSpan } from '@sentry/core'; +import type { SpanLink } from '@sentry/core'; const MODULE_NAME = 'dataloader'; const PACKAGE_NAME = '@sentry/instrumentation-dataloader'; +const ORIGIN = 'auto.db.otel.dataloader'; type DataloaderInternal = typeof Dataloader.prototype & { _batchLoadFn: Dataloader.BatchLoadFn; - _batch: { spanLinks?: Link[] } | null; + _batch: { spanLinks?: SpanLink[] } | null; }; type LoadFn = (typeof Dataloader.prototype)['load']; @@ -47,6 +47,27 @@ function extractModuleExports(module: any) { : module; // CommonJS } +function getSpanName( + dataloader: DataloaderInternal, + operation: 'load' | 'loadMany' | 'batch' | 'prime' | 'clear' | 'clearAll', +): string { + const dataloaderName = dataloader.name; + if (dataloaderName) { + return `${MODULE_NAME}.${operation} ${dataloaderName}`; + } + + return `${MODULE_NAME}.${operation}`; +} + +// `load`, `loadMany` and `batch` are all cache reads +function getSpanOp(operation: 'load' | 'loadMany' | 'batch' | 'prime' | 'clear' | 'clearAll'): string | undefined { + if (operation === 'load' || operation === 'loadMany' || operation === 'batch') { + return 'cache.get'; + } + + return undefined; +} + export class DataloaderInstrumentation extends InstrumentationBase { constructor(config: DataloaderInstrumentationConfig = {}) { super(PACKAGE_NAME, SDK_VERSION, config); @@ -79,64 +100,37 @@ export class DataloaderInstrumentation extends InstrumentationBase, ): Dataloader.BatchLoadFn { + // oxlint-disable-next-line typescript/no-this-alias const instrumentation = this; return function patchedBatchLoadFn( this: DataloaderInternal, ...args: Parameters> ) { - if (!instrumentation.isEnabled() || !instrumentation.shouldCreateSpans()) { + if (!instrumentation.isEnabled()) { return batchLoadFn.call(this, ...args); } - const parent = context.active(); - const span = instrumentation.tracer.startSpan( - instrumentation.getSpanName(this, 'batch'), - { links: this._batch?.spanLinks as Link[] | undefined }, - parent, + return startSpan( + { + name: getSpanName(this, 'batch'), + links: this._batch?.spanLinks, + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: ORIGIN, + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: getSpanOp('batch'), + }, + onlyIfParent: true, + }, + () => batchLoadFn.apply(this, args), ); - - return context.with(trace.setSpan(parent, span), () => { - return (batchLoadFn.apply(this, args) as Promise) - .then(value => { - span.end(); - return value; - }) - .catch((err: Error) => { - span.recordException(err); - span.setStatus({ - code: SpanStatusCode.ERROR, - message: err.message, - }); - span.end(); - throw err; - }); - }); }; } private _getPatchedConstructor(constructor: typeof Dataloader): typeof Dataloader { + // oxlint-disable-next-line typescript/no-this-alias const instrumentation = this; const prototype = constructor.prototype; @@ -152,7 +146,7 @@ export class DataloaderInstrumentation extends InstrumentationBase; + args[0] = instrumentation._wrapBatchLoadFn(args[0]); } return (constructor as any).apply(this, args); @@ -171,49 +165,31 @@ export class DataloaderInstrumentation extends InstrumentationBase) { - if (!instrumentation.shouldCreateSpans()) { - return original.call(this, ...args); - } - - const parent = context.active(); - const span = instrumentation.tracer.startSpan( - instrumentation.getSpanName(this, 'load'), - { kind: SpanKind.CLIENT }, - parent, - ); - - return context.with(trace.setSpan(parent, span), () => { - const result = original - .call(this, ...args) - .then((value: unknown) => { - span.end(); - return value; - }) - .catch((err: Error) => { - span.recordException(err); - span.setStatus({ - code: SpanStatusCode.ERROR, - message: err.message, - }); - span.end(); - throw err; - }); + return startSpan( + { + name: getSpanName(this, 'load'), + kind: SpanKind.CLIENT, + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: ORIGIN, + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: getSpanOp('load'), + }, + onlyIfParent: true, + }, + span => { + const result = original.call(this, ...args); - const loader = this as DataloaderInternal; + if (this._batch) { + if (!this._batch.spanLinks) { + this._batch.spanLinks = []; + } - if (loader._batch) { - if (!loader._batch.spanLinks) { - loader._batch.spanLinks = []; + this._batch.spanLinks.push({ context: span.spanContext() }); } - loader._batch.spanLinks.push({ context: span.spanContext() } as Link); - } - - return result; - }); + return result; + }, + ); }; } @@ -226,28 +202,19 @@ export class DataloaderInstrumentation extends InstrumentationBase) { - if (!instrumentation.shouldCreateSpans()) { - return original.call(this, ...args); - } - - const parent = context.active(); - const span = instrumentation.tracer.startSpan( - instrumentation.getSpanName(this, 'loadMany'), - { kind: SpanKind.CLIENT }, - parent, + return startSpan( + { + name: getSpanName(this, 'loadMany'), + kind: SpanKind.CLIENT, + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: ORIGIN, + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: getSpanOp('loadMany'), + }, + onlyIfParent: true, + }, + () => original.call(this, ...args), ); - - return context.with(trace.setSpan(parent, span), () => { - // .loadMany never rejects, as errors from internal .load - // calls are caught by dataloader lib - return original.call(this, ...args).then((value: unknown) => { - span.end(); - return value; - }); - }); }; } @@ -260,27 +227,19 @@ export class DataloaderInstrumentation extends InstrumentationBase) { - if (!instrumentation.shouldCreateSpans()) { - return original.call(this, ...args); - } - - const parent = context.active(); - const span = instrumentation.tracer.startSpan( - instrumentation.getSpanName(this, 'prime'), - { kind: SpanKind.CLIENT }, - parent, + return startSpan( + { + name: getSpanName(this, 'prime'), + kind: SpanKind.CLIENT, + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: ORIGIN, + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: getSpanOp('prime'), + }, + onlyIfParent: true, + }, + () => original.call(this, ...args), ); - - const ret = context.with(trace.setSpan(parent, span), () => { - return original.call(this, ...args); - }); - - span.end(); - - return ret; }; } @@ -293,27 +252,19 @@ export class DataloaderInstrumentation extends InstrumentationBase) { - if (!instrumentation.shouldCreateSpans()) { - return original.call(this, ...args); - } - - const parent = context.active(); - const span = instrumentation.tracer.startSpan( - instrumentation.getSpanName(this, 'clear'), - { kind: SpanKind.CLIENT }, - parent, + return startSpan( + { + name: getSpanName(this, 'clear'), + kind: SpanKind.CLIENT, + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: ORIGIN, + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: getSpanOp('clear'), + }, + onlyIfParent: true, + }, + () => original.call(this, ...args), ); - - const ret = context.with(trace.setSpan(parent, span), () => { - return original.call(this, ...args); - }); - - span.end(); - - return ret; }; } @@ -326,27 +277,19 @@ export class DataloaderInstrumentation extends InstrumentationBase) { - if (!instrumentation.shouldCreateSpans()) { - return original.call(this, ...args); - } - - const parent = context.active(); - const span = instrumentation.tracer.startSpan( - instrumentation.getSpanName(this, 'clearAll'), - { kind: SpanKind.CLIENT }, - parent, + return startSpan( + { + name: getSpanName(this, 'clearAll'), + kind: SpanKind.CLIENT, + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: ORIGIN, + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: getSpanOp('clearAll'), + }, + onlyIfParent: true, + }, + () => original.call(this, ...args), ); - - const ret = context.with(trace.setSpan(parent, span), () => { - return original.call(this, ...args); - }); - - span.end(); - - return ret; }; } } diff --git a/packages/node/src/integrations/tracing/dataloader/vendored/types.ts b/packages/node/src/integrations/tracing/dataloader/vendored/types.ts index 6a97d980b619..f5f58c4cd400 100644 --- a/packages/node/src/integrations/tracing/dataloader/vendored/types.ts +++ b/packages/node/src/integrations/tracing/dataloader/vendored/types.ts @@ -20,11 +20,23 @@ import type { InstrumentationConfig } from '@opentelemetry/instrumentation'; -export interface DataloaderInstrumentationConfig extends InstrumentationConfig { - /** - * Whether the instrumentation requires a parent span, if set to true - * and there is no parent span, no additional spans are created - * (default: true) - */ - requireParentSpan?: boolean; +export type DataloaderInstrumentationConfig = InstrumentationConfig; + +/* Simplified types inlined from dataloader. */ +// oxlint-disable-next-line typescript/no-explicit-any +export declare class DataLoader { + public constructor(batchLoadFn: DataLoader.BatchLoadFn, options?: any); + public load(key: K): Promise; + public loadMany(keys: ArrayLike): Promise>; + public prime(key: K, value: V | Error): this; + public clear(key: K): this; + public clearAll(): this; + public name: string | undefined; + // oxlint-disable-next-line typescript/no-explicit-any + [key: string]: any; +} + +// eslint-disable-next-line @typescript-eslint/no-namespace +export declare namespace DataLoader { + type BatchLoadFn = (keys: ReadonlyArray) => PromiseLike>; } From 7c1e212132965e57e424fd02c6a35033a6b0e928 Mon Sep 17 00:00:00 2001 From: Francesco Gringl-Novy Date: Thu, 11 Jun 2026 17:13:04 +0200 Subject: [PATCH 2/5] fixes and improvements --- .../dataloader/vendored/instrumentation.ts | 75 +++++++++---------- .../tracing/dataloader/vendored/types.ts | 34 +++++---- 2 files changed, 55 insertions(+), 54 deletions(-) diff --git a/packages/node/src/integrations/tracing/dataloader/vendored/instrumentation.ts b/packages/node/src/integrations/tracing/dataloader/vendored/instrumentation.ts index 1fe52e92defd..7f33919b8932 100644 --- a/packages/node/src/integrations/tracing/dataloader/vendored/instrumentation.ts +++ b/packages/node/src/integrations/tracing/dataloader/vendored/instrumentation.ts @@ -21,34 +21,31 @@ import { InstrumentationBase, InstrumentationNodeModuleDefinition, isWrapped } from '@opentelemetry/instrumentation'; import { SpanKind } from '@opentelemetry/api'; -import type { DataLoader as Dataloader, DataloaderInstrumentationConfig } from './types'; +import type { BatchLoadFn, DataLoader, DataLoaderConstructor } from './types'; import { SDK_VERSION, SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, startSpan } from '@sentry/core'; -import type { SpanLink } from '@sentry/core'; const MODULE_NAME = 'dataloader'; const PACKAGE_NAME = '@sentry/instrumentation-dataloader'; const ORIGIN = 'auto.db.otel.dataloader'; -type DataloaderInternal = typeof Dataloader.prototype & { - _batchLoadFn: Dataloader.BatchLoadFn; - _batch: { spanLinks?: SpanLink[] } | null; -}; +type LoadFn = DataLoader['load']; +type LoadManyFn = DataLoader['loadMany']; +type PrimeFn = DataLoader['prime']; +type ClearFn = DataLoader['clear']; +type ClearAllFn = DataLoader['clearAll']; -type LoadFn = (typeof Dataloader.prototype)['load']; -type LoadManyFn = (typeof Dataloader.prototype)['loadMany']; -type PrimeFn = (typeof Dataloader.prototype)['prime']; -type ClearFn = (typeof Dataloader.prototype)['clear']; -type ClearAllFn = (typeof Dataloader.prototype)['clearAll']; +function isModule(module: unknown): module is { [Symbol.toStringTag]: 'Module'; default: DataLoaderConstructor } { + return (module as { [Symbol.toStringTag]: string })[Symbol.toStringTag] === 'Module'; +} -// eslint-disable-next-line @typescript-eslint/no-explicit-any -function extractModuleExports(module: any) { - return module[Symbol.toStringTag] === 'Module' +function extractModuleExports(module: any): DataLoaderConstructor { + return isModule(module) ? module.default // ESM - : module; // CommonJS + : (module as DataLoaderConstructor); // CommonJS } function getSpanName( - dataloader: DataloaderInternal, + dataloader: DataLoader, operation: 'load' | 'loadMany' | 'batch' | 'prime' | 'clear' | 'clearAll', ): string { const dataloaderName = dataloader.name; @@ -68,8 +65,8 @@ function getSpanOp(operation: 'load' | 'loadMany' | 'batch' | 'prime' | 'clear' return undefined; } -export class DataloaderInstrumentation extends InstrumentationBase { - constructor(config: DataloaderInstrumentationConfig = {}) { +export class DataloaderInstrumentation extends InstrumentationBase { + constructor(config = {}) { super(PACKAGE_NAME, SDK_VERSION, config); } @@ -100,16 +97,11 @@ export class DataloaderInstrumentation extends InstrumentationBase, - ): Dataloader.BatchLoadFn { + private _wrapBatchLoadFn(batchLoadFn: BatchLoadFn): BatchLoadFn { // oxlint-disable-next-line typescript/no-this-alias const instrumentation = this; - return function patchedBatchLoadFn( - this: DataloaderInternal, - ...args: Parameters> - ) { + return function patchedBatchLoadFn(this: DataLoader, ...args: Parameters>) { if (!instrumentation.isEnabled()) { return batchLoadFn.call(this, ...args); } @@ -129,7 +121,7 @@ export class DataloaderInstrumentation extends InstrumentationBase) { + return function patchedLoad(this: DataLoader, ...args: Parameters) { return startSpan( { name: getSpanName(this, 'load'), @@ -193,7 +186,8 @@ export class DataloaderInstrumentation extends InstrumentationBase) { + return function patchedLoadMany(this: DataLoader, ...args: Parameters) { return startSpan( { name: getSpanName(this, 'loadMany'), @@ -218,7 +212,8 @@ export class DataloaderInstrumentation extends InstrumentationBase) { + return function patchedPrime(this: DataLoader, ...args: Parameters) { return startSpan( { name: getSpanName(this, 'prime'), @@ -243,7 +238,8 @@ export class DataloaderInstrumentation extends InstrumentationBase) { + return function patchedClear(this: DataLoader, ...args: Parameters) { return startSpan( { name: getSpanName(this, 'clear'), @@ -268,7 +264,8 @@ export class DataloaderInstrumentation extends InstrumentationBase) { + return function patchedClearAll(this: DataLoader, ...args: Parameters) { return startSpan( { name: getSpanName(this, 'clearAll'), diff --git a/packages/node/src/integrations/tracing/dataloader/vendored/types.ts b/packages/node/src/integrations/tracing/dataloader/vendored/types.ts index f5f58c4cd400..c497288149a7 100644 --- a/packages/node/src/integrations/tracing/dataloader/vendored/types.ts +++ b/packages/node/src/integrations/tracing/dataloader/vendored/types.ts @@ -18,25 +18,29 @@ * - Upstream version: @opentelemetry/instrumentation-dataloader@0.35.0 */ -import type { InstrumentationConfig } from '@opentelemetry/instrumentation'; - -export type DataloaderInstrumentationConfig = InstrumentationConfig; +import type { SpanLink } from '@sentry/core'; /* Simplified types inlined from dataloader. */ -// oxlint-disable-next-line typescript/no-explicit-any -export declare class DataLoader { - public constructor(batchLoadFn: DataLoader.BatchLoadFn, options?: any); - public load(key: K): Promise; - public loadMany(keys: ArrayLike): Promise>; - public prime(key: K, value: V | Error): this; - public clear(key: K): this; - public clearAll(): this; - public name: string | undefined; + +export type BatchLoadFn = (keys: ReadonlyArray) => PromiseLike>; + +/** A `DataLoader` instance. */ +export interface DataLoader { + _batchLoadFn: BatchLoadFn; + _batch: { spanLinks?: SpanLink[] } | null; + load(key: K): Promise; + loadMany(keys: ArrayLike): Promise>; + prime(key: K, value: V | Error): this; + clear(key: K): this; + clearAll(): this; + name: string | undefined; // oxlint-disable-next-line typescript/no-explicit-any [key: string]: any; } -// eslint-disable-next-line @typescript-eslint/no-namespace -export declare namespace DataLoader { - type BatchLoadFn = (keys: ReadonlyArray) => PromiseLike>; +/** The `DataLoader` class/constructor. */ +export interface DataLoaderConstructor { + // oxlint-disable-next-line typescript/no-explicit-any + new (batchLoadFn: BatchLoadFn, options?: any): DataLoader; + prototype: DataLoader; } From 8cbbcfa00e7ce3c2df0106034ba76cde7e9bbc1b Mon Sep 17 00:00:00 2001 From: Francesco Gringl-Novy Date: Fri, 12 Jun 2026 09:32:56 +0200 Subject: [PATCH 3/5] only link active spans --- .../integrations/tracing/dataloader/vendored/instrumentation.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/node/src/integrations/tracing/dataloader/vendored/instrumentation.ts b/packages/node/src/integrations/tracing/dataloader/vendored/instrumentation.ts index 7f33919b8932..7a9a2b05ac02 100644 --- a/packages/node/src/integrations/tracing/dataloader/vendored/instrumentation.ts +++ b/packages/node/src/integrations/tracing/dataloader/vendored/instrumentation.ts @@ -172,7 +172,7 @@ export class DataloaderInstrumentation extends InstrumentationBase { span => { const result = original.call(this, ...args); - if (this._batch) { + if (this._batch && span.isRecording()) { if (!this._batch.spanLinks) { this._batch.spanLinks = []; } From 36592a28a319c4eb6229d44fe9d173f49140f28a Mon Sep 17 00:00:00 2001 From: Francesco Gringl-Novy Date: Fri, 12 Jun 2026 09:51:28 +0200 Subject: [PATCH 4/5] fix linting --- .oxlintrc.base.json | 1 - .../integrations/tracing/dataloader/vendored/instrumentation.ts | 2 ++ 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/.oxlintrc.base.json b/.oxlintrc.base.json index 3f7dfb18900e..4e8d59a39db9 100644 --- a/.oxlintrc.base.json +++ b/.oxlintrc.base.json @@ -142,7 +142,6 @@ }, { "files": [ - "**/integrations/tracing/dataloader/vendored/**/*.ts", "**/integrations/fs/vendored/**/*.ts", "**/integrations/tracing/knex/vendored/**/*.ts", "**/integrations/tracing/mongo/vendored/**/*.ts", diff --git a/packages/node/src/integrations/tracing/dataloader/vendored/instrumentation.ts b/packages/node/src/integrations/tracing/dataloader/vendored/instrumentation.ts index 7a9a2b05ac02..d964414d8d89 100644 --- a/packages/node/src/integrations/tracing/dataloader/vendored/instrumentation.ts +++ b/packages/node/src/integrations/tracing/dataloader/vendored/instrumentation.ts @@ -38,6 +38,7 @@ function isModule(module: unknown): module is { [Symbol.toStringTag]: 'Module'; return (module as { [Symbol.toStringTag]: string })[Symbol.toStringTag] === 'Module'; } +// oxlint-disable-next-line typescript/no-explicit-any function extractModuleExports(module: any): DataLoaderConstructor { return isModule(module) ? module.default // ESM @@ -130,6 +131,7 @@ export class DataloaderInstrumentation extends InstrumentationBase { return constructor; } + // oxlint-disable-next-line typescript/no-explicit-any function PatchedDataloader(this: DataLoader, ...args: any[]) { // BatchLoadFn is the first constructor argument // https://github.com/graphql/dataloader/blob/77c2cd7ca97e8795242018ebc212ce2487e729d2/src/index.js#L47 From f61397884c3489558ee4f89571eee01b6704125e Mon Sep 17 00:00:00 2001 From: Francesco Gringl-Novy Date: Mon, 15 Jun 2026 10:31:52 +0200 Subject: [PATCH 5/5] use unknown --- .../tracing/dataloader/vendored/instrumentation.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/node/src/integrations/tracing/dataloader/vendored/instrumentation.ts b/packages/node/src/integrations/tracing/dataloader/vendored/instrumentation.ts index d964414d8d89..6097139998cf 100644 --- a/packages/node/src/integrations/tracing/dataloader/vendored/instrumentation.ts +++ b/packages/node/src/integrations/tracing/dataloader/vendored/instrumentation.ts @@ -38,8 +38,7 @@ function isModule(module: unknown): module is { [Symbol.toStringTag]: 'Module'; return (module as { [Symbol.toStringTag]: string })[Symbol.toStringTag] === 'Module'; } -// oxlint-disable-next-line typescript/no-explicit-any -function extractModuleExports(module: any): DataLoaderConstructor { +function extractModuleExports(module: unknown): DataLoaderConstructor { return isModule(module) ? module.default // ESM : (module as DataLoaderConstructor); // CommonJS