Skip to content

Commit

Permalink
feat(remix): Migrate to opentelemetry-instrumentation-remix. (#12110)
Browse files Browse the repository at this point in the history
Ref: #11040

Migrates Remix server-side SDK to
[opentelemetry-instrumentation-remix](https://www.npmjs.com/package/opentelemetry-instrumentation-remix).

This PR also keeps the original implementation not the break the
developer experience for non-Express Remix projects.
Remix projects using Express are supported as is using the new
`autoInstrumentRemix` option.

**Usage with Express:**

```javascript
// instrument.(cjs | mjs)
const Sentry = require('@sentry/remix');

Sentry.init({
  dsn: YOUR_DSN
  // ...
  // auto instrument Remix with OpenTelemetry
  autoInstrumentRemix: true,
  // Optionally capture action formData attributes with errors.
  // This requires `sendDefaultPii` set to true as well.
  captureActionFormDataKeys: {
    file: true,
    text: true,
  },
  // To capture action formData attributes.
  sendDefaultPii: true
});
``` 

```javascript
// server.(cjs | mjs)
// import the Sentry instrumentation file before anything else.
import './instrument.cjs';
// alternatively `require('./instrument.cjs')`

// ...

const app = express();

// ...
```

**Usage with built-in Remix server:**

You need to run the Remix server with `NODE_OPTIONS=`--require(...)`
set.

```json
// package.json

// ...
  "scripts": {
    "start": "NODE_OPTIONS='--require=./instrument.server.cjs' remix-serve build"
    // or
    "start": "NODE_OPTIONS='--import=./instrument.server.mjs' remix-serve build"
  }
// ...
```

This PR _**removes**_:
- Express server adapter.
There is no need to use `wrapExpressCreateRequestHandler` anymore even
if you don't opt-in to `autoInstrumentRemix`.
`wrapExpressCreateRequestHandler` is kept exported as a no-op function.

- Built in HTTP incoming request instrumentation for both `legacy` and
`otel` implementations. Instead we mark `requestHandler` spans as the
root `http.server` spans.

When `autoInstrumentRemix` is set to `true`, this update _**replaces**_:

- Performance tracing on `action` / `loader` / `documentRequest`
functions. Leaving them to be traced by
`opentelemetry-instrumentation-remix`
- Request handler instrumentation as they are also traced by
`opentelemetry-instrumentation-remix`
- Auto-instrumentation for http as default integration for Remix SDK.


- With the new instrumentation, `pageload` span is the child of the
`loader` span. (Example:
[Trace](https://sentry-sdks.sentry.io/performance/trace/e51c640933786fd491bb428fb1eff826/?fov=0%2C436.5&node=error-cd2dc8610c5e4affb7356147daa77393&statsPeriod=7d&timestamp=1718029546))

- Legacy instrumentation keeps recording `pageload` span as the child of
the `http.server` span. (Example:
[Trace](https://sentry-sdks.sentry.io/performance/trace/01107a555eeee9b44d1f8c25ea0c45f3/?fov=0%2C818.599853515625&node=txn-0695246d026b4da792cf8f5e0a372b50&statsPeriod=7d&timestamp=1718103519))


Also:

Migrates Remix integration tests from Jest to Vitest.
- Related issues: jestjs/jest#15033
elastic/require-in-the-middle#50

Fixes Backlogged Issues:
- Fixes: #12082 -
[Code](https://github.com/getsentry/sentry-javascript/pull/12110/files#diff-754e32c1c14ac3c3c93eedeb7680548b986bc02a8b0bc63d2efc189210322acdR416-R440)
- Fixes: #9737 -
[Test](https://github.com/getsentry/sentry-javascript/pull/12110/files#diff-1484a5311699a814a39921d440f831e9babe69f8225a0de667ebd8cc63ca5accR109)
- Fixes: #12285
  • Loading branch information
onurtemizkan authored Jun 13, 2024
1 parent 073b649 commit 1201eb2
Show file tree
Hide file tree
Showing 119 changed files with 4,055 additions and 746 deletions.
7 changes: 4 additions & 3 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -917,10 +917,8 @@ jobs:
matrix:
node: [18, 20, 22]
remix: [1, 2]
# Remix v2 only supports Node 18+, so run Node 14, 16 tests separately
# Remix v2 only supports Node 18+, so run 16 tests separately
include:
- node: 14
remix: 1
- node: 16
remix: 1
steps:
Expand Down Expand Up @@ -1042,8 +1040,11 @@ jobs:
'create-react-app',
'create-next-app',
'create-remix-app',
'create-remix-app-legacy',
'create-remix-app-v2',
'create-remix-app-v2-legacy',
'create-remix-app-express',
'create-remix-app-express-legacy',
'create-remix-app-express-vite-dev',
'node-express-esm-loader',
'node-express-esm-preload',
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
/**
* This is intended to be a basic starting point for linting in your app.
* It relies on recommended configs out of the box for simplicity, but you can
* and should modify this configuration to best suit your team's needs.
*/

/** @type {import('eslint').Linter.Config} */
module.exports = {
root: true,
parserOptions: {
ecmaVersion: 'latest',
sourceType: 'module',
ecmaFeatures: {
jsx: true,
},
},
env: {
browser: true,
commonjs: true,
es6: true,
},

// Base config
extends: ['eslint:recommended'],

overrides: [
// React
{
files: ['**/*.{js,jsx,ts,tsx}'],
plugins: ['react', 'jsx-a11y'],
extends: [
'plugin:react/recommended',
'plugin:react/jsx-runtime',
'plugin:react-hooks/recommended',
'plugin:jsx-a11y/recommended',
],
settings: {
react: {
version: 'detect',
},
formComponents: ['Form'],
linkComponents: [
{ name: 'Link', linkAttribute: 'to' },
{ name: 'NavLink', linkAttribute: 'to' },
],
'import/resolver': {
typescript: {},
},
},
},

// Typescript
{
files: ['**/*.{ts,tsx}'],
plugins: ['@typescript-eslint', 'import'],
parser: '@typescript-eslint/parser',
settings: {
'import/internal-regex': '^~/',
'import/resolver': {
node: {
extensions: ['.ts', '.tsx'],
},
typescript: {
alwaysTryTypes: true,
},
},
},
extends: ['plugin:@typescript-eslint/recommended', 'plugin:import/recommended', 'plugin:import/typescript'],
},

// Node
{
files: ['.eslintrc.cjs', 'server.js'],
env: {
node: true,
},
},
],
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
node_modules

/.cache
/build
/public/build
.env
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
@sentry:registry=http://127.0.0.1:4873
@sentry-internal:registry=http://127.0.0.1:4873
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import { RemixBrowser, useLocation, useMatches } from '@remix-run/react';
import * as Sentry from '@sentry/remix';
import { StrictMode, startTransition, useEffect } from 'react';
import { hydrateRoot } from 'react-dom/client';

Sentry.init({
environment: 'qa', // dynamic sampling bias to keep transactions
dsn: window.ENV.SENTRY_DSN,
integrations: [
Sentry.browserTracingIntegration({
useEffect,
useLocation,
useMatches,
}),
Sentry.replayIntegration(),
],
// Performance Monitoring
tracesSampleRate: 1.0, // Capture 100% of the transactions, reduce in production!
replaysSessionSampleRate: 0.1, // This sets the sample rate at 10%. You may want to change it to 100% while in development and then sample at a lower rate in production.
replaysOnErrorSampleRate: 1.0, // If you're not already sampling the entire session, change the sample rate to 100% when sampling sessions where errors occur.
tunnel: 'http://localhost:3031/', // proxy server
});

Sentry.addEventProcessor(event => {
if (
event.type === 'transaction' &&
(event.contexts?.trace?.op === 'pageload' || event.contexts?.trace?.op === 'navigation')
) {
const eventId = event.event_id;
if (eventId) {
window.recordedTransactions = window.recordedTransactions || [];
window.recordedTransactions.push(eventId);
}
}

return event;
});

startTransition(() => {
hydrateRoot(
document,
<StrictMode>
<RemixBrowser />
</StrictMode>,
);
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
import * as Sentry from '@sentry/remix';

import { PassThrough } from 'node:stream';
import * as isbotModule from 'isbot';

import type { AppLoadContext, EntryContext } from '@remix-run/node';
import { createReadableStreamFromReadable } from '@remix-run/node';
import { installGlobals } from '@remix-run/node';
import { RemixServer } from '@remix-run/react';
import { renderToPipeableStream } from 'react-dom/server';

installGlobals();

const ABORT_DELAY = 5_000;

export const handleError = Sentry.wrapRemixHandleError;

export default function handleRequest(
request: Request,
responseStatusCode: number,
responseHeaders: Headers,
remixContext: EntryContext,
loadContext: AppLoadContext,
) {
return isBotRequest(request.headers.get('user-agent'))
? handleBotRequest(request, responseStatusCode, responseHeaders, remixContext)
: handleBrowserRequest(request, responseStatusCode, responseHeaders, remixContext);
}

// We have some Remix apps in the wild already running with isbot@3 so we need
// to maintain backwards compatibility even though we want new apps to use
// isbot@4. That way, we can ship this as a minor Semver update to @remix-run/dev.
function isBotRequest(userAgent: string | null) {
if (!userAgent) {
return false;
}

// isbot >= 3.8.0, >4
if ('isbot' in isbotModule && typeof isbotModule.isbot === 'function') {
return isbotModule.isbot(userAgent);
}

// isbot < 3.8.0
if ('default' in isbotModule && typeof isbotModule.default === 'function') {
return isbotModule.default(userAgent);
}

return false;
}

function handleBotRequest(
request: Request,
responseStatusCode: number,
responseHeaders: Headers,
remixContext: EntryContext,
) {
return new Promise((resolve, reject) => {
let shellRendered = false;
const { pipe, abort } = renderToPipeableStream(
<RemixServer context={remixContext} url={request.url} abortDelay={ABORT_DELAY} />,
{
onAllReady() {
shellRendered = true;
const body = new PassThrough();
const stream = createReadableStreamFromReadable(body);

responseHeaders.set('Content-Type', 'text/html');

resolve(
new Response(stream, {
headers: responseHeaders,
status: responseStatusCode,
}),
);

pipe(body);
},
onShellError(error: unknown) {
reject(error);
},
onError(error: unknown) {
responseStatusCode = 500;
// Log streaming rendering errors from inside the shell. Don't log
// errors encountered during initial shell rendering since they'll
// reject and get logged in handleDocumentRequest.
if (shellRendered) {
console.error(error);
}
},
},
);

setTimeout(abort, ABORT_DELAY);
});
}

function handleBrowserRequest(
request: Request,
responseStatusCode: number,
responseHeaders: Headers,
remixContext: EntryContext,
) {
return new Promise((resolve, reject) => {
let shellRendered = false;
const { pipe, abort } = renderToPipeableStream(
<RemixServer context={remixContext} url={request.url} abortDelay={ABORT_DELAY} />,
{
onShellReady() {
shellRendered = true;
const body = new PassThrough();
const stream = createReadableStreamFromReadable(body);

responseHeaders.set('Content-Type', 'text/html');

resolve(
new Response(stream, {
headers: responseHeaders,
status: responseStatusCode,
}),
);

pipe(body);
},
onShellError(error: unknown) {
reject(error);
},
onError(error: unknown) {
responseStatusCode = 500;
// Log streaming rendering errors from inside the shell. Don't log
// errors encountered during initial shell rendering since they'll
// reject and get logged in handleDocumentRequest.
if (shellRendered) {
console.error(error);
}
},
},
);

setTimeout(abort, ABORT_DELAY);
});
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
import { cssBundleHref } from '@remix-run/css-bundle';
import { LinksFunction, MetaFunction, json } from '@remix-run/node';
import {
Links,
LiveReload,
Meta,
Outlet,
Scripts,
ScrollRestoration,
useLoaderData,
useRouteError,
} from '@remix-run/react';
import { captureRemixErrorBoundaryError, withSentry } from '@sentry/remix';
import type { SentryMetaArgs } from '@sentry/remix';

export const links: LinksFunction = () => [...(cssBundleHref ? [{ rel: 'stylesheet', href: cssBundleHref }] : [])];

export const loader = () => {
return json({
ENV: {
SENTRY_DSN: process.env.E2E_TEST_DSN,
},
});
};

export const meta = ({ data }: SentryMetaArgs<MetaFunction<typeof loader>>) => {
return [
{
env: data.ENV,
},
{
name: 'sentry-trace',
content: data.sentryTrace,
},
{
name: 'baggage',
content: data.sentryBaggage,
},
];
};

export function ErrorBoundary() {
const error = useRouteError();
const eventId = captureRemixErrorBoundaryError(error);

return (
<div>
<span>ErrorBoundary Error</span>
<span id="event-id">{eventId}</span>
</div>
);
}

function App() {
const { ENV } = useLoaderData();

return (
<html lang="en">
<head>
<meta charSet="utf-8" />
<meta name="viewport" content="width=device-width,initial-scale=1" />
<script
dangerouslySetInnerHTML={{
__html: `window.ENV = ${JSON.stringify(ENV)}`,
}}
/>
<Meta />
<Links />
</head>
<body>
<Outlet />
<ScrollRestoration />
<Scripts />
<LiveReload />
</body>
</html>
);
}

export default withSentry(App);
Loading

0 comments on commit 1201eb2

Please sign in to comment.