From 80b09317731fbd060d7ce3b9397ff0e304103047 Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Mon, 8 Jul 2024 09:52:11 +0100 Subject: [PATCH 1/6] fix(instrumentation-express): Get `router` path from layer stack. --- .../src/instrumentation.ts | 2 +- .../src/internal-types.ts | 1 + .../src/utils.ts | 47 +++++++++-- .../test/express.test.ts | 78 ++++++++++++++++++ .../fixtures/use-express-nested-router.mjs | 81 +++++++++++++++++++ .../test/fixtures/use-express-router.mjs | 72 +++++++++++++++++ .../test/utils.test.ts | 49 ++++++++++- 7 files changed, 321 insertions(+), 9 deletions(-) create mode 100644 plugins/node/opentelemetry-instrumentation-express/test/fixtures/use-express-nested-router.mjs create mode 100644 plugins/node/opentelemetry-instrumentation-express/test/fixtures/use-express-router.mjs diff --git a/plugins/node/opentelemetry-instrumentation-express/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-express/src/instrumentation.ts index bcff7a52e2..155b63f494 100644 --- a/plugins/node/opentelemetry-instrumentation-express/src/instrumentation.ts +++ b/plugins/node/opentelemetry-instrumentation-express/src/instrumentation.ts @@ -180,7 +180,7 @@ export class ExpressInstrumentation extends InstrumentationBase 0 ? route : '/', }; - const metadata = getLayerMetadata(layer, layerPath); + const metadata = getLayerMetadata(route, layer, layerPath); const type = metadata.attributes[ AttributeNames.EXPRESS_TYPE ] as ExpressLayerType; diff --git a/plugins/node/opentelemetry-instrumentation-express/src/internal-types.ts b/plugins/node/opentelemetry-instrumentation-express/src/internal-types.ts index a661bf9de4..ebdc260bc3 100644 --- a/plugins/node/opentelemetry-instrumentation-express/src/internal-types.ts +++ b/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 = { diff --git a/plugins/node/opentelemetry-instrumentation-express/src/utils.ts b/plugins/node/opentelemetry-instrumentation-express/src/utils.ts index fa7b0ee9a0..da6949cb66 100644 --- a/plugins/node/opentelemetry-instrumentation-express/src/utils.ts +++ b/plugins/node/opentelemetry-instrumentation-express/src/utils.ts @@ -44,12 +44,32 @@ export const storeLayerPath = (request: PatchedRequest, value?: string) => { (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) { + path += stackLayer.route.path; + } else if (stackLayer.handle?.stack) { + path = 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 ): { @@ -57,20 +77,31 @@ export const getLayerMetadata = ( name: string; } => { if (layer.name === 'router') { + let extractedRouterPath = ''; + + if (layer.handle?.stack) { + extractedRouterPath = getRouterPath(extractedRouterPath, layer); + } + + if (!extractedRouterPath) { + extractedRouterPath = 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 +190,13 @@ export const asErrorAndMessage = ( 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) => { diff --git a/plugins/node/opentelemetry-instrumentation-express/test/express.test.ts b/plugins/node/opentelemetry-instrumentation-express/test/express.test.ts index 43b5da1324..d2f57c40e6 100644 --- a/plugins/node/opentelemetry-instrumentation-express/test/express.test.ts +++ b/plugins/node/opentelemetry-instrumentation-express/test/express.test.ts @@ -638,6 +638,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, diff --git a/plugins/node/opentelemetry-instrumentation-express/test/fixtures/use-express-nested-router.mjs b/plugins/node/opentelemetry-instrumentation-express/test/fixtures/use-express-nested-router.mjs new file mode 100644 index 0000000000..98ba0bf236 --- /dev/null +++ b/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(); diff --git a/plugins/node/opentelemetry-instrumentation-express/test/fixtures/use-express-router.mjs b/plugins/node/opentelemetry-instrumentation-express/test/fixtures/use-express-router.mjs new file mode 100644 index 0000000000..5846da88ae --- /dev/null +++ b/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(); diff --git a/plugins/node/opentelemetry-instrumentation-express/test/utils.test.ts b/plugins/node/opentelemetry-instrumentation-express/test/utils.test.ts index 098bab4e8c..f8dac5035b 100644 --- a/plugins/node/opentelemetry-instrumentation-express/test/utils.test.ts +++ b/plugins/node/opentelemetry-instrumentation-express/test/utils.test.ts @@ -94,6 +94,7 @@ describe('Utils', () => { it('should return router metadata', () => { assert.deepEqual( utils.getLayerMetadata( + '/test', { name: 'router', } as ExpressLayer, @@ -112,6 +113,7 @@ describe('Utils', () => { it('should return request handler metadata', () => { assert.deepEqual( utils.getLayerMetadata( + '/:id', { name: 'bound dispatch', } as ExpressLayer, @@ -129,7 +131,7 @@ describe('Utils', () => { it('should return middleware metadata', () => { assert.deepEqual( - utils.getLayerMetadata({ + utils.getLayerMetadata('', { name: 'bodyParser', } as ExpressLayer), { @@ -143,6 +145,51 @@ describe('Utils', () => { }); }); + describe('reconstructRouterPath()', () => { + it('should reconstruct a simple router path', () => { + const layer = { + handle: { + stack: [ + { + route: { + path: '/test', + }, + }, + ], + }, + }; + + assert.strictEqual( + utils.getRouterPath('', layer as unknown as ExpressLayer), + '/test' + ); + }); + + it('should reconstruct a parameterized router path', () => { + const layer = { + handle: { + stack: [ + { + handle: { + stack: [ + { + route: { + path: '/:id', + }, + }, + ], + }, + }, + ], + }, + }; + + assert.strictEqual( + utils.getRouterPath('', layer as unknown as ExpressLayer), + '/:id' + ); + }); + }); describe('asErrorAndMessage', () => { it('should special case Error instances', () => { const input = new Error('message'); From 6561563b1be1d7f27e9fb634699f2633cc18bc3e Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Thu, 25 Jul 2024 17:06:28 +0100 Subject: [PATCH 2/6] Apply suggestions from code review Co-authored-by: Abhijeet Prasad --- .../src/utils.ts | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-express/src/utils.ts b/plugins/node/opentelemetry-instrumentation-express/src/utils.ts index da6949cb66..5bbee5fbd8 100644 --- a/plugins/node/opentelemetry-instrumentation-express/src/utils.ts +++ b/plugins/node/opentelemetry-instrumentation-express/src/utils.ts @@ -54,9 +54,11 @@ export const getRouterPath = (path: string, layer: ExpressLayer): string => { const stackLayer = layer.handle?.stack[0]; if (stackLayer.route?.path) { - path += stackLayer.route.path; - } else if (stackLayer.handle?.stack) { - path = getRouterPath(path, stackLayer); + return `${path}${stackLayer.route.path}`; + } + + if (stackLayer.handle?.stack) { + return getRouterPath(path, stackLayer); } return path; @@ -77,15 +79,8 @@ export const getLayerMetadata = ( name: string; } => { if (layer.name === 'router') { - let extractedRouterPath = ''; - - if (layer.handle?.stack) { - extractedRouterPath = getRouterPath(extractedRouterPath, layer); - } - - if (!extractedRouterPath) { - extractedRouterPath = layerPath || route || '/'; - } + const maybeRouterPath = getRouterPath('', layer); + const extractedRouterPath = maybeRouterPath ? maybeRouterPath : layerPath || route || '/'; return { attributes: { From 61a57d1700015e9a9c542bbfcf659754f38d3fee Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Thu, 25 Jul 2024 17:11:37 +0100 Subject: [PATCH 3/6] Fix linter --- .../node/opentelemetry-instrumentation-express/src/utils.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-express/src/utils.ts b/plugins/node/opentelemetry-instrumentation-express/src/utils.ts index 5bbee5fbd8..76b26ce634 100644 --- a/plugins/node/opentelemetry-instrumentation-express/src/utils.ts +++ b/plugins/node/opentelemetry-instrumentation-express/src/utils.ts @@ -56,7 +56,7 @@ export const getRouterPath = (path: string, layer: ExpressLayer): string => { if (stackLayer.route?.path) { return `${path}${stackLayer.route.path}`; } - + if (stackLayer.handle?.stack) { return getRouterPath(path, stackLayer); } @@ -80,7 +80,9 @@ export const getLayerMetadata = ( } => { if (layer.name === 'router') { const maybeRouterPath = getRouterPath('', layer); - const extractedRouterPath = maybeRouterPath ? maybeRouterPath : layerPath || route || '/'; + const extractedRouterPath = maybeRouterPath + ? maybeRouterPath + : layerPath || route || '/'; return { attributes: { From 6804892356b352c80047192c958e6e59edab95a2 Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Thu, 25 Jul 2024 17:35:06 +0100 Subject: [PATCH 4/6] Update `getRouterPath` --- .../node/opentelemetry-instrumentation-express/src/utils.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-express/src/utils.ts b/plugins/node/opentelemetry-instrumentation-express/src/utils.ts index 76b26ce634..fdc8f3728c 100644 --- a/plugins/node/opentelemetry-instrumentation-express/src/utils.ts +++ b/plugins/node/opentelemetry-instrumentation-express/src/utils.ts @@ -51,13 +51,13 @@ export const storeLayerPath = (request: PatchedRequest, value?: string) => { * @returns The reconstructed path */ export const getRouterPath = (path: string, layer: ExpressLayer): string => { - const stackLayer = layer.handle?.stack[0]; + const stackLayer = layer.handle?.stack?.[0]; - if (stackLayer.route?.path) { + if (stackLayer?.route?.path) { return `${path}${stackLayer.route.path}`; } - if (stackLayer.handle?.stack) { + if (stackLayer?.handle?.stack) { return getRouterPath(path, stackLayer); } From a3b4d7224e2c8a693f0e4ac7e75870a64c657460 Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Tue, 3 Sep 2024 08:51:29 +0000 Subject: [PATCH 5/6] Update tests. --- .../test/express.test.ts | 70 ++++++++++--------- 1 file changed, 36 insertions(+), 34 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-express/test/express.test.ts b/plugins/node/opentelemetry-instrumentation-express/test/express.test.ts index a371b9a7b0..c0cd404258 100644 --- a/plugins/node/opentelemetry-instrumentation-express/test/express.test.ts +++ b/plugins/node/opentelemetry-instrumentation-express/test/express.test.ts @@ -656,23 +656,23 @@ describe('ExpressInstrumentation', () => { 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[0].name, 'GET'); + assert.strictEqual(spans[0].kind, testUtils.OtlpSpanKind.CLIENT); + assert.strictEqual(spans[1].name, 'GET /api/user/:id'); + assert.strictEqual(spans[1].kind, testUtils.OtlpSpanKind.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[2].name, 'middleware - query'); 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[3].parentSpanId, spans[1].spanId); + assert.strictEqual(spans[4].name, 'middleware - simpleMiddleware'); 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[4].parentSpanId, spans[1].spanId); + assert.strictEqual(spans[5].name, 'router - /api/user/:id'); assert.strictEqual(spans[5].kind, SpanKind.SERVER); - assert.strictEqual(spans[5].parentSpanId, spans[0].spanId); + assert.strictEqual(spans[5].parentSpanId, spans[1].spanId); + assert.strictEqual(spans[6].name, 'request handler - /api/user/:id'); + assert.strictEqual(spans[6].kind, SpanKind.SERVER); + assert.strictEqual(spans[6].parentSpanId, spans[1].spanId); }, }); }); @@ -692,29 +692,31 @@ describe('ExpressInstrumentation', () => { 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[0].name, 'GET'); + assert.strictEqual(spans[0].kind, testUtils.OtlpSpanKind.CLIENT); + assert.strictEqual(spans[1].name, 'GET /api/user/:id/posts/:postId'); + assert.strictEqual(spans[1].kind, testUtils.OtlpSpanKind.SERVER); + assert.strictEqual(spans[2].name, 'middleware - query'); + assert.strictEqual(spans[2].kind, testUtils.OtlpSpanKind.INTERNAL); + assert.strictEqual(spans[2].parentSpanId, spans[1].spanId); + assert.strictEqual(spans[3].name, 'middleware - expressInit'); + assert.strictEqual(spans[3].kind, testUtils.OtlpSpanKind.INTERNAL); + assert.strictEqual(spans[3].parentSpanId, spans[1].spanId); + assert.strictEqual(spans[4].name, 'middleware - simpleMiddleware'); + assert.strictEqual(spans[4].kind, testUtils.OtlpSpanKind.INTERNAL); + assert.strictEqual(spans[4].parentSpanId, spans[1].spanId); + assert.strictEqual(spans[5].name, 'router - /api/user/:id'); + assert.strictEqual(spans[5].kind, testUtils.OtlpSpanKind.INTERNAL); + assert.strictEqual(spans[5].parentSpanId, spans[1].spanId); + assert.strictEqual(spans[6].name, 'router - /:postId'); + assert.strictEqual(spans[6].kind, testUtils.OtlpSpanKind.INTERNAL); + assert.strictEqual(spans[6].parentSpanId, spans[1].spanId); assert.strictEqual( - spans[6].name, + spans[7].name, 'request handler - /api/user/:id/posts/:postId' ); - assert.strictEqual(spans[6].kind, SpanKind.SERVER); - assert.strictEqual(spans[6].parentSpanId, spans[0].spanId); + assert.strictEqual(spans[7].kind, testUtils.OtlpSpanKind.INTERNAL); + assert.strictEqual(spans[7].parentSpanId, spans[1].spanId); }, }); }); @@ -753,7 +755,7 @@ describe('ExpressInstrumentation', () => { spans[5].name, 'request handler - /\\/test\\/regex/' ); - assert.strictEqual(spans[5].kind, SpanKind.SERVER); + assert.strictEqual(spans[5].kind, testUtils.OtlpSpanKind.INTERNAL); assert.strictEqual(spans[5].parentSpanId, spans[1].spanId); }, }); From b1902dc08e067be20c884b1af3c4b23506b4906c Mon Sep 17 00:00:00 2001 From: Jamie Danielson Date: Mon, 16 Sep 2024 18:38:50 -0400 Subject: [PATCH 6/6] use testUtils for proper span kind --- .../test/express.test.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-express/test/express.test.ts b/plugins/node/opentelemetry-instrumentation-express/test/express.test.ts index 56d1098424..5991688ffd 100644 --- a/plugins/node/opentelemetry-instrumentation-express/test/express.test.ts +++ b/plugins/node/opentelemetry-instrumentation-express/test/express.test.ts @@ -662,16 +662,16 @@ describe('ExpressInstrumentation', () => { assert.strictEqual(spans[1].kind, testUtils.OtlpSpanKind.SERVER); assert.strictEqual(spans[1].parentSpanId, spans[0].spanId); assert.strictEqual(spans[2].name, 'middleware - query'); - assert.strictEqual(spans[3].kind, SpanKind.SERVER); + assert.strictEqual(spans[3].kind, testUtils.OtlpSpanKind.INTERNAL); assert.strictEqual(spans[3].parentSpanId, spans[1].spanId); assert.strictEqual(spans[4].name, 'middleware - simpleMiddleware'); - assert.strictEqual(spans[4].kind, SpanKind.SERVER); + assert.strictEqual(spans[4].kind, testUtils.OtlpSpanKind.INTERNAL); assert.strictEqual(spans[4].parentSpanId, spans[1].spanId); assert.strictEqual(spans[5].name, 'router - /api/user/:id'); - assert.strictEqual(spans[5].kind, SpanKind.SERVER); + assert.strictEqual(spans[5].kind, testUtils.OtlpSpanKind.INTERNAL); assert.strictEqual(spans[5].parentSpanId, spans[1].spanId); assert.strictEqual(spans[6].name, 'request handler - /api/user/:id'); - assert.strictEqual(spans[6].kind, SpanKind.SERVER); + assert.strictEqual(spans[6].kind, testUtils.OtlpSpanKind.INTERNAL); assert.strictEqual(spans[6].parentSpanId, spans[1].spanId); }, });