feat(koa): Skip update HTTP's span name and update RpcMetadata's rout…

…e 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 <[email protected]> Co-authored-by: Amir Blum <[email protected]>
open-telemetry · Aug 13, 2023 · 825b5a8 · 825b5a8
1 parent eeda32a
commit 825b5a8
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 39 deletions.
diff --git 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<typeof koa> {
 
       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<typeof koa> {
         );
       }
 
+      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
@@ -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,