Skip to content

Commit

Permalink
fix(remix): Provide sentry-trace and baggage via root loader. (#5509
Browse files Browse the repository at this point in the history
)

We are currently wrapping [`meta` functions](https://remix.run/docs/en/v1/api/conventions#meta) to create `sentry-trace` and `baggage` `<meta>` tags in the server-side rendered HTML.

It all seems convenient to use that facility (not bothering users to configure anything through the whole process), but turns out it's breaking the hydration. While on React 17, it's just a warning (and `<meta>` tags are still available at the end). On React 18, hydration logic fails and falls back to client-side rendering. 

The problem is that the HTML template for hydration is generated on build time, and uses the `meta` functions before we wrap them. And when we finally wrap it on initial runtime (we are wrapping `createRequestHandler`), the updated template doesn't match.

But we also have `loader` functions available for us that can pass data from server to client-side, and their return values are also available in `meta` functions. Furthermore, using a `loader` data in `meta` seems to spare it from hydration and let it add `<meta>` tags whenever the data is available (which is `handleDocumentRequest` in this case, so just before the start of our `pageload` transaction).

Also, it turns out we don't need to add `<meta>` tags to every sub-route. If we add them to the `root` route, they will be available on the sub-routes. 

So, this PR removes the monkey patching logic for `meta` functions. Instead, introduces a special `loader` wrapper for `root`. This will require the users to set `sentry-trace` and `baggage` in `meta` functions in `root.tsx`.

It will look like:

```typescript
// root.tsx
export const meta: MetaFunction = ({data}) => {
    return {
        charset: "utf-8",
        title: "New Remix App",
        viewport: "width=device-width,initial-scale=1",
        'sentry-trace': data.sentryTrace,
        baggage: data.sentryBaggage,
    };
};
```

Co-authored-by: Abhijeet Prasad <aprasad@sentry.io>
  • Loading branch information
onurtemizkan and AbhiPrasad committed Aug 3, 2022
1 parent 9b7131f commit 31fd072
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 24 deletions.
54 changes: 31 additions & 23 deletions packages/remix/src/utils/instrumentServer.ts
@@ -1,7 +1,7 @@
/* eslint-disable max-lines */
import { captureException, getCurrentHub } from '@sentry/node';
import { getActiveTransaction } from '@sentry/tracing';
import { addExceptionMechanism, fill, loadModule, logger, serializeBaggage } from '@sentry/utils';
import { getActiveTransaction, hasTracingEnabled } from '@sentry/tracing';
import { addExceptionMechanism, fill, isNodeEnv, 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 @@ -72,6 +72,8 @@ interface HandleDataRequestFunction {

interface ServerEntryModule {
default: HandleDocumentRequestFunction;
meta: MetaFunction;
loader: DataFunction;
handleDataRequest?: HandleDataRequestFunction;
}

Expand Down Expand Up @@ -237,33 +239,31 @@ function makeWrappedLoader(origAction: DataFunction): DataFunction {
return makeWrappedDataFunction(origAction, 'loader');
}

function makeWrappedMeta(origMeta: MetaFunction | HtmlMetaDescriptor = {}): MetaFunction {
return function (
this: unknown,
args: { data: AppData; parentsData: RouteData; params: Params; location: Location },
): HtmlMetaDescriptor {
let origMetaResult;
if (origMeta instanceof Function) {
origMetaResult = origMeta.call(this, args);
} else {
origMetaResult = origMeta;
}
function getTraceAndBaggage(): { sentryTrace?: string; sentryBaggage?: string } {
const transaction = getActiveTransaction();
const currentScope = getCurrentHub().getScope();

const scope = getCurrentHub().getScope();
if (scope) {
const span = scope.getSpan();
const transaction = getActiveTransaction();
if (isNodeEnv() && hasTracingEnabled()) {
if (currentScope) {
const span = currentScope.getSpan();

if (span && transaction) {
return {
...origMetaResult,
'sentry-trace': span.toTraceparent(),
baggage: serializeBaggage(transaction.getBaggage()),
sentryTrace: span.toTraceparent(),
sentryBaggage: serializeBaggage(transaction.getBaggage()),
};
}
}
}

return origMetaResult;
return {};
}

function makeWrappedRootLoader(origLoader: DataFunction): DataFunction {
return async function (this: unknown, args: DataFunctionArgs): Promise<Response | AppData> {
const res = await origLoader.call(this, args);

return { ...res, ...getTraceAndBaggage() };
};
}

Expand Down Expand Up @@ -378,8 +378,6 @@ function makeWrappedCreateRequestHandler(
for (const [id, route] of Object.entries(build.routes)) {
const wrappedRoute = { ...route, module: { ...route.module } };

fill(wrappedRoute.module, 'meta', makeWrappedMeta);

if (wrappedRoute.module.action) {
fill(wrappedRoute.module, 'action', makeWrappedAction);
}
Expand All @@ -388,6 +386,16 @@ function makeWrappedCreateRequestHandler(
fill(wrappedRoute.module, 'loader', makeWrappedLoader);
}

// Entry module should have a loader function to provide `sentry-trace` and `baggage`
// They will be available for the root `meta` function as `data.sentryTrace` and `data.sentryBaggage`
if (!wrappedRoute.parentId) {
if (!wrappedRoute.module.loader) {
wrappedRoute.module.loader = () => ({});
}

fill(wrappedRoute.module, 'loader', makeWrappedRootLoader);
}

routes[id] = wrappedRoute;
}

Expand Down
4 changes: 3 additions & 1 deletion packages/remix/test/integration/app/root.tsx
Expand Up @@ -2,10 +2,12 @@ import type { MetaFunction } from '@remix-run/node';
import { Links, LiveReload, Meta, Outlet, Scripts, ScrollRestoration } from '@remix-run/react';
import { withSentry } from '@sentry/remix';

export const meta: MetaFunction = () => ({
export const meta: MetaFunction = ({ data }) => ({
charset: 'utf-8',
title: 'New Remix App',
viewport: 'width=device-width,initial-scale=1',
'sentry-trace': data.sentryTrace,
baggage: data.sentryBaggage,
});

function App() {
Expand Down
34 changes: 34 additions & 0 deletions packages/remix/test/integration/test/client/meta-tags.test.ts
@@ -1,4 +1,6 @@
import { test, expect } from '@playwright/test';
import { getFirstSentryEnvelopeRequest } from './utils/helpers';
import { Event } from '@sentry/types';

test('should inject `sentry-trace` and `baggage` meta tags inside the root page.', async ({ page }) => {
await page.goto('/');
Expand Down Expand Up @@ -27,3 +29,35 @@ test('should inject `sentry-trace` and `baggage` meta tags inside a parameterize

expect(sentryBaggageContent).toEqual(expect.any(String));
});

test('should send transactions with corresponding `sentry-trace` and `baggage` inside root page', async ({ page }) => {
const envelope = await getFirstSentryEnvelopeRequest<Event>(page, '/');

const sentryTraceTag = await page.$('meta[name="sentry-trace"]');
const sentryTraceContent = await sentryTraceTag?.getAttribute('content');
const sentryBaggageTag = await page.$('meta[name="baggage"]');
const sentryBaggageContent = await sentryBaggageTag?.getAttribute('content');

expect(sentryTraceContent).toContain(
`${envelope.contexts?.trace.trace_id}-${envelope.contexts?.trace.parent_span_id}-`,
);

expect(sentryBaggageContent).toContain(envelope.contexts?.trace.trace_id);
});

test('should send transactions with corresponding `sentry-trace` and `baggage` inside a parameterized route', async ({
page,
}) => {
const envelope = await getFirstSentryEnvelopeRequest<Event>(page, '/loader-json-response/0');

const sentryTraceTag = await page.$('meta[name="sentry-trace"]');
const sentryTraceContent = await sentryTraceTag?.getAttribute('content');
const sentryBaggageTag = await page.$('meta[name="baggage"]');
const sentryBaggageContent = await sentryBaggageTag?.getAttribute('content');

expect(sentryTraceContent).toContain(
`${envelope.contexts?.trace.trace_id}-${envelope.contexts?.trace.parent_span_id}-`,
);

expect(sentryBaggageContent).toContain(envelope.contexts?.trace.trace_id);
});

0 comments on commit 31fd072

Please sign in to comment.