From 825b5a89cb6e8a667c3fcfb3f25bb954d4c260dc Mon Sep 17 00:00:00 2001 From: Chi Ma Date: Sun, 13 Aug 2023 14:28:34 +0700 Subject: [PATCH] feat(koa): Skip update HTTP's span name and update RpcMetadata's route instead (#1567) * feat(koa): Skip update HTTP's span name and update RpcMetadata's route instead * make the logic of rpcMetadata.route the same as previously * Remove unused variable --------- Co-authored-by: Haddas Bronfman <85441461+haddasbronfman@users.noreply.github.com> Co-authored-by: Amir Blum --- .../src/instrumentation.ts | 21 +++--------- .../test/koa.test.ts | 34 +++++++------------ 2 files changed, 16 insertions(+), 39 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-koa/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-koa/src/instrumentation.ts index f270875efac..df359e9c3b5 100644 --- a/plugins/node/opentelemetry-instrumentation-koa/src/instrumentation.ts +++ b/plugins/node/opentelemetry-instrumentation-koa/src/instrumentation.ts @@ -29,10 +29,9 @@ import { KoaLayerType, KoaInstrumentationConfig, } from './types'; -import { AttributeNames } from './enums/AttributeNames'; import { VERSION } from './version'; import { getMiddlewareMetadata, isLayerIgnored } from './utils'; -import { getRPCMetadata, RPCType, setRPCMetadata } from '@opentelemetry/core'; +import { getRPCMetadata, RPCType } from '@opentelemetry/core'; import { kLayerPatched, KoaPatchedMiddleware } from './internal-types'; /** Koa instrumentation for OpenTelemetry */ @@ -174,21 +173,8 @@ export class KoaInstrumentation extends InstrumentationBase { const rpcMetadata = getRPCMetadata(api.context.active()); - if ( - metadata.attributes[AttributeNames.KOA_TYPE] === KoaLayerType.ROUTER && - rpcMetadata?.type === RPCType.HTTP - ) { - rpcMetadata.span.updateName( - `${context.method} ${context._matchedRoute}` - ); - } - - let newContext = api.trace.setSpan(api.context.active(), span); - if (rpcMetadata?.type === RPCType.HTTP) { - newContext = setRPCMetadata( - newContext, - Object.assign(rpcMetadata, { route: context._matchedRoute }) - ); + if (rpcMetadata?.type === RPCType.HTTP && context._matchedRoute) { + rpcMetadata.route = context._matchedRoute.toString(); } if (this.getConfig().requestHook) { @@ -208,6 +194,7 @@ export class KoaInstrumentation extends InstrumentationBase { ); } + const newContext = api.trace.setSpan(api.context.active(), span); return api.context.with(newContext, async () => { try { return await middlewareLayer(context, next); diff --git a/plugins/node/opentelemetry-instrumentation-koa/test/koa.test.ts b/plugins/node/opentelemetry-instrumentation-koa/test/koa.test.ts index 062db59cdc8..4d11b7d069f 100644 --- a/plugins/node/opentelemetry-instrumentation-koa/test/koa.test.ts +++ b/plugins/node/opentelemetry-instrumentation-koa/test/koa.test.ts @@ -34,7 +34,7 @@ import * as sinon from 'sinon'; import { AddressInfo } from 'net'; import { KoaLayerType, KoaRequestInfo } from '../src/types'; import { AttributeNames } from '../src/enums/AttributeNames'; -import { RPCType, setRPCMetadata } from '@opentelemetry/core'; +import { RPCMetadata, RPCType, setRPCMetadata } from '@opentelemetry/core'; const httpRequest = { get: (options: http.ClientRequestArgs | string) => { @@ -134,7 +134,7 @@ describe('Koa Instrumentation', () => { describe('Instrumenting @koa/router calls', () => { it('should create a child span for middlewares', async () => { const rootSpan = tracer.startSpan('rootSpan'); - const rpcMetadata = { type: RPCType.HTTP, span: rootSpan }; + const rpcMetadata: RPCMetadata = { type: RPCType.HTTP, span: rootSpan }; app.use((ctx, next) => context.with( setRPCMetadata( @@ -174,17 +174,14 @@ describe('Koa Instrumentation', () => { '/post/:id' ); - const exportedRootSpan = memoryExporter - .getFinishedSpans() - .find(span => span.name === 'GET /post/:id'); - assert.notStrictEqual(exportedRootSpan, undefined); + assert.strictEqual(rpcMetadata.route, '/post/:id'); } ); }); it('should create a named child span for middlewares', async () => { const rootSpan = tracer.startSpan('rootSpan'); - const rpcMetadata = { type: RPCType.HTTP, span: rootSpan }; + const rpcMetadata: RPCMetadata = { type: RPCType.HTTP, span: rootSpan }; app.use((ctx, next) => context.with( setRPCMetadata( @@ -224,17 +221,14 @@ describe('Koa Instrumentation', () => { '/post/:id' ); - const exportedRootSpan = memoryExporter - .getFinishedSpans() - .find(span => span.name === 'GET /post/:id'); - assert.notStrictEqual(exportedRootSpan, undefined); + assert.strictEqual(rpcMetadata.route, '/post/:id'); } ); }); it('should correctly instrument nested routers', async () => { const rootSpan = tracer.startSpan('rootSpan'); - const rpcMetadata = { type: RPCType.HTTP, span: rootSpan }; + const rpcMetadata: RPCMetadata = { type: RPCType.HTTP, span: rootSpan }; app.use((ctx, next) => context.with( setRPCMetadata( @@ -276,17 +270,14 @@ describe('Koa Instrumentation', () => { '/:first/post/:id' ); - const exportedRootSpan = memoryExporter - .getFinishedSpans() - .find(span => span.name === 'GET /:first/post/:id'); - assert.notStrictEqual(exportedRootSpan, undefined); + assert.strictEqual(rpcMetadata.route, '/:first/post/:id'); } ); }); it('should correctly instrument prefixed routers', async () => { const rootSpan = tracer.startSpan('rootSpan'); - const rpcMetadata = { type: RPCType.HTTP, span: rootSpan }; + const rpcMetadata: RPCMetadata = { type: RPCType.HTTP, span: rootSpan }; app.use((ctx, next) => context.with( setRPCMetadata( @@ -326,10 +317,7 @@ describe('Koa Instrumentation', () => { '/:first/post/:id' ); - const exportedRootSpan = memoryExporter - .getFinishedSpans() - .find(span => span.name === 'GET /:first/post/:id'); - assert.notStrictEqual(exportedRootSpan, undefined); + assert.strictEqual(rpcMetadata.route, '/:first/post/:id'); } ); }); @@ -364,7 +352,9 @@ describe('Koa Instrumentation', () => { .find(span => span.name.includes('spanCreateMiddleware')); assert.notStrictEqual(fooParentSpan, undefined); - const fooSpan = memoryExporter.getFinishedSpans().find(span => 'foo'); + const fooSpan = memoryExporter + .getFinishedSpans() + .find(span => span.name === 'foo'); assert.notStrictEqual(fooSpan, undefined); assert.strictEqual( fooSpan!.parentSpanId,