diff --git a/plugins/node/opentelemetry-plugin-express/package.json b/plugins/node/opentelemetry-plugin-express/package.json index f565359ccc..1da61611f8 100644 --- a/plugins/node/opentelemetry-plugin-express/package.json +++ b/plugins/node/opentelemetry-plugin-express/package.json @@ -49,6 +49,7 @@ "@types/node": "14.0.27", "@types/shimmer": "1.0.1", "codecov": "3.7.2", + "eslint-plugin-header": "^3.1.0", "express": "4.17.1", "gts": "2.0.2", "mocha": "7.2.0", diff --git a/plugins/node/opentelemetry-plugin-express/src/express.ts b/plugins/node/opentelemetry-plugin-express/src/express.ts index d5d66cefd3..2f34d7874c 100644 --- a/plugins/node/opentelemetry-plugin-express/src/express.ts +++ b/plugins/node/opentelemetry-plugin-express/src/express.ts @@ -170,7 +170,7 @@ export class ExpressPlugin extends BasePlugin { ) { storeLayerPath(req, layerPath); const route = (req[_LAYERS_STORE_PROPERTY] as string[]) - .filter(path => path !== '/') + .filter(path => path !== '/' && path !== '/*') .join(''); const attributes: Attributes = { [AttributeNames.COMPONENT]: ExpressPlugin.component, @@ -180,23 +180,33 @@ export class ExpressPlugin extends BasePlugin { const type = metadata.attributes[ AttributeNames.EXPRESS_TYPE ] as ExpressLayerType; - // verify against the config if the layer should be ignored - if (isLayerIgnored(metadata.name, type, plugin._config)) { - return original.apply(this, arguments); - } - if (plugin._tracer.getCurrentSpan() === undefined) { - return original.apply(this, arguments); - } - // Rename the root http span once we reach the request handler + + // Rename the root http span in case we haven't done it already + // once we reach the request handler if ( metadata.attributes[AttributeNames.EXPRESS_TYPE] === ExpressLayerType.REQUEST_HANDLER ) { const parent = plugin._tracer.getCurrentSpan(); - if (parent) { - parent.updateName(`${req.method} ${route}`); + // eslint-disable-next-line @typescript-eslint/ban-ts-ignore + // @ts-ignore + const parentSpanName = parent && parent.name; + if (parent && parentSpanName) { + const parentRoute = parentSpanName.split(' ')[1]; + if (!route.includes(parentRoute)) { + parent.updateName(`${req.method} ${route}`); + } } } + + // verify against the config if the layer should be ignored + if (isLayerIgnored(metadata.name, type, plugin._config)) { + return original.apply(this, arguments); + } + if (plugin._tracer.getCurrentSpan() === undefined) { + return original.apply(this, arguments); + } + const span = plugin._tracer.startSpan(metadata.name, { attributes: Object.assign(attributes, metadata.attributes), }); diff --git a/plugins/node/opentelemetry-plugin-express/test/express.test.ts b/plugins/node/opentelemetry-plugin-express/test/express.test.ts index 81f4d3259c..8515e35df2 100644 --- a/plugins/node/opentelemetry-plugin-express/test/express.test.ts +++ b/plugins/node/opentelemetry-plugin-express/test/express.test.ts @@ -102,6 +102,8 @@ describe('Express Plugin', () => { await tracer.withSpan(rootSpan, async () => { await httpRequest.get(`http://localhost:${port}/toto/tata`); rootSpan.end(); + // @ts-ignore + assert.strictEqual(rootSpan.name, 'GET /toto/:id'); assert.notStrictEqual( memoryExporter .getFinishedSpans() @@ -204,6 +206,65 @@ describe('Express Plugin', () => { }); server.close(); }); + it('should ignore all ExpressLayerType based on config. root span name should be modified when route exists', async () => { + plugin.disable(); + const config: ExpressPluginConfig = { + ignoreLayersType: [ + ExpressLayerType.MIDDLEWARE, + ExpressLayerType.ROUTER, + ExpressLayerType.REQUEST_HANDLER, + ], + }; + plugin.enable(express, provider, logger, config); + const rootSpan = tracer.startSpan('rootSpan'); + const app = express(); + app.use((req, res, next) => tracer.withSpan(rootSpan, next)); + app.use(express.json()); + app.use((req, res, next) => { + for (let i = 0; i < 1000; i++) { + continue; + } + return next(); + }); + const router = express.Router(); + app.use('/toto', router); + router.get('/:id', (req, res) => { + setImmediate(() => { + res.status(200).end(); + }); + }); + const server = http.createServer(app); + await new Promise(resolve => server.listen(0, resolve)); + + const port = (server.address() as AddressInfo).port; + assert.strictEqual(memoryExporter.getFinishedSpans().length, 0); + await tracer.withSpan(rootSpan, async () => { + await httpRequest.get(`http://localhost:${port}/toto/tata`); + rootSpan.end(); + // @ts-ignore + assert.strictEqual(rootSpan.name, 'GET /toto/:id'); + + assert.deepStrictEqual( + memoryExporter + .getFinishedSpans() + .filter( + span => + span.attributes[AttributeNames.EXPRESS_TYPE] === + ExpressLayerType.MIDDLEWARE || + span.attributes[AttributeNames.EXPRESS_TYPE] === + ExpressLayerType.ROUTER || + span.attributes[AttributeNames.EXPRESS_TYPE] === + ExpressLayerType.REQUEST_HANDLER + ).length, + 0 + ); + const exportedRootSpan = memoryExporter + .getFinishedSpans() + .find(span => span.name === 'GET /toto/:id'); + assert.notStrictEqual(exportedRootSpan, undefined); + }); + server.close(); + }); }); describe('Disabling plugin', () => {