feat(instrumentation-express): Use router path in router span names

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

@@ -180,7 +180,7 @@ export class ExpressInstrumentation extends InstrumentationBase<ExpressInstrumen
  const attributes: Attributes = {
  [SEMATTRS_HTTP_ROUTE]: route.length > 0 ? route : '/',
  };
- const metadata = getLayerMetadata(layer, layerPath);
+ const metadata = getLayerMetadata(route, layer, layerPath);
  const type = metadata.attributes[
  AttributeNames.EXPRESS_TYPE
  ] as ExpressLayerType;

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

@@ -64,6 +64,7 @@ export type ExpressLayer = {
  params: { [key: string]: string };
  path: string;
  regexp: RegExp;
+ route?: ExpressLayer;
 };
 
 export type LayerMetadata = {

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

@@ -44,33 +44,61 @@
  (request[_LAYERS_STORE_PROPERTY] as string[]).push(value);
 };
 
+/**
+ * Recursively search the router path from layer stack
+ * @param path The path to reconstruct
+ * @param layer The layer to reconstruct from
+ * @returns The reconstructed path
+ */
+export const getRouterPath = (path: string, layer: ExpressLayer): string => {
+ const stackLayer = layer.handle?.stack?.[0];
+
+ if (stackLayer?.route?.path) {
+ return `${path}${stackLayer.route.path}`;
+ }
+
+ if (stackLayer?.handle?.stack) {
+ return getRouterPath(path, stackLayer);
+ }
+
+ return path;
+};
+
 /**
  * Parse express layer context to retrieve a name and attributes.
+ * @param route The route of the layer
  * @param layer Express layer
  * @param [layerPath] if present, the path on which the layer has been mounted
  */
 export const getLayerMetadata = (
+ route: string,
  layer: ExpressLayer,
  layerPath?: string
 ): {
  attributes: Attributes;
  name: string;
 } => {
  if (layer.name === 'router') {
+ const maybeRouterPath = getRouterPath('', layer);
+ const extractedRouterPath = maybeRouterPath
+ ? maybeRouterPath
+ : layerPath || route || '/';
+
  return {
  attributes: {
- [AttributeNames.EXPRESS_NAME]: layerPath,
+ [AttributeNames.EXPRESS_NAME]: extractedRouterPath,
  [AttributeNames.EXPRESS_TYPE]: ExpressLayerType.ROUTER,
  },
- name: `router - ${layerPath}`,
+ name: `router - ${extractedRouterPath}`,
  };
  } else if (layer.name === 'bound dispatch') {
  return {
  attributes: {
- [AttributeNames.EXPRESS_NAME]: layerPath ?? 'request handler',
+ [AttributeNames.EXPRESS_NAME]:
+ (route || layerPath) ?? 'request handler',
  [AttributeNames.EXPRESS_TYPE]: ExpressLayerType.REQUEST_HANDLER,
  },
- name: `request handler${layer.path ? ` - ${layerPath}` : ''}`,
+ name: `request handler${layer.path ? ` - ${route || layerPath}` : ''}`,
  };
  } else {
  return {
@@ -159,11 +187,13 @@
 export const getLayerPath = (
  args: [LayerPathSegment | LayerPathSegment[], ...unknown[]]
 ): string | undefined => {
- if (Array.isArray(args[0])) {
- return args[0].map(arg => extractLayerPathSegment(arg) || '').join(',');
+ const firstArg = args[0];
+
+ if (Array.isArray(firstArg)) {
+ return firstArg.map(arg => extractLayerPathSegment(arg) || '').join(',');
  }
 
- return extractLayerPathSegment(args[0]);
+ return extractLayerPathSegment(firstArg);
 };
 
 const extractLayerPathSegment = (arg: LayerPathSegment) => {

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

@@ -641,6 +641,84 @@ describe('ExpressInstrumentation', () => {
  });
  });
 
+ it('should work with Express routers', async () => {
+ await testUtils.runTestFixture({
+ cwd: __dirname,
+ argv: ['fixtures/use-express-router.mjs'],
+ env: {
+ NODE_OPTIONS:
+ '--experimental-loader=@opentelemetry/instrumentation/hook.mjs',
+ NODE_NO_WARNINGS: '1',
+ },
+ checkResult: (err, stdout, stderr) => {
+ assert.ifError(err);
+ },
+ checkCollector: (collector: testUtils.TestCollector) => {
+ const spans = collector.sortedSpans;
+
+ assert.strictEqual(spans[0].name, 'GET /api/user/:id');
+ assert.strictEqual(spans[0].kind, SpanKind.CLIENT);
+ assert.strictEqual(spans[1].name, 'middleware - query');
+ assert.strictEqual(spans[1].kind, SpanKind.SERVER);
+ assert.strictEqual(spans[1].parentSpanId, spans[0].spanId);
+ assert.strictEqual(spans[2].name, 'middleware - expressInit');
+ assert.strictEqual(spans[2].kind, SpanKind.SERVER);
+ assert.strictEqual(spans[2].parentSpanId, spans[0].spanId);
+ assert.strictEqual(spans[3].name, 'middleware - simpleMiddleware');
+ assert.strictEqual(spans[3].kind, SpanKind.SERVER);
+ assert.strictEqual(spans[3].parentSpanId, spans[0].spanId);
+ assert.strictEqual(spans[4].name, 'router - /api/user/:id');
+ assert.strictEqual(spans[4].kind, SpanKind.SERVER);
+ assert.strictEqual(spans[4].parentSpanId, spans[0].spanId);
+ assert.strictEqual(spans[5].name, 'request handler - /api/user/:id');
+ assert.strictEqual(spans[5].kind, SpanKind.SERVER);
+ assert.strictEqual(spans[5].parentSpanId, spans[0].spanId);
+ },
+ });
+ });
+
+ it('should work with nested Express routers', async () => {
+ await testUtils.runTestFixture({
+ cwd: __dirname,
+ argv: ['fixtures/use-express-nested-router.mjs'],
+ env: {
+ NODE_OPTIONS:
+ '--experimental-loader=@opentelemetry/instrumentation/hook.mjs',
+ NODE_NO_WARNINGS: '1',
+ },
+ checkResult: (err, stdout, stderr) => {
+ assert.ifError(err);
+ },
+ checkCollector: (collector: testUtils.TestCollector) => {
+ const spans = collector.sortedSpans;
+
+ assert.strictEqual(spans[0].name, 'GET /api/user/:id/posts/:postId');
+ assert.strictEqual(spans[0].kind, SpanKind.CLIENT);
+ assert.strictEqual(spans[1].name, 'middleware - query');
+ assert.strictEqual(spans[1].kind, SpanKind.SERVER);
+ assert.strictEqual(spans[1].parentSpanId, spans[0].spanId);
+ assert.strictEqual(spans[2].name, 'middleware - expressInit');
+ assert.strictEqual(spans[2].kind, SpanKind.SERVER);
+ assert.strictEqual(spans[2].parentSpanId, spans[0].spanId);
+ assert.strictEqual(spans[3].name, 'middleware - simpleMiddleware');
+ assert.strictEqual(spans[3].kind, SpanKind.SERVER);
+ assert.strictEqual(spans[3].parentSpanId, spans[0].spanId);
+ assert.strictEqual(spans[4].name, 'router - /api/user/:id');
+ assert.strictEqual(spans[4].kind, SpanKind.SERVER);
+ assert.strictEqual(spans[4].parentSpanId, spans[0].spanId);
+ assert.strictEqual(spans[5].name, 'router - /:postId');
+ assert.strictEqual(spans[5].kind, SpanKind.SERVER);
+ assert.strictEqual(spans[5].parentSpanId, spans[0].spanId);
+ assert.strictEqual(
+ spans[6].name,
+ 'request handler - /api/user/:id/posts/:postId'
+ );
+ assert.strictEqual(spans[6].kind, SpanKind.SERVER);
+ assert.strictEqual(spans[6].parentSpanId, spans[0].spanId);
+ },
+ });
+ });
+
  it('should set a correct transaction name for routes specified in RegEx', async () => {
  await testUtils.runTestFixture({
  cwd: __dirname,

plugins/node/opentelemetry-instrumentation-express/test/fixtures/use-express-nested-router.mjs

@@ -0,0 +1,81 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { promisify } from 'util';
+import { createTestNodeSdk } from '@opentelemetry/contrib-test-utils';
+
+import { HttpInstrumentation } from '@opentelemetry/instrumentation-http';
+import { ExpressInstrumentation } from '../../build/src/index.js';
+
+const sdk = createTestNodeSdk({
+ serviceName: 'use-express-nested',
+ instrumentations: [
+ new HttpInstrumentation(),
+ new ExpressInstrumentation()
+ ]
+})
+
+sdk.start();
+
+import express from 'express';
+import * as http from 'http';
+
+const app = express();
+
+app.use(async function simpleMiddleware(req, res, next) {
+ // Wait a short delay to ensure this "middleware - ..." span clearly starts
+ // before the "router - ..." span. The test rely on a start-time-based sort
+ // of the produced spans. If they start in the same millisecond, then tests
+ // can be flaky.
+ await promisify(setTimeout)(10);
+ next();
+});
+
+const userRouter = express.Router();
+const postsRouter = express.Router();
+
+postsRouter.get('/:postId', (req, res, next) => {
+ res.json({ hello: 'yes' });
+ res.end();
+ next();
+});
+
+userRouter.get('/api/user/:id', (req, res, next) => {
+ res.json({ hello: 'yes' });
+ res.end();
+ next();
+});
+
+userRouter.use('/api/user/:id/posts', postsRouter);
+
+app.use(userRouter);
+
+const server = http.createServer(app);
+await new Promise(resolve => server.listen(0, resolve));
+const port = server.address().port;
+
+
+await new Promise(resolve => {
+ http.get(`http://localhost:${port}/api/user/123/posts/321`, (res) => {
+ res.resume();
+ res.on('end', data => {
+ resolve(data);
+ });
+ })
+});
+
+await new Promise(resolve => server.close(resolve));
+await sdk.shutdown();

plugins/node/opentelemetry-instrumentation-express/test/fixtures/use-express-router.mjs

@@ -0,0 +1,72 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { promisify } from 'util';
+import { createTestNodeSdk } from '@opentelemetry/contrib-test-utils';
+
+import { HttpInstrumentation } from '@opentelemetry/instrumentation-http';
+import { ExpressInstrumentation } from '../../build/src/index.js';
+
+const sdk = createTestNodeSdk({
+ serviceName: 'use-express-nested',
+ instrumentations: [
+ new HttpInstrumentation(),
+ new ExpressInstrumentation()
+ ]
+})
+
+sdk.start();
+
+import express from 'express';
+import * as http from 'http';
+
+const app = express();
+
+app.use(async function simpleMiddleware(req, res, next) {
+ // Wait a short delay to ensure this "middleware - ..." span clearly starts
+ // before the "router - ..." span. The test rely on a start-time-based sort
+ // of the produced spans. If they start in the same millisecond, then tests
+ // can be flaky.
+ await promisify(setTimeout)(10);
+ next();
+});
+
+const router = express.Router();
+
+router.get('/api/user/:id', (req, res, next) => {
+ res.json({ hello: 'yes' });
+ res.end();
+ next();
+});
+
+app.use(router);
+
+const server = http.createServer(app);
+await new Promise(resolve => server.listen(0, resolve));
+const port = server.address().port;
+
+
+await new Promise(resolve => {
+ http.get(`http://localhost:${port}/api/user/123`, (res) => {
+ res.resume();
+ res.on('end', data => {
+ resolve(data);
+ });
+ })
+});
+
+await new Promise(resolve => server.close(resolve));
+await sdk.shutdown();