handle route naming

Author: s1gr1d

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

@@ -32,6 +32,8 @@ import {
   getLayerPath,
   isLayerIgnored,
   storeLayerPath,
+  getActualMatchedRoute,
+  getConstructedRoute,
 } from './utils';
 /** @knipignore */
 import { PACKAGE_NAME, PACKAGE_VERSION } from './version';
@@ -188,25 +190,21 @@ export class ExpressInstrumentation extends InstrumentationBase<ExpressInstrumen
         res: express.Response
       ) {
         const { isLayerPathStored } = storeLayerPath(req, layerPath);
-        const route = (req[_LAYERS_STORE_PROPERTY] as string[])
-          .filter(path => path !== '/' && path !== '/*')
-          .join('')
-          // remove duplicate slashes to normalize route
-          .replace(/\/{2,}/g, '/');
 
-        const actualRoute = route.length > 0 ? route : undefined;
+        const constructedRoute = getConstructedRoute(req);
+        const actualMatchedRoute = getActualMatchedRoute(req);
 
         const attributes: Attributes = {
-          [SEMATTRS_HTTP_ROUTE]: actualRoute,
+          [SEMATTRS_HTTP_ROUTE]: actualMatchedRoute,
         };
-        const metadata = getLayerMetadata(route, layer, layerPath);
+        const metadata = getLayerMetadata(constructedRoute, layer, layerPath);
         const type = metadata.attributes[
           AttributeNames.EXPRESS_TYPE
         ] as ExpressLayerType;
 
         const rpcMetadata = getRPCMetadata(context.active());
         if (rpcMetadata?.type === RPCType.HTTP) {
-          rpcMetadata.route = actualRoute;
+          rpcMetadata.route = actualMatchedRoute;
         }
 
         // verify against the config if the layer should be ignored
@@ -225,7 +223,7 @@ export class ExpressInstrumentation extends InstrumentationBase<ExpressInstrumen
           {
             request: req,
             layerType: type,
-            route,
+            route: constructedRoute,
           },
           metadata.name
         );
@@ -243,7 +241,7 @@ export class ExpressInstrumentation extends InstrumentationBase<ExpressInstrumen
               requestHook(span, {
                 request: req,
                 layerType: type,
-                route,
+                route: constructedRoute,
               }),
             e => {
               if (e) {

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

@@ -213,3 +213,81 @@ const extractLayerPathSegment = (arg: LayerPathSegment) => {
 
   return;
 };
+
+export function getConstructedRoute(req: {
+  originalUrl: PatchedRequest['originalUrl'];
+  [_LAYERS_STORE_PROPERTY]?: string[];
+}) {
+  const layersStore: string[] = Array.isArray(req[_LAYERS_STORE_PROPERTY])
+    ? (req[_LAYERS_STORE_PROPERTY] as string[])
+    : [];
+
+  const meaningfulPaths = layersStore.filter(
+    path => path !== '/' && path !== '/*'
+  );
+
+  // Handle standalone wildcard case
+  if (meaningfulPaths.length === 1 && meaningfulPaths[0] === '*') {
+    return '*';
+  }
+
+  // Construct the route by joining paths and normalizing
+  return meaningfulPaths.join('').replace(/\/{2,}/g, '/'); // Remove duplicate slashes
+}
+
+/**
+ * Extracts the actual matched route from Express request for OpenTelemetry instrumentation.
+ * Returns the route that should be used as the http.route attribute.
+ *
+ * @param req - The Express request object with layers store
+ * @param layersStoreProperty - The property name where layer paths are stored
+ * @returns The matched route string or undefined if no valid route is found
+ */
+export function getActualMatchedRoute(req: {
+  originalUrl: PatchedRequest['originalUrl'];
+  [_LAYERS_STORE_PROPERTY]?: string[];
+}): string | undefined {
+  const layersStore: string[] = Array.isArray(req[_LAYERS_STORE_PROPERTY])
+    ? (req[_LAYERS_STORE_PROPERTY] as string[])
+    : [];
+
+  // If no layers are stored, no route can be determined
+  if (layersStore.length === 0) {
+    return undefined;
+  }
+
+  // Handle root path case - if all paths are root, only return root if originalUrl is also root
+  if (layersStore.every(path => path === '/')) {
+    return req.originalUrl === '/' ? '/' : undefined;
+  }
+
+  const constructedRoute = getConstructedRoute(req);
+  if (constructedRoute === '*') {
+    return constructedRoute;
+  }
+
+  // Ensure route starts with '/' if it doesn't already
+  const normalizedRoute = constructedRoute.startsWith('/')
+    ? constructedRoute
+    : `/${constructedRoute}`;
+
+  // Validate that this appears to be a matched route
+  // A route is considered matched if:
+  // 1. We have a constructed route
+  // 2. The original URL matches or starts with our route pattern
+  const isValidRoute =
+    normalizedRoute.length > 0 &&
+    (req.originalUrl === normalizedRoute ||
+      req.originalUrl.startsWith(normalizedRoute) ||
+      isRoutePattern(normalizedRoute));
+
+  return isValidRoute ? normalizedRoute : undefined;
+}
+
+/**
+ * Checks if a route contains parameter patterns (e.g., :id, :userId)
+ * which are valid even if they don't exactly match the original URL
+ */
+function isRoutePattern(route: string): boolean {
+  return route.includes(':') || route.includes('*');
+}

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

@@ -89,13 +89,17 @@ describe('ExpressInstrumentation', () => {
           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')
+          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');
+          assert.ok(
+            middlewareSpans.length > 0,
+            'Middleware spans should be created'
+          );
 
           for (const span of spans) {
             assert.strictEqual(