Skip to content

Commit a49a39b

Browse files
committed
handle route naming
1 parent 5d8b843 commit a49a39b

File tree

4 files changed

+366
-17
lines changed

4 files changed

+366
-17
lines changed

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

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@ import {
3232
getLayerPath,
3333
isLayerIgnored,
3434
storeLayerPath,
35+
getActualMatchedRoute,
36+
getConstructedRoute,
3537
} from './utils';
3638
/** @knipignore */
3739
import { PACKAGE_NAME, PACKAGE_VERSION } from './version';
@@ -172,25 +174,21 @@ export class ExpressInstrumentation extends InstrumentationBase<ExpressInstrumen
172174
res: express.Response
173175
) {
174176
storeLayerPath(req, layerPath);
175-
const route = (req[_LAYERS_STORE_PROPERTY] as string[])
176-
.filter(path => path !== '/' && path !== '/*')
177-
.join('')
178-
// remove duplicate slashes to normalize route
179-
.replace(/\/{2,}/g, '/');
180177

181-
const actualRoute = route.length > 0 ? route : undefined;
178+
const constructedRoute = getConstructedRoute(req);
179+
const actualMatchedRoute = getActualMatchedRoute(req);
182180

183181
const attributes: Attributes = {
184-
[SEMATTRS_HTTP_ROUTE]: actualRoute,
182+
[SEMATTRS_HTTP_ROUTE]: actualMatchedRoute,
185183
};
186-
const metadata = getLayerMetadata(route, layer, layerPath);
184+
const metadata = getLayerMetadata(constructedRoute, layer, layerPath);
187185
const type = metadata.attributes[
188186
AttributeNames.EXPRESS_TYPE
189187
] as ExpressLayerType;
190188

191189
const rpcMetadata = getRPCMetadata(context.active());
192190
if (rpcMetadata?.type === RPCType.HTTP) {
193-
rpcMetadata.route = actualRoute;
191+
rpcMetadata.route = actualMatchedRoute;
194192
}
195193

196194
// verify against the config if the layer should be ignored
@@ -209,7 +207,7 @@ export class ExpressInstrumentation extends InstrumentationBase<ExpressInstrumen
209207
{
210208
request: req,
211209
layerType: type,
212-
route,
210+
route: constructedRoute,
213211
},
214212
metadata.name
215213
);
@@ -224,7 +222,7 @@ export class ExpressInstrumentation extends InstrumentationBase<ExpressInstrumen
224222
requestHook(span, {
225223
request: req,
226224
layerType: type,
227-
route,
225+
route: constructedRoute,
228226
}),
229227
e => {
230228
if (e) {

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

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,3 +207,81 @@ const extractLayerPathSegment = (arg: LayerPathSegment) => {
207207

208208
return;
209209
};
210+
211+
export function getConstructedRoute(req: {
212+
originalUrl: PatchedRequest['originalUrl'];
213+
[_LAYERS_STORE_PROPERTY]?: string[];
214+
}) {
215+
const layersStore: string[] = Array.isArray(req[_LAYERS_STORE_PROPERTY])
216+
? (req[_LAYERS_STORE_PROPERTY] as string[])
217+
: [];
218+
219+
const meaningfulPaths = layersStore.filter(
220+
path => path !== '/' && path !== '/*'
221+
);
222+
223+
// Handle standalone wildcard case
224+
if (meaningfulPaths.length === 1 && meaningfulPaths[0] === '*') {
225+
return '*';
226+
}
227+
228+
// Construct the route by joining paths and normalizing
229+
return meaningfulPaths.join('').replace(/\/{2,}/g, '/'); // Remove duplicate slashes
230+
}
231+
232+
/**
233+
* Extracts the actual matched route from Express request for OpenTelemetry instrumentation.
234+
* Returns the route that should be used as the http.route attribute.
235+
*
236+
* @param req - The Express request object with layers store
237+
* @param layersStoreProperty - The property name where layer paths are stored
238+
* @returns The matched route string or undefined if no valid route is found
239+
*/
240+
export function getActualMatchedRoute(req: {
241+
originalUrl: PatchedRequest['originalUrl'];
242+
[_LAYERS_STORE_PROPERTY]?: string[];
243+
}): string | undefined {
244+
const layersStore: string[] = Array.isArray(req[_LAYERS_STORE_PROPERTY])
245+
? (req[_LAYERS_STORE_PROPERTY] as string[])
246+
: [];
247+
248+
// If no layers are stored, no route can be determined
249+
if (layersStore.length === 0) {
250+
return undefined;
251+
}
252+
253+
// Handle root path case - if all paths are root, only return root if originalUrl is also root
254+
if (layersStore.every(path => path === '/')) {
255+
return req.originalUrl === '/' ? '/' : undefined;
256+
}
257+
258+
const constructedRoute = getConstructedRoute(req);
259+
if (constructedRoute === '*') {
260+
return constructedRoute;
261+
}
262+
263+
// Ensure route starts with '/' if it doesn't already
264+
const normalizedRoute = constructedRoute.startsWith('/')
265+
? constructedRoute
266+
: `/${constructedRoute}`;
267+
268+
// Validate that this appears to be a matched route
269+
// A route is considered matched if:
270+
// 1. We have a constructed route
271+
// 2. The original URL matches or starts with our route pattern
272+
const isValidRoute =
273+
normalizedRoute.length > 0 &&
274+
(req.originalUrl === normalizedRoute ||
275+
req.originalUrl.startsWith(normalizedRoute) ||
276+
isRoutePattern(normalizedRoute));
277+
278+
return isValidRoute ? normalizedRoute : undefined;
279+
}
280+
281+
/**
282+
* Checks if a route contains parameter patterns (e.g., :id, :userId)
283+
* which are valid even if they don't exactly match the original URL
284+
*/
285+
function isRoutePattern(route: string): boolean {
286+
return route.includes(':') || route.includes('*');
287+
}

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

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -85,13 +85,17 @@ describe('ExpressInstrumentation', () => {
8585
const spans = memoryExporter.getFinishedSpans();
8686

8787
// 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')
88+
const middlewareSpans = spans.filter(
89+
span =>
90+
span.name.includes('middleware') ||
91+
span.name.includes('expressInit') ||
92+
span.name.includes('jsonParser')
9293
);
9394

94-
assert.ok(middlewareSpans.length > 0, 'Middleware spans should be created');
95+
assert.ok(
96+
middlewareSpans.length > 0,
97+
'Middleware spans should be created'
98+
);
9599

96100
for (const span of spans) {
97101
assert.strictEqual(

0 commit comments

Comments
 (0)