diff --git a/plugins/node/opentelemetry-instrumentation-express/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-express/src/instrumentation.ts index 3df15aa699..013650327c 100644 --- a/plugins/node/opentelemetry-instrumentation-express/src/instrumentation.ts +++ b/plugins/node/opentelemetry-instrumentation-express/src/instrumentation.ts @@ -29,6 +29,7 @@ import { AttributeNames } from './enums/AttributeNames'; import { asErrorAndMessage, getLayerMetadata, + getLayerPath, isLayerIgnored, storeLayerPath, } from './utils'; @@ -125,7 +126,7 @@ export class ExpressInstrumentation extends InstrumentationBase< const layer = this.stack[this.stack.length - 1] as ExpressLayer; instrumentation._applyPatch( layer, - typeof args[0] === 'string' ? args[0] : undefined + getLayerPath(args) ); return route; }; @@ -146,7 +147,7 @@ export class ExpressInstrumentation extends InstrumentationBase< const layer = this.stack[this.stack.length - 1] as ExpressLayer; instrumentation._applyPatch( layer, - typeof args[0] === 'string' ? args[0] : undefined + getLayerPath(args) ); return route; }; @@ -168,7 +169,7 @@ export class ExpressInstrumentation extends InstrumentationBase< instrumentation._applyPatch.call( instrumentation, layer, - typeof args[0] === 'string' ? args[0] : undefined + getLayerPath(args) ); return route; }; diff --git a/plugins/node/opentelemetry-instrumentation-express/src/utils.ts b/plugins/node/opentelemetry-instrumentation-express/src/utils.ts index 7e51b84124..fb7446ba76 100644 --- a/plugins/node/opentelemetry-instrumentation-express/src/utils.ts +++ b/plugins/node/opentelemetry-instrumentation-express/src/utils.ts @@ -145,3 +145,24 @@ export const asErrorAndMessage = ( error instanceof Error ? [error, error.message] : [String(error), String(error)]; + + +/** + * Extracts the layer path from the route arguments + * + * @param args - Arguments of the route + * @returns The layer path + */ +export const getLayerPath = (args: unknown[]) => { + if (typeof args[0] === 'string') { + return args[0]; + } + + if (Array.isArray(args[0])) { + return args[0].map((arg) => ( + typeof arg === 'string' || arg instanceof RegExp ? arg : '') + ).join(','); + } + + return; +} diff --git a/plugins/node/opentelemetry-instrumentation-express/test/express.test.ts b/plugins/node/opentelemetry-instrumentation-express/test/express.test.ts index f573a4a669..241f35bec4 100644 --- a/plugins/node/opentelemetry-instrumentation-express/test/express.test.ts +++ b/plugins/node/opentelemetry-instrumentation-express/test/express.test.ts @@ -566,4 +566,39 @@ describe('ExpressInstrumentation', () => { }, }); }); + + it('should work with ESM usage and multiple routes including regex', async () => { + await testUtils.runTestFixture({ + cwd: __dirname, + argv: ['fixtures/use-express-regex.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; + + console.debug(spans); + + assert.strictEqual(spans[0].name, 'GET /test/arr/:id,/\\\/test\\\/arr[0-9]*\\\/required(path)?(\\\/optionalPath)?\\\/(lastParam)?/'); + 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, 'request handler - /test/arr/:id,/\\\/test\\\/arr[0-9]*\\\/required(path)?(\\\/optionalPath)?\\\/(lastParam)?/'); + assert.strictEqual(spans[4].kind, SpanKind.SERVER); + assert.strictEqual(spans[4].parentSpanId, spans[0].spanId); + }, + }); + }); }); diff --git a/plugins/node/opentelemetry-instrumentation-express/test/fixtures/use-express-regex.mjs b/plugins/node/opentelemetry-instrumentation-express/test/fixtures/use-express-regex.mjs new file mode 100644 index 0000000000..e5aebae064 --- /dev/null +++ b/plugins/node/opentelemetry-instrumentation-express/test/fixtures/use-express-regex.mjs @@ -0,0 +1,67 @@ +/* + * 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. + */ + +// Use express from an ES module: +// node --experimental-loader=@opentelemetry/instrumentation/hook.mjs use-express-regex.mjs + +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-regex', + 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(); +}); + +app.get(['/test/arr/:id', /\/test\/arr[0-9]*\/required(path)?(\/optionalPath)?\/(lastParam)?/], (_req, res) => { + res.send({ response: 'response' }); +}); + +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}/test/arr/requiredPath/optionalPath/lastParam`, (res) => { + res.resume(); + res.on('end', () => { + resolve(); + }); + }) +}); + +await new Promise(resolve => server.close(resolve)); +await sdk.shutdown();