diff --git a/CHANGELOG.md b/CHANGELOG.md index 12c063ca..f0710703 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## Unreleased +- feat(remix): Switch to OTEL setup (#593) - deps: Bump glob to `10.4.2` (#599) ## 3.23.3 diff --git a/src/remix/codemods/express-server.ts b/src/remix/codemods/express-server.ts index cf274397..587d9487 100644 --- a/src/remix/codemods/express-server.ts +++ b/src/remix/codemods/express-server.ts @@ -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` @@ -52,114 +42,3 @@ export async function findCustomExpressServerImplementation() { return null; } - -// Wrap createRequestHandler with `wrapExpressCreateRequestHandler` from `@sentry/remix` -export async function instrumentExpressCreateRequestHandler( - expressServerPath: string, -): Promise { - 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, - ), - ]); -} diff --git a/src/remix/codemods/handle-error.ts b/src/remix/codemods/handle-error.ts index 7557b54c..e2cb7843 100644 --- a/src/remix/codemods/handle-error.ts +++ b/src/remix/codemods/handle-error.ts @@ -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'; @@ -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 @@ -51,6 +51,7 @@ export function instrumentHandleError( hasSentryContent( generateCode(handleErrorFunction).code, originalEntryServerMod.$code, + 'captureRemixServerException', ) ) { return false; diff --git a/src/remix/remix-wizard.ts b/src/remix/remix-wizard.ts index caf1c3bc..7b0343dc 100644 --- a/src/remix/remix-wizard.ts +++ b/src/remix/remix-wizard.ts @@ -24,7 +24,7 @@ import { isRemixV2, loadRemixConfig, runRemixReveal, - instrumentExpressServer, + insertServerInstrumentationFile, } from './sdk-setup'; import { debug } from '../utils/debug'; import { traceStep, withTelemetry } from '../telemetry'; @@ -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(); diff --git a/src/remix/sdk-example.ts b/src/remix/sdk-example.ts index b252a28d..cbdfec64 100644 --- a/src/remix/sdk-example.ts +++ b/src/remix/sdk-example.ts @@ -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 (
diff --git a/src/remix/sdk-setup.ts b/src/remix/sdk-setup.ts index ea729b2a..215eb8ed 100644 --- a/src/remix/sdk-setup.ts +++ b/src/remix/sdk-setup.ts @@ -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; @@ -87,7 +95,8 @@ function insertClientInitCall( }); const originalHooksModAST = originalHooksMod.$ast as Program; - const initCallInsertionIndex = getInitCallInsertionIndex(originalHooksModAST); + const initCallInsertionIndex = + getAfterImportsInsertionIndex(originalHooksModAST); originalHooksModAST.body.splice( initCallInsertionIndex, @@ -109,7 +118,8 @@ function insertServerInitCall( const originalHooksModAST = originalHooksMod.$ast as Program; - const initCallInsertionIndex = getInitCallInsertionIndex(originalHooksModAST); + const initCallInsertionIndex = + getBeforeImportsInsertionIndex(originalHooksModAST); originalHooksModAST.body.splice( initCallInsertionIndex, @@ -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, @@ -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); -} diff --git a/src/remix/templates.ts b/src/remix/templates.ts index f52b7452..12e9e605 100644 --- a/src/remix/templates.ts +++ b/src/remix/templates.ts @@ -6,6 +6,6 @@ export const ERROR_BOUNDARY_TEMPLATE_V2 = `const ErrorBoundary = () => { `; export const HANDLE_ERROR_TEMPLATE_V2 = `function handleError(error, { request }) { - Sentry.captureRemixServerException(error, 'remix.server', request); + Sentry.captureRemixServerException(error, 'remix.server', request, true); } `; diff --git a/src/remix/utils.ts b/src/remix/utils.ts index 514a7f7f..d897b845 100644 --- a/src/remix/utils.ts +++ b/src/remix/utils.ts @@ -7,16 +7,23 @@ import clack from '@clack/prompts'; import chalk from 'chalk'; import { PackageDotJson, hasPackageInstalled } from '../utils/package-json'; -// Copied from sveltekit wizard +export const POSSIBLE_SERVER_INSTRUMENTATION_PATHS = [ + './instrumentation', + './instrumentation.server', +]; + export function hasSentryContent( fileName: string, fileContent: string, + expectedContent = '@sentry/remix', ): boolean { - const includesContent = fileContent.includes('@sentry/remix'); + const includesContent = fileContent.includes(expectedContent); if (includesContent) { clack.log.warn( - `File ${chalk.cyan(path.basename(fileName))} already contains Sentry code. + `File ${chalk.cyan( + path.basename(fileName), + )} already contains ${expectedContent}. Skipping adding Sentry functionality to ${chalk.cyan( path.basename(fileName), )}.`, @@ -26,14 +33,57 @@ Skipping adding Sentry functionality to ${chalk.cyan( return includesContent; } +export function serverHasInstrumentationImport( + serverFileName: string, + serverFileContent: string, +): boolean { + const includesServerInstrumentationImport = + POSSIBLE_SERVER_INSTRUMENTATION_PATHS.some((path) => + serverFileContent.includes(path), + ); + + if (includesServerInstrumentationImport) { + clack.log.warn( + `File ${chalk.cyan( + path.basename(serverFileName), + )} already contains instrumentation import. +Skipping adding instrumentation functionality to ${chalk.cyan( + path.basename(serverFileName), + )}.`, + ); + } + + return includesServerInstrumentationImport; +} + /** - * We want to insert the init call on top of the file but after all import statements + * We want to insert the init call on top of the file, before any other imports. */ -export function getInitCallInsertionIndex( +export function getBeforeImportsInsertionIndex( originalHooksModAST: Program, ): number { - for (let x = originalHooksModAST.body.length - 1; x >= 0; x--) { - if (originalHooksModAST.body[x].type === 'ImportDeclaration') { + for (let x = 0; x < originalHooksModAST.body.length - 1; x++) { + if ( + originalHooksModAST.body[x].type === 'ImportDeclaration' && + // @ts-expect-error - source is available in body + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access + originalHooksModAST.body[x].source.value === '@sentry/remix' + ) { + return x + 1; + } + } + + return 0; +} + +/** + * We want to insert the handleError function just after all imports + */ +export function getAfterImportsInsertionIndex( + originalEntryServerModAST: Program, +): number { + for (let x = originalEntryServerModAST.body.length - 1; x >= 0; x--) { + if (originalEntryServerModAST.body[x].type === 'ImportDeclaration') { return x + 1; } }