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

feat(remix): Switch to OTEL setup #593

Merged
merged 5 commits into from
Jun 19, 2024
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

## Unreleased

- feat(remix): Switch to OTEL setup (#593)
- deps: Bump glob to `10.4.2` (#599)

## 3.23.3
Expand Down
121 changes: 0 additions & 121 deletions src/remix/codemods/express-server.ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,9 @@
// @ts-expect-error - clack is ESM and TS complains about that. It works though
import clack from '@clack/prompts';
import chalk from 'chalk';
import * as recast from 'recast';
import { visit } from 'ast-types';
import {
ProxifiedImportItem,
generateCode,
loadFile,
writeFile,
// @ts-expect-error - magicast is ESM and TS complains about that. It works though
} from 'magicast';
import type { Program } from '@babel/types';
import * as fs from 'fs';

import { getInitCallInsertionIndex, hasSentryContent } from '../utils';
import { findFile } from '../../utils/ast-utils';

// Try to find the Express server implementation that contains `createRequestHandler` from `@remix-run/express`
Expand Down Expand Up @@ -52,114 +42,3 @@ export async function findCustomExpressServerImplementation() {

return null;
}

// Wrap createRequestHandler with `wrapExpressCreateRequestHandler` from `@sentry/remix`
export async function instrumentExpressCreateRequestHandler(
expressServerPath: string,
): Promise<boolean> {
const originalExpressServerMod = await loadFile(expressServerPath);

if (
hasSentryContent(
generateCode(originalExpressServerMod.$ast).code,
originalExpressServerMod.$code,
)
) {
clack.log.warn(
`Express server in ${chalk.cyan(
expressServerPath,
)} already has Sentry instrumentation. Skipping.`,
);

return false;
}

originalExpressServerMod.imports.$add({
from: '@sentry/remix',
imported: 'wrapExpressCreateRequestHandler',
local: 'wrapExpressCreateRequestHandler',
});

const createRequestHandlerImport =
originalExpressServerMod.imports.$items.find(
(imp) =>
imp.from === '@remix-run/express' &&
imp.imported === 'createRequestHandler',
);

visit(originalExpressServerMod.$ast, {
visitIdentifier(path) {
if (
path.value.name === 'createRequestHandler' &&
path.parentPath.value.type === 'CallExpression'
) {
path.value.name = 'sentryCreateRequestHandler';
}

this.traverse(path);
},
});

// Insert the const declaration right after the imports
// Where we want to insert the const declaration is the same as where we would want to insert the init call.
const insertionIndex = getInitCallInsertionIndex(
originalExpressServerMod.$ast as Program,
);

const createRequestHandlerConst = wrapCreateRequestHandlerWithSentry(
createRequestHandlerImport,
);

if (!createRequestHandlerConst) {
// Todo: throw error
}

(originalExpressServerMod.$ast as Program).body.splice(
insertionIndex,
0,
// @ts-expect-error - string works here because the AST is proxified by magicast
// eslint-disable-next-line @typescript-eslint/no-unsafe-argument
createRequestHandlerConst,
);

try {
await writeFile(originalExpressServerMod.$ast, expressServerPath);

clack.log.info(
`Successfully instrumented Express server in ${chalk.cyan(
expressServerPath,
)}.`,
);
} catch (e) {
clack.log.warn(
`Could not write to Express server in ${chalk.cyan(expressServerPath)}.`,
);

throw e;
}

return true;
}

// Wrap `createRequestHandler` with `wrapExpressCreateRequestHandler` and set const name to `sentryCreateRequestHandler`
export function wrapCreateRequestHandlerWithSentry(
createRequestHandlerImport: ProxifiedImportItem | undefined,
) {
if (!createRequestHandlerImport) {
return;
}

const createRequestHandler = createRequestHandlerImport.local;

const wrapCreateRequestHandler = recast.types.builders.callExpression(
recast.types.builders.identifier('wrapExpressCreateRequestHandler'),
[recast.types.builders.identifier(createRequestHandler)],
);

return recast.types.builders.variableDeclaration('const', [
recast.types.builders.variableDeclarator(
recast.types.builders.identifier('sentryCreateRequestHandler'),
wrapCreateRequestHandler,
),
]);
}
5 changes: 3 additions & 2 deletions src/remix/codemods/handle-error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import type { Program } from '@babel/types';
import * as recast from 'recast';

import { HANDLE_ERROR_TEMPLATE_V2 } from '../templates';
import { getInitCallInsertionIndex, hasSentryContent } from '../utils';
import { getAfterImportsInsertionIndex, hasSentryContent } from '../utils';

// @ts-expect-error - clack is ESM and TS complains about that. It works though
import clack from '@clack/prompts';
Expand Down Expand Up @@ -41,7 +41,7 @@ export function instrumentHandleError(
.body[0];

originalEntryServerModAST.body.splice(
getInitCallInsertionIndex(originalEntryServerModAST),
getAfterImportsInsertionIndex(originalEntryServerModAST),
0,
// @ts-expect-error - string works here because the AST is proxified by magicast
// eslint-disable-next-line @typescript-eslint/no-unsafe-argument
Expand All @@ -51,6 +51,7 @@ export function instrumentHandleError(
hasSentryContent(
generateCode(handleErrorFunction).code,
originalEntryServerMod.$code,
'captureRemixServerException',
)
) {
return false;
Expand Down
52 changes: 26 additions & 26 deletions src/remix/remix-wizard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import {
isRemixV2,
loadRemixConfig,
runRemixReveal,
instrumentExpressServer,
insertServerInstrumentationFile,
} from './sdk-setup';
import { debug } from '../utils/debug';
import { traceStep, withTelemetry } from '../telemetry';
Expand Down Expand Up @@ -145,34 +145,34 @@ async function runRemixWizardWithTelemetry(
}
});

await traceStep('Initialize Sentry on server entry', async () => {
try {
await initializeSentryOnEntryServer(dsn, isV2, isTS);
} catch (e) {
clack.log.warn(`Could not initialize Sentry on server entry.
Please do it manually using instructions from https://docs.sentry.io/platforms/javascript/guides/remix/manual-setup/`);
debug(e);
}
});
let customServerInstrumented = false;

await traceStep('Instrument custom Express server', async () => {
try {
const hasExpressAdapter = hasPackageInstalled(
'@remix-run/express',
packageJson,
);

if (!hasExpressAdapter) {
return;
await traceStep(
'Create server instrumentation file and import it',
async () => {
try {
customServerInstrumented = await insertServerInstrumentationFile(dsn);
} catch (e) {
clack.log.warn(
'Could not create a server instrumentation file. Please do it manually using instructions from https://docs.sentry.io/platforms/javascript/guides/remix/manual-setup/',
);
debug(e);
}
},
);

await instrumentExpressServer();
} catch (e) {
clack.log.warn(`Could not instrument custom Express server.
Please do it manually using instructions from https://docs.sentry.io/platforms/javascript/guides/remix/manual-setup/#custom-express-server`);
debug(e);
}
});
if (!customServerInstrumented) {
await traceStep('Initialize Sentry on server entry', async () => {
try {
await initializeSentryOnEntryServer(dsn, isV2, isTS);
} catch (e) {
clack.log
.warn(`Could not automatically add Sentry initialization to server entry.
Please do it manually using instructions from https://docs.sentry.io/platforms/javascript/guides/remix/manual-setup/`);
debug(e);
}
});
}

const shouldCreateExamplePage = await askShouldCreateExamplePage();

Expand Down
2 changes: 0 additions & 2 deletions src/remix/sdk-example.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,6 @@ export function getSentryExamplePageContents(options: {
: `https://${options.orgSlug}.sentry.io/issues/?project=${options.projectId}`;

return `
import * as Sentry from '@sentry/remix';

export default function SentryExamplePage() {
return (
<div>
Expand Down
113 changes: 91 additions & 22 deletions src/remix/sdk-setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,26 @@ import clack from '@clack/prompts';
import chalk from 'chalk';
import { gte, minVersion } from 'semver';

// @ts-expect-error - magicast is ESM and TS complains about that. It works though
import { builders, generateCode, loadFile, writeFile } from 'magicast';
import {
builders,
generateCode,
loadFile,
parseModule,
writeFile,
// @ts-expect-error - magicast is ESM and TS complains about that. It works though
} from 'magicast';
import { PackageDotJson, getPackageVersion } from '../utils/package-json';
import { getInitCallInsertionIndex, hasSentryContent } from './utils';
import {
getAfterImportsInsertionIndex,
getBeforeImportsInsertionIndex,
hasSentryContent,
serverHasInstrumentationImport,
} from './utils';
import { instrumentRootRouteV1 } from './codemods/root-v1';
import { instrumentRootRouteV2 } from './codemods/root-v2';
import { instrumentHandleError } from './codemods/handle-error';
import {
findCustomExpressServerImplementation,
instrumentExpressCreateRequestHandler,
} from './codemods/express-server';
import { getPackageDotJson } from '../utils/clack-utils';
import { findCustomExpressServerImplementation } from './codemods/express-server';

export type PartialRemixConfig = {
unstable_dev?: boolean;
Expand Down Expand Up @@ -87,7 +95,8 @@ function insertClientInitCall(
});

const originalHooksModAST = originalHooksMod.$ast as Program;
const initCallInsertionIndex = getInitCallInsertionIndex(originalHooksModAST);
const initCallInsertionIndex =
getAfterImportsInsertionIndex(originalHooksModAST);

originalHooksModAST.body.splice(
initCallInsertionIndex,
Expand All @@ -109,7 +118,8 @@ function insertServerInitCall(

const originalHooksModAST = originalHooksMod.$ast as Program;

const initCallInsertionIndex = getInitCallInsertionIndex(originalHooksModAST);
const initCallInsertionIndex =
getBeforeImportsInsertionIndex(originalHooksModAST);

originalHooksModAST.body.splice(
initCallInsertionIndex,
Expand All @@ -120,6 +130,78 @@ function insertServerInitCall(
);
}

export async function createServerInstrumentationFile(dsn: string) {
// create an empty file named `instrument.server.mjs`
const instrumentationFile = 'instrumentation.server.mjs';
const instrumentationFileMod = parseModule('');

instrumentationFileMod.imports.$add({
from: '@sentry/remix',
imported: '*',
local: 'Sentry',
});

const initCall = builders.functionCall('Sentry.init', {
dsn,
tracesSampleRate: 1.0,
autoInstrumentRemix: true,
});

const instrumentationFileModAST = instrumentationFileMod.$ast as Program;

const initCallInsertionIndex = getAfterImportsInsertionIndex(
instrumentationFileModAST,
);

instrumentationFileModAST.body.splice(
initCallInsertionIndex,
0,
// @ts-expect-error - string works here because the AST is proxified by magicast
// eslint-disable-next-line @typescript-eslint/no-unsafe-argument
generateCode(initCall).code,
);

await writeFile(instrumentationFileModAST, instrumentationFile);

return instrumentationFile;
}

export async function insertServerInstrumentationFile(dsn: string) {
const instrumentationFile = await createServerInstrumentationFile(dsn);

const expressServerPath = await findCustomExpressServerImplementation();

if (!expressServerPath) {
return false;
}

const originalExpressServerMod = await loadFile(expressServerPath);

if (
serverHasInstrumentationImport(
expressServerPath,
originalExpressServerMod.$code,
)
) {
clack.log.warn(
`File ${chalk.cyan(
path.basename(expressServerPath),
)} already contains instrumentation import.
Skipping adding instrumentation functionality to ${chalk.cyan(
path.basename(expressServerPath),
)}.`,
);

return true;
}

originalExpressServerMod.$code = `import './${instrumentationFile}';\n${originalExpressServerMod.$code}`;

fs.writeFileSync(expressServerPath, originalExpressServerMod.$code);

return true;
}

export function isRemixV2(
remixConfig: PartialRemixConfig,
packageJson: PackageDotJson,
Expand Down Expand Up @@ -347,16 +429,3 @@ export async function initializeSentryOnEntryServer(
)}.`,
);
}

export async function instrumentExpressServer() {
const expressServerPath = await findCustomExpressServerImplementation();

if (!expressServerPath) {
clack.log.warn(
`Could not find custom Express server implementation. Please instrument it manually.`,
);
return;
}

await instrumentExpressCreateRequestHandler(expressServerPath);
}
Loading
Loading