Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(node): Adjust Express URL parameterization for array routes #5495

Merged
merged 9 commits into from
Aug 1, 2022
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@ app.get(/\/test\/regex/, (_req, res) => {
res.send({ response: 'response 2' });
});

app.get(['/test/array1', /\/test\/array[2-9]/], (_req, res) => {
Lms24 marked this conversation as resolved.
Show resolved Hide resolved
res.send({ response: 'response 3' });
});

app.use(Sentry.Handlers.errorHandler());

export default app;
29 changes: 29 additions & 0 deletions packages/node-integration-tests/suites/express/tracing/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,3 +53,32 @@ test('should set a correct transaction name for routes specified in RegEx', asyn
},
});
});

test.each([
['', 'array1'],
['', 'array5'],
])('%s should set a correct transaction name for routes consisting of arrays of routes', async (_, segment) => {
Lms24 marked this conversation as resolved.
Show resolved Hide resolved
const url = await runServer(__dirname, `${__dirname}/server.ts`);
const envelope = await getEnvelopeRequest(`${url}/${segment}`);

expect(envelope).toHaveLength(3);

assertSentryTransaction(envelope[2], {
transaction: 'GET /test/array1,/\\/test\\/array[2-9]',
transaction_info: {
source: 'route',
},
contexts: {
trace: {
data: {
url: `/test/${segment}`,
},
op: 'http.server',
status: 'ok',
tags: {
'http.status_code': '200',
},
},
},
});
});
69 changes: 59 additions & 10 deletions packages/tracing/src/integrations/node/express.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ type Router = {
/* Extend the CrossPlatformRequest type with a patched parameter to build a reconstructed route */
type PatchedRequest = CrossPlatformRequest & { _reconstructedRoute?: string };

/* Type used for pathing the express router prototype */
/* Types used for patching the express router prototype */
type ExpressRouter = Router & {
_router?: ExpressRouter;
stack?: Layer[];
Expand All @@ -51,14 +51,15 @@ type ExpressRouter = Router & {
) => unknown;
};

/* Type used for pathing the express router prototype */
type Layer = {
match: (path: string) => boolean;
handle_request: (req: PatchedRequest, res: ExpressResponse, next: () => void) => void;
route?: { path: string | RegExp };
route?: { path: RouteType | RouteType[] };
path?: string;
};

type RouteType = string | RegExp;

interface ExpressResponse {
once(name: string, callback: () => void): void;
}
Expand Down Expand Up @@ -273,11 +274,8 @@ function instrumentRouter(appOrRouter: ExpressRouter): void {
req._reconstructedRoute = '';
}

// If the layer's partial route has params, the route is stored in layer.route.
// Since a route might be defined with a RegExp, we convert it toString to make sure we end up with a string
const lrp = layer.route?.path;
const isRegex = isRegExp(lrp);
const layerRoutePath = isRegex ? lrp?.toString() : (lrp as string);
// If the layer's partial route has params, is a regex or an array, the route is stored in layer.route.
const { layerRoutePath, isRegex, numExtraSegments }: LayerRoutePathInfo = getLayerRoutePathString(layer);

// Otherwise, the hardcoded path (i.e. a partial route without params) is stored in layer.path
const partialRoute = layerRoutePath || layer.path || '';
Expand All @@ -301,7 +299,7 @@ function instrumentRouter(appOrRouter: ExpressRouter): void {
// Now we check if we are in the "last" part of the route. We determine this by comparing the
// number of URL segments from the original URL to that of our reconstructed parameterized URL.
// If we've reached our final destination, we update the transaction name.
const urlLength = getNumberOfUrlSegments(req.originalUrl || '');
const urlLength = getNumberOfUrlSegments(req.originalUrl || '') + numExtraSegments;
const routeLength = getNumberOfUrlSegments(req._reconstructedRoute);

if (urlLength === routeLength) {
Expand All @@ -319,7 +317,58 @@ function instrumentRouter(appOrRouter: ExpressRouter): void {
};
}

type LayerRoutePathInfo = {
layerRoutePath?: string;
isRegex: boolean;
numExtraSegments: number;
};

/**
* Extracts and stringifies the layer's route which can either be a string with parameters (`users/:id`),
* a RegEx (`/test/`) or an array of strings and regexes (`['/path1', /\/path[2-5]/, /path/:id]`).
*
* @param layer the layer to extract the stringified route from
*
* @returns an object containing the stringified route, a flag determining if the route was a regex
* and the number of extra segments to the matched path that are additionally in the route,
* if the route was an array (defaults to 0).
*/
function getLayerRoutePathString(layer: Layer): LayerRoutePathInfo {
const lrp = layer.route?.path;

const isRegex = isRegExp(lrp);
const isArray = Array.isArray(lrp);

if (!lrp) {
return { isRegex, numExtraSegments: 0 };
}

const numExtraSegments = isArray
? getNumberOfArrayUrlSegments(lrp as RouteType[]) - getNumberOfUrlSegments(layer.path || '')
Lms24 marked this conversation as resolved.
Show resolved Hide resolved
: 0;

const layerRoutePath = isArray
? (lrp as RouteType[]).map(r => r.toString()).join(',')
: isRegex
? lrp.toString()
: (lrp as string | undefined);
Lms24 marked this conversation as resolved.
Show resolved Hide resolved

return { layerRoutePath, isRegex, numExtraSegments };
}

/**
* Returns the number of URL segments in an array of routes
*
* Example: ['/api/test', /\/api\/post[0-9]/, '/users/:id/details`] -> 7
*/
function getNumberOfArrayUrlSegments(routesArray: RouteType[]): number {
return routesArray.reduce((accNumSegments: number, currentRoute: RouteType) => {
// array members can be a RegEx -> convert them toString
return accNumSegments + getNumberOfUrlSegments(currentRoute.toString());
}, 0);
}

function getNumberOfUrlSegments(url: string): number {
// split at '/' or at '\/' to split regex urls correctly
return url.split(/\\?\//).filter(s => s.length > 0).length;
return url.split(/\\?\//).filter(s => s.length > 0 && s !== ',').length;
}