Skip to content

Commit 5d8b843

Browse files
committed
fix(express): Ensure 404 routes don't attach route attribute
1 parent 5ecbd03 commit 5d8b843

File tree

2 files changed

+44
-2
lines changed

2 files changed

+44
-2
lines changed

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -178,8 +178,10 @@ export class ExpressInstrumentation extends InstrumentationBase<ExpressInstrumen
178178
// remove duplicate slashes to normalize route
179179
.replace(/\/{2,}/g, '/');
180180

181+
const actualRoute = route.length > 0 ? route : undefined;
182+
181183
const attributes: Attributes = {
182-
[SEMATTRS_HTTP_ROUTE]: route.length > 0 ? route : '/',
184+
[SEMATTRS_HTTP_ROUTE]: actualRoute,
183185
};
184186
const metadata = getLayerMetadata(route, layer, layerPath);
185187
const type = metadata.attributes[
@@ -188,7 +190,7 @@ export class ExpressInstrumentation extends InstrumentationBase<ExpressInstrumen
188190

189191
const rpcMetadata = getRPCMetadata(context.active());
190192
if (rpcMetadata?.type === RPCType.HTTP) {
191-
rpcMetadata.route = route || '/';
193+
rpcMetadata.route = actualRoute;
192194
}
193195

194196
// verify against the config if the layer should be ignored

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

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,46 @@ describe('ExpressInstrumentation', () => {
6363
server?.close();
6464
});
6565

66+
it('does not attach semantic route attribute for 404 page', async () => {
67+
const rootSpan = tracer.startSpan('rootSpan');
68+
const httpServer = await serverWithMiddleware(tracer, rootSpan, app => {
69+
app.use(express.json());
70+
});
71+
server = httpServer.server;
72+
port = httpServer.port;
73+
assert.strictEqual(memoryExporter.getFinishedSpans().length, 0);
74+
75+
await context.with(
76+
trace.setSpan(context.active(), rootSpan),
77+
async () => {
78+
try {
79+
await httpRequest.get(
80+
`http://localhost:${port}/non-existing-route`
81+
);
82+
} catch (error) {}
83+
rootSpan.end();
84+
85+
const spans = memoryExporter.getFinishedSpans();
86+
87+
// Should have middleware spans but no request handler span
88+
const middlewareSpans = spans.filter(span =>
89+
span.name.includes('middleware') ||
90+
span.name.includes('expressInit') ||
91+
span.name.includes('jsonParser')
92+
);
93+
94+
assert.ok(middlewareSpans.length > 0, 'Middleware spans should be created');
95+
96+
for (const span of spans) {
97+
assert.strictEqual(
98+
span.attributes[SEMATTRS_HTTP_ROUTE],
99+
undefined, // none of the spans have the HTTP route attribute
100+
`Span "${span.name}" should not have HTTP route attribute for non-existing route`
101+
);
102+
}
103+
}
104+
);
105+
});
66106
it('should create a child span for middlewares', async () => {
67107
const rootSpan = tracer.startSpan('rootSpan');
68108
const customMiddleware: express.RequestHandler = (req, res, next) => {

0 commit comments

Comments
 (0)