Skip to content

fix(remix): Paramaterize server side transactions #5491

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

Merged
merged 5 commits into from
Aug 2, 2022
Merged
Show file tree
Hide file tree
Changes from all 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
103 changes: 90 additions & 13 deletions packages/remix/src/utils/instrumentServer.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,7 @@
/* eslint-disable max-lines */
import { captureException, getCurrentHub } from '@sentry/node';
import { getActiveTransaction } from '@sentry/tracing';
import {
addExceptionMechanism,
fill,
loadModule,
logger,
serializeBaggage,
stripUrlQueryAndFragment,
} from '@sentry/utils';
import { addExceptionMechanism, fill, loadModule, logger, serializeBaggage } from '@sentry/utils';

// Types vendored from @remix-run/server-runtime@1.6.0:
// https://github.com/remix-run/remix/blob/f3691d51027b93caa3fd2cdfe146d7b62a6eb8f2/packages/remix-server-runtime/server.ts
Expand Down Expand Up @@ -92,6 +85,18 @@ interface DataFunction {
(args: DataFunctionArgs): Promise<Response> | Response | Promise<AppData> | AppData;
}

interface ReactRouterDomPkg {
matchRoutes: (routes: ServerRoute[], pathname: string) => RouteMatch<ServerRoute>[] | null;
}

// Taken from Remix Implementation
// https://github.com/remix-run/remix/blob/97999d02493e8114c39d48b76944069d58526e8d/packages/remix-server-runtime/routeMatching.ts#L6-L10
export interface RouteMatch<Route> {
params: Params;
pathname: string;
route: Route;
}

// Taken from Remix Implementation
// https://github.com/remix-run/remix/blob/7688da5c75190a2e29496c78721456d6e12e3abe/packages/remix-server-runtime/responses.ts#L54-L62
// eslint-disable-next-line @typescript-eslint/no-explicit-any
Expand Down Expand Up @@ -262,18 +267,61 @@ function makeWrappedMeta(origMeta: MetaFunction | HtmlMetaDescriptor = {}): Meta
};
}

function wrapRequestHandler(origRequestHandler: RequestHandler): RequestHandler {
function createRoutes(manifest: ServerRouteManifest, parentId?: string): ServerRoute[] {
return Object.entries(manifest)
.filter(([, route]) => route.parentId === parentId)
.map(([id, route]) => ({
...route,
children: createRoutes(manifest, id),
}));
}

// Remix Implementation:
// https://github.com/remix-run/remix/blob/38e127b1d97485900b9c220d93503de0deb1fc81/packages/remix-server-runtime/routeMatching.ts#L12-L24
//
// Changed so that `matchRoutes` function is passed in.
function matchServerRoutes(
routes: ServerRoute[],
pathname: string,
pkg?: ReactRouterDomPkg,
): RouteMatch<ServerRoute>[] | null {
if (!pkg) {
return null;
}

const matches = pkg.matchRoutes(routes, pathname);
if (!matches) {
return null;
}

return matches.map(match => ({
params: match.params,
pathname: match.pathname,
route: match.route,
}));
}

function wrapRequestHandler(origRequestHandler: RequestHandler, build: ServerBuild): RequestHandler {
const routes = createRoutes(build.routes);
const pkg = loadModule<ReactRouterDomPkg>('react-router-dom');
return async function (this: unknown, request: Request, loadContext?: unknown): Promise<Response> {
const hub = getCurrentHub();
const currentScope = hub.getScope();

const url = new URL(request.url);
const matches = matchServerRoutes(routes, url.pathname, pkg);

const match = matches && getRequestMatch(url, matches);
const name = match === null ? url.pathname : match.route.id;
const source = match === null ? 'url' : 'route';
const transaction = hub.startTransaction({
name: stripUrlQueryAndFragment(request.url),
name,
op: 'http.server',
tags: {
method: request.method,
},
metadata: {
source: 'url',
source,
},
});

Expand All @@ -290,6 +338,33 @@ function wrapRequestHandler(origRequestHandler: RequestHandler): RequestHandler
};
}

// https://github.com/remix-run/remix/blob/97999d02493e8114c39d48b76944069d58526e8d/packages/remix-server-runtime/server.ts#L573-L586
function isIndexRequestUrl(url: URL): boolean {
for (const param of url.searchParams.getAll('index')) {
// only use bare `?index` params without a value
// ✅ /foo?index
// ✅ /foo?index&index=123
// ✅ /foo?index=123&index
// ❌ /foo?index=123
if (param === '') {
return true;
}
}

return false;
}

// https://github.com/remix-run/remix/blob/97999d02493e8114c39d48b76944069d58526e8d/packages/remix-server-runtime/server.ts#L588-L596
function getRequestMatch(url: URL, matches: RouteMatch<ServerRoute>[]): RouteMatch<ServerRoute> {
const match = matches.slice(-1)[0];

if (!isIndexRequestUrl(url) && match.route.id.endsWith('/index')) {
return matches.slice(-2)[0];
}

return match;
}

function makeWrappedCreateRequestHandler(
origCreateRequestHandler: CreateRequestHandlerFunction,
): CreateRequestHandlerFunction {
Expand All @@ -316,9 +391,11 @@ function makeWrappedCreateRequestHandler(
routes[id] = wrappedRoute;
}

const requestHandler = origCreateRequestHandler.call(this, { ...build, routes, entry: wrappedEntry }, mode);
const newBuild = { ...build, routes, entry: wrappedEntry };

const requestHandler = origCreateRequestHandler.call(this, newBuild, mode);

return wrapRequestHandler(requestHandler);
return wrapRequestHandler(requestHandler, newBuild);
};
}

Expand Down
8 changes: 6 additions & 2 deletions packages/remix/test/integration/test/server/loader.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,17 @@ describe('Remix API Loaders', () => {
const transaction = envelope[2];

assertSentryTransaction(transaction, {
transaction: 'routes/loader-json-response/$id',
transaction_info: {
source: 'route',
},
spans: [
{
description: url,
description: 'routes/loader-json-response/$id',
op: 'remix.server.loader',
},
{
description: url,
description: 'routes/loader-json-response/$id',
op: 'remix.server.documentRequest',
},
],
Expand Down