diff --git a/plugins/node/opentelemetry-instrumentation-express/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-express/src/instrumentation.ts index c3f5602cac..7aeb400ab2 100644 --- a/plugins/node/opentelemetry-instrumentation-express/src/instrumentation.ts +++ b/plugins/node/opentelemetry-instrumentation-express/src/instrumentation.ts @@ -315,7 +315,11 @@ export class ExpressInstrumentation extends InstrumentationBase { // some properties holding metadata and state so we need to proxy them // through through patched function // ref: https://github.com/open-telemetry/opentelemetry-js-contrib/issues/1950 - Object.keys(original).forEach(key => { + // Also some apps/libs do their own patching before OTEL and have these properties + // in the proptotype. So we use a `for...in` loop to get own properties and also + // any enumerable prop in the prototype chain + // ref: https://github.com/open-telemetry/opentelemetry-js-contrib/issues/2271 + for (const key in original) { Object.defineProperty(patched, key, { get() { return original[key]; @@ -324,8 +328,7 @@ export class ExpressInstrumentation extends InstrumentationBase { original[key] = value; }, }); - }); - + } return patched; }); } diff --git a/plugins/node/opentelemetry-instrumentation-express/test/express.test.ts b/plugins/node/opentelemetry-instrumentation-express/test/express.test.ts index e13c13a424..43b5da1324 100644 --- a/plugins/node/opentelemetry-instrumentation-express/test/express.test.ts +++ b/plugins/node/opentelemetry-instrumentation-express/test/express.test.ts @@ -517,6 +517,47 @@ describe('ExpressInstrumentation', () => { } ); }); + + it('should keep the handle properties even if router is patched before instrumentation does it', async () => { + const rootSpan = tracer.startSpan('rootSpan'); + let routerLayer: { name: string; handle: { stack: any[] } }; + + const expressApp = express(); + const router = express.Router(); + const CustomRouter: (...p: Parameters) => void = ( + req, + res, + next + ) => router(req, res, next); + router.use('/:slug', (req, res, next) => { + const stack = req.app._router.stack as any[]; + routerLayer = stack.find(router => router.name === 'CustomRouter'); + return res.status(200).end('bar'); + }); + // The patched router now has express router's own properties in its prototype so + // they are not accessible through `Object.keys(...)` + // https://github.com/TryGhost/Ghost/blob/fefb9ec395df8695d06442b6ecd3130dae374d94/ghost/core/core/frontend/web/site.js#L192 + Object.setPrototypeOf(CustomRouter, router); + expressApp.use(CustomRouter); + + const httpServer = await createServer(expressApp); + server = httpServer.server; + port = httpServer.port; + await context.with( + trace.setSpan(context.active(), rootSpan), + async () => { + const response = await httpRequest.get( + `http://localhost:${port}/foo` + ); + assert.strictEqual(response, 'bar'); + rootSpan.end(); + assert.ok( + routerLayer.handle.stack.length === 1, + 'router layer stack is accessible' + ); + } + ); + }); }); describe('Disabling plugin', () => {