fix(express): Ensure 404 routes don't attach route attribute

Author: s1gr1d

plugins/node/opentelemetry-instrumentation-express/src/instrumentation.ts

@@ -194,8 +194,10 @@ export class ExpressInstrumentation extends InstrumentationBase<ExpressInstrumen
           // remove duplicate slashes to normalize route
           .replace(/\/{2,}/g, '/');
 
+        const actualRoute = route.length > 0 ? route : undefined;
+
         const attributes: Attributes = {
-          [SEMATTRS_HTTP_ROUTE]: route.length > 0 ? route : '/',
+          [SEMATTRS_HTTP_ROUTE]: actualRoute,
         };
         const metadata = getLayerMetadata(route, layer, layerPath);
         const type = metadata.attributes[
@@ -204,7 +206,7 @@ export class ExpressInstrumentation extends InstrumentationBase<ExpressInstrumen
 
         const rpcMetadata = getRPCMetadata(context.active());
         if (rpcMetadata?.type === RPCType.HTTP) {
-          rpcMetadata.route = route || '/';
+          rpcMetadata.route = actualRoute;
         }
 
         // verify against the config if the layer should be ignored

plugins/node/opentelemetry-instrumentation-express/test/express.test.ts

@@ -67,6 +67,46 @@ describe('ExpressInstrumentation', () => {
       server?.close();
     });
 
+    it('does not attach semantic route attribute for 404 page', async () => {
+      const rootSpan = tracer.startSpan('rootSpan');
+      const httpServer = await serverWithMiddleware(tracer, rootSpan, app => {
+        app.use(express.json());
+      });
+      server = httpServer.server;
+      port = httpServer.port;
+      assert.strictEqual(memoryExporter.getFinishedSpans().length, 0);
+
+      await context.with(
+        trace.setSpan(context.active(), rootSpan),
+        async () => {
+          try {
+            await httpRequest.get(
+              `http://localhost:${port}/non-existing-route`
+            );
+          } catch (error) {}
+          rootSpan.end();
+
+          const spans = memoryExporter.getFinishedSpans();
+
+          // Should have middleware spans but no request handler span
+          const middlewareSpans = spans.filter(span =>
+            span.name.includes('middleware') ||
+            span.name.includes('expressInit') ||
+            span.name.includes('jsonParser')
+          );
+
+          assert.ok(middlewareSpans.length > 0, 'Middleware spans should be created');
+
+          for (const span of spans) {
+            assert.strictEqual(
+              span.attributes[SEMATTRS_HTTP_ROUTE],
+              undefined, // none of the spans have the HTTP route attribute
+              `Span "${span.name}" should not have HTTP route attribute for non-existing route`
+            );
+          }
+        }
+      );
+    });
     it('should create a child span for middlewares', async () => {
       const rootSpan = tracer.startSpan('rootSpan');
       const customMiddleware: express.RequestHandler = (req, res, next) => {