From 823281b0c0bdb7d0155da069b05dd728ee512ee7 Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Tue, 24 Sep 2024 13:14:37 +0300 Subject: [PATCH 1/2] feat(instrumentation-dataloader): Enhance `dataloader` instrumentation. --- .../src/instrumentation.ts | 159 +++++++- .../test/dataloader.test.ts | 379 +++++++++++++++++- 2 files changed, 506 insertions(+), 32 deletions(-) diff --git a/plugins/node/instrumentation-dataloader/src/instrumentation.ts b/plugins/node/instrumentation-dataloader/src/instrumentation.ts index 53bc18a6b2..34c4638e6a 100644 --- a/plugins/node/instrumentation-dataloader/src/instrumentation.ts +++ b/plugins/node/instrumentation-dataloader/src/instrumentation.ts @@ -35,13 +35,13 @@ const MODULE_NAME = 'dataloader'; type DataloaderInternal = typeof Dataloader.prototype & { _batchLoadFn: Dataloader.BatchLoadFn; _batch: { spanLinks?: Link[] } | null; - - // TODO: Remove this once types on Dataloader get fixed https://github.com/graphql/dataloader/pull/334 - name?: string | null; }; 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']; export class DataloaderInstrumentation extends InstrumentationBase { constructor(config: DataloaderInstrumentationConfig = {}) { @@ -56,17 +56,18 @@ export class DataloaderInstrumentation extends InstrumentationBase { this._patchLoad(dataloader.prototype); this._patchLoadMany(dataloader.prototype); + this._patchPrime(dataloader.prototype); + this._patchClear(dataloader.prototype); + this._patchClearAll(dataloader.prototype); return this._getPatchedConstructor(dataloader); }, dataloader => { - if (isWrapped(dataloader.prototype.load)) { - this._unwrap(dataloader.prototype, 'load'); - } - - if (isWrapped(dataloader.prototype.loadMany)) { - this._unwrap(dataloader.prototype, 'loadMany'); - } + ['load', 'loadMany', 'prime', 'clear', 'clearAll'].forEach(method => { + if (isWrapped(dataloader.prototype[method])) { + this._unwrap(dataloader.prototype, method); + } + }); } ) as InstrumentationNodeModuleDefinition, ]; @@ -80,7 +81,7 @@ export class DataloaderInstrumentation extends InstrumentationBase { + span.setAttribute('cache.key', [args[0]]); + span.setAttribute('cache.hit', value !== undefined); + span.setAttribute( + 'cache.item_size', + value ? JSON.stringify(value).length : 0 + ); + span.end(); return value; }) @@ -242,10 +250,139 @@ export class DataloaderInstrumentation extends InstrumentationBase { + span.setAttribute('cache.key', Array.from(args[0])); + span.setAttribute( + 'cache.hit', + value.every((v: unknown) => v !== undefined) + ); + span.setAttribute( + 'cache.item_size', + value.reduce( + (acc: number, v: unknown) => acc + JSON.stringify(v).length, + 0 + ) + ); + span.end(); return value; }); }); }; } + + private _patchPrime(proto: typeof Dataloader.prototype) { + if (isWrapped(proto.prime)) { + this._unwrap(proto, 'prime'); + } + + this._wrap(proto, 'prime', this._getPatchedPrime.bind(this)); + } + + private _getPatchedPrime(original: PrimeFn): PrimeFn { + const instrumentation = this; + + return function patchedPrime( + this: DataloaderInternal, + ...args: Parameters + ) { + 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 + ); + + const ret = context.with(trace.setSpan(parent, span), () => { + return original.call(this, ...args); + }); + + span.setAttribute('cache.key', [args[0]]); + span.setAttribute( + 'cache.item_size', + args[1] ? JSON.stringify(args[1]).length : 0 + ); + + span.end(); + + return ret; + }; + } + + private _patchClear(proto: typeof Dataloader.prototype) { + if (isWrapped(proto.clear)) { + this._unwrap(proto, 'clear'); + } + + this._wrap(proto, 'clear', this._getPatchedClear.bind(this)); + } + + private _getPatchedClear(original: ClearFn): ClearFn { + const instrumentation = this; + + return function patchedClear( + this: DataloaderInternal, + ...args: Parameters + ) { + 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 + ); + + const ret = context.with(trace.setSpan(parent, span), () => { + span.setAttribute('cache.key', [args[0]]); + + return original.call(this, ...args); + }); + + span.end(); + + return ret; + }; + } + + private _patchClearAll(proto: typeof Dataloader.prototype) { + if (isWrapped(proto.clearAll)) { + this._unwrap(proto, 'clearAll'); + } + + this._wrap(proto, 'clearAll', this._getPatchedClearAll.bind(this)); + } + + private _getPatchedClearAll(original: ClearAllFn): ClearAllFn { + const instrumentation = this; + + return function patchedClearAll( + this: DataloaderInternal, + ...args: Parameters + ) { + 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 + ); + + const ret = context.with(trace.setSpan(parent, span), () => { + return original.call(this, ...args); + }); + + span.end(); + + return ret; + }; + } } diff --git a/plugins/node/instrumentation-dataloader/test/dataloader.test.ts b/plugins/node/instrumentation-dataloader/test/dataloader.test.ts index 9686c21d2e..67a295b604 100644 --- a/plugins/node/instrumentation-dataloader/test/dataloader.test.ts +++ b/plugins/node/instrumentation-dataloader/test/dataloader.test.ts @@ -31,9 +31,14 @@ extraInstrumentation.disable(); import * as assert from 'assert'; import * as Dataloader from 'dataloader'; +import * as crypto from 'crypto'; + +function getMd5HashFromIdx(idx: number) { + return crypto.createHash('md5').update(String(idx)).digest('hex'); +} describe('DataloaderInstrumentation', () => { - let dataloader: Dataloader; + let dataloader: Dataloader; let contextManager: AsyncHooksContextManager; const memoryExporter = new InMemorySpanExporter(); @@ -48,9 +53,14 @@ describe('DataloaderInstrumentation', () => { instrumentation.enable(); contextManager = new AsyncHooksContextManager(); context.setGlobalContextManager(contextManager.enable()); - dataloader = new Dataloader(async keys => keys.map((_, idx) => idx), { - cache: false, - }); + dataloader = new Dataloader( + async keys => + keys.map((_, idx) => { + return getMd5HashFromIdx(idx); + }), + { cache: true } + ); + assert.strictEqual(memoryExporter.getFinishedSpans().length, 0); }); @@ -64,7 +74,7 @@ describe('DataloaderInstrumentation', () => { describe('load', () => { it('creates a span', async () => { - assert.strictEqual(await dataloader.load('test'), 0); + assert.strictEqual(await dataloader.load('test'), getMd5HashFromIdx(0)); // We should have exactly two spans (one for .load and one for the following batch) assert.strictEqual(memoryExporter.getFinishedSpans().length, 2); @@ -87,7 +97,10 @@ describe('DataloaderInstrumentation', () => { await context.with( trace.setSpan(context.active(), rootSpan), async () => { - assert.strictEqual(await dataloader.load('test'), 0); + assert.strictEqual( + await dataloader.load('test'), + getMd5HashFromIdx(0) + ); const [_, loadSpan] = memoryExporter.getFinishedSpans(); assert.strictEqual( @@ -105,7 +118,10 @@ describe('DataloaderInstrumentation', () => { await context.with( trace.setSpan(context.active(), rootSpan), async () => { - assert.strictEqual(await dataloader.load('test'), 0); + assert.strictEqual( + await dataloader.load('test'), + getMd5HashFromIdx(0) + ); const [_, loadSpan] = memoryExporter.getFinishedSpans(); assert.strictEqual( @@ -127,14 +143,19 @@ describe('DataloaderInstrumentation', () => { } catch (e) {} // All spans should be finished, both load as well as the batch ones should have errored - assert.strictEqual(memoryExporter.getFinishedSpans().length, 2); - const [batchSpan, loadSpan] = memoryExporter.getFinishedSpans(); + assert.strictEqual(memoryExporter.getFinishedSpans().length, 3); + const [batchSpan, clearSpan, loadSpan] = + memoryExporter.getFinishedSpans(); assert.deepStrictEqual(loadSpan.status, { code: SpanStatusCode.ERROR, message: 'Error message', }); + assert.deepStrictEqual(clearSpan.status, { + code: SpanStatusCode.UNSET, + }); + assert.deepStrictEqual(batchSpan.status, { code: SpanStatusCode.ERROR, message: 'Error message', @@ -163,11 +184,45 @@ describe('DataloaderInstrumentation', () => { assert.strictEqual(batchSpan.name, 'dataloader.batch test-name'); } }); + + it('correctly sets cache.key, cache.hit and cache.item_size attributes when data is available', async () => { + assert.strictEqual(await dataloader.load('test'), getMd5HashFromIdx(0)); + + // We should have exactly two spans (one for .load and one for the batch) + assert.strictEqual(memoryExporter.getFinishedSpans().length, 2); + const [_batchSpan, loadSpan] = memoryExporter.getFinishedSpans(); + + assert.deepStrictEqual(loadSpan.attributes, { + 'cache.key': ['test'], + 'cache.hit': true, + 'cache.item_size': 34, + }); + }); + + it('correctly sets cache.key, cache.hit and cache.item_size attributes when data is not available', async () => { + const namedDataloader = new Dataloader(async keys => { + return keys.map(() => undefined); + }); + + assert.strictEqual(await namedDataloader.load('unavailable'), undefined); + + // We should have exactly two spans (one for .load and one for the batch) + assert.strictEqual(memoryExporter.getFinishedSpans().length, 2); + const [_batchSpan, loadSpan] = memoryExporter.getFinishedSpans(); + + assert.deepStrictEqual(loadSpan.attributes, { + 'cache.key': ['unavailable'], + 'cache.hit': false, + 'cache.item_size': 0, + }); + }); }); describe('loadMany', () => { it('creates an additional span', async () => { - assert.deepStrictEqual(await dataloader.loadMany(['test']), [0]); + assert.deepStrictEqual(await dataloader.loadMany(['test']), [ + getMd5HashFromIdx(0), + ]); // We should have exactly three spans (one for .loadMany, one for the underlying .load // and one for the following batch) @@ -198,7 +253,9 @@ describe('DataloaderInstrumentation', () => { await context.with( trace.setSpan(context.active(), rootSpan), async () => { - assert.deepStrictEqual(await dataloader.loadMany(['test']), [0]); + assert.deepStrictEqual(await dataloader.loadMany(['test']), [ + getMd5HashFromIdx(0), + ]); const [, , loadManySpan] = memoryExporter.getFinishedSpans(); assert.strictEqual( @@ -216,7 +273,9 @@ describe('DataloaderInstrumentation', () => { await context.with( trace.setSpan(context.active(), rootSpan), async () => { - assert.deepStrictEqual(await dataloader.loadMany(['test']), [0]); + assert.deepStrictEqual(await dataloader.loadMany(['test']), [ + getMd5HashFromIdx(0), + ]); const [, , loadManySpan] = memoryExporter.getFinishedSpans(); assert.strictEqual( @@ -240,8 +299,8 @@ describe('DataloaderInstrumentation', () => { // All spans should be finished, both load as well as the batch ones should have errored // but loadMany one should not have errored - assert.strictEqual(memoryExporter.getFinishedSpans().length, 3); - const [batchSpan, loadSpan, loadManySpan] = + assert.strictEqual(memoryExporter.getFinishedSpans().length, 4); + const [batchSpan, clearSpan, loadSpan, loadManySpan] = memoryExporter.getFinishedSpans(); assert.deepStrictEqual(loadSpan.status, { @@ -257,6 +316,10 @@ describe('DataloaderInstrumentation', () => { assert.deepStrictEqual(loadManySpan.status, { code: SpanStatusCode.UNSET, }); + + assert.deepStrictEqual(clearSpan.status, { + code: SpanStatusCode.UNSET, + }); }); it('correctly uses a generated name in spans', async () => { @@ -287,11 +350,271 @@ describe('DataloaderInstrumentation', () => { }); }); + describe('clear', () => { + it('creates a span', async () => { + dataloader.clear('test'); + + // We should have exactly one span + assert.strictEqual(memoryExporter.getFinishedSpans().length, 1); + const [clearSpan] = memoryExporter.getFinishedSpans(); + + assert.strictEqual(clearSpan.name, 'dataloader.clear'); + assert.strictEqual(clearSpan.kind, SpanKind.CLIENT); + }); + + it('attaches span to parent', async () => { + const rootSpan: any = tracer.startSpan('root'); + + await context.with( + trace.setSpan(context.active(), rootSpan), + async () => { + dataloader.clear('test'); + + const [clearSpan] = memoryExporter.getFinishedSpans(); + assert.strictEqual( + clearSpan.parentSpanId, + rootSpan.spanContext().spanId + ); + } + ); + }); + + it('attaches span to parent with required parent', async () => { + instrumentation.setConfig({ requireParentSpan: true }); + const rootSpan: any = tracer.startSpan('root'); + + await context.with( + trace.setSpan(context.active(), rootSpan), + async () => { + dataloader.clear('test'); + + const [clearSpan] = memoryExporter.getFinishedSpans(); + assert.strictEqual( + clearSpan.parentSpanId, + rootSpan.spanContext().spanId + ); + } + ); + }); + + it('never errors', async () => { + dataloader.clear('test'); + + // All spans should be finished, but none should have errored + assert.strictEqual(memoryExporter.getFinishedSpans().length, 1); + const [clearSpan] = memoryExporter.getFinishedSpans(); + + assert.deepStrictEqual(clearSpan.status, { + code: SpanStatusCode.UNSET, + }); + }); + + it('correctly uses dataloader name (if available)', async () => { + const namedDataloader = new Dataloader( + async keys => keys.map((_, idx) => idx), + { name: 'test-name' } + ); + + namedDataloader.clear('test'); + + // We should have exactly one span + assert.strictEqual(memoryExporter.getFinishedSpans().length, 1); + const [clearSpan] = memoryExporter.getFinishedSpans(); + + if ((namedDataloader as { name?: string | null }).name === undefined) { + // For versions of dataloader + // package that does not support name, we should not be adding anything to the names + assert.strictEqual(clearSpan.name, 'dataloader.clear'); + } else { + assert.strictEqual(clearSpan.name, 'dataloader.clear test-name'); + } + }); + + it('correctly sets attributes for clearing', async () => { + dataloader.clear('test'); + + // We should have exactly one span + assert.strictEqual(memoryExporter.getFinishedSpans().length, 1); + const [clearSpan] = memoryExporter.getFinishedSpans(); + + assert.deepStrictEqual(clearSpan.attributes, { + 'cache.key': ['test'], + }); + }); + }); + + describe('clearAll', () => { + it('creates a span', async () => { + dataloader.clearAll(); + + // We should have exactly one span + assert.strictEqual(memoryExporter.getFinishedSpans().length, 1); + const [clearSpan] = memoryExporter.getFinishedSpans(); + + assert.strictEqual(clearSpan.name, 'dataloader.clearAll'); + assert.strictEqual(clearSpan.kind, SpanKind.CLIENT); + }); + + it('attaches span to parent', async () => { + const rootSpan: any = tracer.startSpan('root'); + + await context.with( + trace.setSpan(context.active(), rootSpan), + async () => { + dataloader.clearAll(); + + const [clearSpan] = memoryExporter.getFinishedSpans(); + assert.strictEqual( + clearSpan.parentSpanId, + rootSpan.spanContext().spanId + ); + } + ); + }); + + it('attaches span to parent with required parent', async () => { + instrumentation.setConfig({ requireParentSpan: true }); + const rootSpan: any = tracer.startSpan('root'); + + await context.with( + trace.setSpan(context.active(), rootSpan), + async () => { + dataloader.clearAll(); + + const [clearSpan] = memoryExporter.getFinishedSpans(); + assert.strictEqual( + clearSpan.parentSpanId, + rootSpan.spanContext().spanId + ); + } + ); + }); + + it('never errors', async () => { + dataloader.clearAll(); + + // All spans should be finished, but none should have errored + assert.strictEqual(memoryExporter.getFinishedSpans().length, 1); + const [clearSpan] = memoryExporter.getFinishedSpans(); + + assert.deepStrictEqual(clearSpan.status, { + code: SpanStatusCode.UNSET, + }); + }); + }); + + describe('prime', () => { + it('creates a span', async () => { + dataloader.prime('test', '1'); + + // We should have exactly one span + assert.strictEqual(memoryExporter.getFinishedSpans().length, 1); + const [primeSpan] = memoryExporter.getFinishedSpans(); + + assert.strictEqual(primeSpan.name, 'dataloader.prime'); + assert.strictEqual(primeSpan.kind, SpanKind.CLIENT); + }); + + it('attaches span to parent', async () => { + const rootSpan: any = tracer.startSpan('root'); + + await context.with( + trace.setSpan(context.active(), rootSpan), + async () => { + dataloader.prime('test', '1'); + + const [primeSpan] = memoryExporter.getFinishedSpans(); + assert.strictEqual( + primeSpan.parentSpanId, + rootSpan.spanContext().spanId + ); + } + ); + }); + + it('attaches span to parent with required parent', async () => { + instrumentation.setConfig({ requireParentSpan: true }); + const rootSpan: any = tracer.startSpan('root'); + + await context.with( + trace.setSpan(context.active(), rootSpan), + async () => { + dataloader.prime('test', '1'); + + const [primeSpan] = memoryExporter.getFinishedSpans(); + assert.strictEqual( + primeSpan.parentSpanId, + rootSpan.spanContext().spanId + ); + } + ); + }); + + it('never errors', async () => { + dataloader.prime('test', '1'); + + // All spans should be finished, but none should have errored + assert.strictEqual(memoryExporter.getFinishedSpans().length, 1); + const [primeSpan] = memoryExporter.getFinishedSpans(); + + assert.deepStrictEqual(primeSpan.status, { + code: SpanStatusCode.UNSET, + }); + }); + + it('correctly uses dataloader name (if available)', async () => { + const namedDataloader = new Dataloader( + async keys => keys.map((_, idx) => idx), + { name: 'test-name' } + ); + + namedDataloader.prime('test', 1); + + // We should have exactly one span + assert.strictEqual(memoryExporter.getFinishedSpans().length, 1); + const [primeSpan] = memoryExporter.getFinishedSpans(); + + if ((namedDataloader as { name?: string | null }).name === undefined) { + // For versions of dataloader + // package that does not support name, we should not be adding anything to the names + assert.strictEqual(primeSpan.name, 'dataloader.prime'); + } else { + assert.strictEqual(primeSpan.name, 'dataloader.prime test-name'); + } + }); + + it('correctly creates spans for chained priming', async () => { + dataloader.prime('test', '1').prime('test2', '2'); + + // We should have exactly two spans + assert.strictEqual(memoryExporter.getFinishedSpans().length, 2); + const [primeSpan1, primeSpan2] = memoryExporter.getFinishedSpans(); + + assert.strictEqual(primeSpan1.name, 'dataloader.prime'); + assert.strictEqual(primeSpan2.name, 'dataloader.prime'); + }); + + it('correctly creates attributes for priming', async () => { + dataloader.prime('test', '1'); + + // We should have exactly one span + assert.strictEqual(memoryExporter.getFinishedSpans().length, 1); + const [primeSpan] = memoryExporter.getFinishedSpans(); + + assert.deepStrictEqual(primeSpan.attributes, { + 'cache.key': ['test'], + 'cache.item_size': 3, + }); + }); + }); + it('should not create anything if disabled', async () => { instrumentation.disable(); - assert.strictEqual(await dataloader.load('test'), 0); - assert.deepStrictEqual(await dataloader.loadMany(['test']), [0]); + assert.strictEqual(await dataloader.load('test'), getMd5HashFromIdx(0)); + assert.deepStrictEqual(await dataloader.loadMany(['test']), [ + getMd5HashFromIdx(0), + ]); assert.strictEqual(memoryExporter.getFinishedSpans().length, 0); // Same goes for any new dataloaders that are created while the instrumentation is disabled @@ -301,14 +624,25 @@ describe('DataloaderInstrumentation', () => { ); assert.strictEqual(await alternativeDataloader.load('test'), 1); assert.deepStrictEqual(await alternativeDataloader.loadMany(['test']), [1]); + assert.strictEqual(alternativeDataloader.clearAll(), alternativeDataloader); + assert.strictEqual( + alternativeDataloader.clear('test'), + alternativeDataloader + ); + assert.strictEqual( + alternativeDataloader.prime('test', 1), + alternativeDataloader + ); assert.strictEqual(memoryExporter.getFinishedSpans().length, 0); }); it('should not create anything if parent span is required, but missing', async () => { instrumentation.setConfig({ requireParentSpan: true }); - assert.strictEqual(await dataloader.load('test'), 0); - assert.deepStrictEqual(await dataloader.loadMany(['test']), [0]); + assert.strictEqual(await dataloader.load('test'), getMd5HashFromIdx(0)); + assert.deepStrictEqual(await dataloader.loadMany(['test']), [ + getMd5HashFromIdx(0), + ]); assert.strictEqual(memoryExporter.getFinishedSpans().length, 0); }); @@ -316,11 +650,14 @@ describe('DataloaderInstrumentation', () => { extraInstrumentation.enable(); // Dataloader created prior to the extra instrumentation - assert.strictEqual(await dataloader.load('test'), 0); + assert.strictEqual(await dataloader.load('test'), getMd5HashFromIdx(0)); assert.strictEqual(memoryExporter.getFinishedSpans().length, 2); - assert.deepStrictEqual(await dataloader.loadMany(['test']), [0]); - assert.strictEqual(memoryExporter.getFinishedSpans().length, 5); + assert.deepStrictEqual(await dataloader.loadMany(['test']), [ + getMd5HashFromIdx(0), + ]); + + assert.strictEqual(memoryExporter.getFinishedSpans().length, 4); memoryExporter.reset(); // Same goes for any new dataloaders that are created after the extra instrumentation is added From 904a05b5784b5c76a5c53601084fcdfaa3bc6acc Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Thu, 26 Sep 2024 16:26:35 +0300 Subject: [PATCH 2/2] Remove attached cache attributes. --- package-lock.json | 8 +-- .../src/instrumentation.ts | 28 --------- .../test/dataloader.test.ts | 57 ------------------- 3 files changed, 4 insertions(+), 89 deletions(-) diff --git a/package-lock.json b/package-lock.json index 820a1917ca..808393bac3 100644 --- a/package-lock.json +++ b/package-lock.json @@ -42,7 +42,7 @@ "version": "0.29.1", "license": "Apache-2.0", "dependencies": { - "@opentelemetry/resources": "^1.0.0", + "@opentelemetry/resources": "^1.10.0", "@opentelemetry/semantic-conventions": "^1.27.0" }, "devDependencies": { @@ -185,7 +185,7 @@ "license": "Apache-2.0", "dependencies": { "@opentelemetry/core": "^1.0.0", - "@opentelemetry/resources": "^1.0.0", + "@opentelemetry/resources": "^1.10.0", "@opentelemetry/semantic-conventions": "^1.27.0", "gcp-metadata": "^6.0.0" }, @@ -53822,7 +53822,7 @@ "requires": { "@opentelemetry/api": "^1.0.0", "@opentelemetry/contrib-test-utils": "^0.41.0", - "@opentelemetry/resources": "^1.0.0", + "@opentelemetry/resources": "^1.10.0", "@opentelemetry/semantic-conventions": "^1.27.0", "@types/mocha": "8.2.3", "@types/node": "18.6.5", @@ -53933,7 +53933,7 @@ "@opentelemetry/api": "^1.0.0", "@opentelemetry/contrib-test-utils": "^0.41.0", "@opentelemetry/core": "^1.0.0", - "@opentelemetry/resources": "^1.0.0", + "@opentelemetry/resources": "^1.10.0", "@opentelemetry/semantic-conventions": "^1.27.0", "@types/mocha": "8.2.3", "@types/node": "18.6.5", diff --git a/plugins/node/instrumentation-dataloader/src/instrumentation.ts b/plugins/node/instrumentation-dataloader/src/instrumentation.ts index 34c4638e6a..57020adf07 100644 --- a/plugins/node/instrumentation-dataloader/src/instrumentation.ts +++ b/plugins/node/instrumentation-dataloader/src/instrumentation.ts @@ -185,13 +185,6 @@ export class DataloaderInstrumentation extends InstrumentationBase { - span.setAttribute('cache.key', [args[0]]); - span.setAttribute('cache.hit', value !== undefined); - span.setAttribute( - 'cache.item_size', - value ? JSON.stringify(value).length : 0 - ); - span.end(); return value; }) @@ -250,19 +243,6 @@ export class DataloaderInstrumentation extends InstrumentationBase { - span.setAttribute('cache.key', Array.from(args[0])); - span.setAttribute( - 'cache.hit', - value.every((v: unknown) => v !== undefined) - ); - span.setAttribute( - 'cache.item_size', - value.reduce( - (acc: number, v: unknown) => acc + JSON.stringify(v).length, - 0 - ) - ); - span.end(); return value; }); @@ -300,12 +280,6 @@ export class DataloaderInstrumentation extends InstrumentationBase { - span.setAttribute('cache.key', [args[0]]); - return original.call(this, ...args); }); diff --git a/plugins/node/instrumentation-dataloader/test/dataloader.test.ts b/plugins/node/instrumentation-dataloader/test/dataloader.test.ts index 67a295b604..ca320a4a77 100644 --- a/plugins/node/instrumentation-dataloader/test/dataloader.test.ts +++ b/plugins/node/instrumentation-dataloader/test/dataloader.test.ts @@ -184,38 +184,6 @@ describe('DataloaderInstrumentation', () => { assert.strictEqual(batchSpan.name, 'dataloader.batch test-name'); } }); - - it('correctly sets cache.key, cache.hit and cache.item_size attributes when data is available', async () => { - assert.strictEqual(await dataloader.load('test'), getMd5HashFromIdx(0)); - - // We should have exactly two spans (one for .load and one for the batch) - assert.strictEqual(memoryExporter.getFinishedSpans().length, 2); - const [_batchSpan, loadSpan] = memoryExporter.getFinishedSpans(); - - assert.deepStrictEqual(loadSpan.attributes, { - 'cache.key': ['test'], - 'cache.hit': true, - 'cache.item_size': 34, - }); - }); - - it('correctly sets cache.key, cache.hit and cache.item_size attributes when data is not available', async () => { - const namedDataloader = new Dataloader(async keys => { - return keys.map(() => undefined); - }); - - assert.strictEqual(await namedDataloader.load('unavailable'), undefined); - - // We should have exactly two spans (one for .load and one for the batch) - assert.strictEqual(memoryExporter.getFinishedSpans().length, 2); - const [_batchSpan, loadSpan] = memoryExporter.getFinishedSpans(); - - assert.deepStrictEqual(loadSpan.attributes, { - 'cache.key': ['unavailable'], - 'cache.hit': false, - 'cache.item_size': 0, - }); - }); }); describe('loadMany', () => { @@ -429,18 +397,6 @@ describe('DataloaderInstrumentation', () => { assert.strictEqual(clearSpan.name, 'dataloader.clear test-name'); } }); - - it('correctly sets attributes for clearing', async () => { - dataloader.clear('test'); - - // We should have exactly one span - assert.strictEqual(memoryExporter.getFinishedSpans().length, 1); - const [clearSpan] = memoryExporter.getFinishedSpans(); - - assert.deepStrictEqual(clearSpan.attributes, { - 'cache.key': ['test'], - }); - }); }); describe('clearAll', () => { @@ -593,19 +549,6 @@ describe('DataloaderInstrumentation', () => { assert.strictEqual(primeSpan1.name, 'dataloader.prime'); assert.strictEqual(primeSpan2.name, 'dataloader.prime'); }); - - it('correctly creates attributes for priming', async () => { - dataloader.prime('test', '1'); - - // We should have exactly one span - assert.strictEqual(memoryExporter.getFinishedSpans().length, 1); - const [primeSpan] = memoryExporter.getFinishedSpans(); - - assert.deepStrictEqual(primeSpan.attributes, { - 'cache.key': ['test'], - 'cache.item_size': 3, - }); - }); }); it('should not create anything if disabled', async () => {