remix-run / remix

Build Better Websites. Create modern, resilient user experiences with web fundamentals.
https://remix.run
MIT License
28.29k stars 2.37k forks source link

Integration with sentry adds a lot of latency, unless `defer` is used #9396

Open aon opened 1 month ago

aon commented 1 month ago

Reproduction

  1. Create a remix project with vite and express
  2. Install sentry
  3. Create a route with a loader and return some static data
  4. Open your browser and load the route

The wait time until the route loads is in my case in the order of 250ms, which is a lot considering it's a local server and the router is just returning static data.

image

Now, if I change to return a deferred response, the request still takes the same amount of time but at least I get the page back instantly.

As a reference, this is when I comment out sentry init in entry.server.tsx:

image

I shouldn't have to use defer to mimic the same behaviour as before. I'm not sure if this error is on the sentry side or in remix, but I believe both have to be somehow related.

System Info

System:
    OS: macOS 14.4.1
    CPU: (12) arm64 Apple M2 Max
    Memory: 1.32 GB / 64.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 20.12.2 - ~/.nvm/versions/node/v20.12.2/bin/node
    npm: 10.5.0 - ~/.nvm/versions/node/v20.12.2/bin/npm
    pnpm: 8.15.4 - /opt/homebrew/bin/pnpm
  Browsers:
    Chrome: 124.0.6367.119
    Safari: 17.4.1
  npmPackages:
    @remix-run/dev: ^2.9.1 => 2.9.1 
    @remix-run/express: ^2.9.1 => 2.9.1 
    @remix-run/node: ^2.9.1 => 2.9.1 
    @remix-run/react: ^2.9.1 => 2.9.1 
    @remix-run/serve: ^2.9.1 => 2.9.1 
    vite: ^5.2.11 => 5.2.11

Used Package Manager

pnpm

Expected Behavior

Pages loaders should when returning static data should be very fast.

Actual Behavior

When sentry is added, page responses goes down significantly.

quantuminformation commented 1 month ago

how did you integrate with remix? im using the grunge stack, do we just use image

aon commented 1 month ago

This is my entry.client.tsx config:

export function initSentry() {
  init({
    enabled: window.ENV.NODE_ENV === "production",
    dsn: window.ENV.SENTRY_DSN,
    environment: window.ENV.NODE_ENV,
    beforeSend(event) {
      if (event.request?.url) {
        const url = new URL(event.request.url)
        if (
          url.protocol === "chrome-extension:" ||
          url.protocol === "moz-extension:"
        ) {
          // This error is from a browser extension, ignore it
          return null
        }
      }
      return event
    },
    integrations: [
      browserTracingIntegration({
        useEffect,
        useLocation,
        useMatches,
      }),
      // Replay is only available in the client
      replayIntegration(),
      browserProfilingIntegration(),
    ],

    tracesSampleRate: 1,
    replaysSessionSampleRate: 0.1,
    replaysOnErrorSampleRate: 1.0,
    profilesSampleRate: 1,
  })
}

This is my entry.server.tsx config:

export function initSentry() {
  init({
    enabled: env.NODE_ENV === "production",
    dsn: env.SENTRY_DSN,
    environment: env.NODE_ENV,
    tracesSampleRate: 1,
    profilesSampleRate: 1,
    integrations: [
      new Integrations.Http({ tracing: true }),
      nodeProfilingIntegration(),
    ],
    tracesSampler(samplingContext) {
      // ignore healthcheck transactions by other services (consul, etc.)
      if (samplingContext.request?.url?.includes("/resources/healthcheck")) {
        return 0
      }
      return 1
    },
    profilesSampler(samplingContext) {
      // ignore healthcheck transactions by other services (consul, etc.)
      if (samplingContext.request?.url?.includes("/resources/healthcheck")) {
        return 0
      }
      return 1
    },
    beforeSendTransaction(event) {
      // ignore all healthcheck related transactions
      //  note that name of header here is case-sensitive
      if (event.request?.headers?.["x-healthcheck"] === "true") {
        return null
      }

      return event
    },
  })
}

And this is my express config:


installGlobals()

const createRequestHandler =
  env.NODE_ENV === "production"
    ? wrapExpressCreateRequestHandler(_createRequestHandler)
    : _createRequestHandler

const viteDevServer =
  env.NODE_ENV === "production"
    ? undefined
    : await import("vite").then((vite) =>
        vite.createServer({
          server: { middlewareMode: true },
        }),
      )

const app = express()

// Fly.io requires the app to trust the proxy
app.set("trust proxy", true)

app.use(requireHttps)
app.get("*", removeTrailingSlash)
app.use(compression())

// http://expressjs.com/en/advanced/best-practice-security.html#at-a-minimum-disable-x-powered-by-header
app.disable("x-powered-by")

app.use(Sentry.Handlers.requestHandler())
app.use(Sentry.Handlers.tracingHandler())

if (viteDevServer) {
  app.use(viteDevServer.middlewares)
} else {
  // Remix fingerprints its assets so we can cache forever.
  app.use(
    "/assets",
    express.static("build/client/assets", { immutable: true, maxAge: "1y" }),
  )

  // Everything else (like favicon.ico) is cached for an hour. You may want to be
  // more aggressive with this caching.
  app.use(express.static("build/client", { maxAge: "1h" }))
}

app.get(["/img/*", "/favicons/*"], (_req, res) => {
  // if we made it past the express.static for these, then we're missing something.
  // So we'll just send a 404 and won't bother calling other middleware.
  return res.status(404).send("Not found")
})

app.use(logger)
app.use(helmet())
app.use(rateLimit)
app.use(auth as RequestHandler)

if (!env.ALLOW_INDEXING) {
  app.use((_, res, next) => {
    res.set("X-Robots-Tag", "noindex, nofollow")
    next()
  })
}

async function getBuild() {
  // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
  const build = viteDevServer
    ? viteDevServer.ssrLoadModule("virtual:remix/server-build")
    : // eslint-disable-next-line @typescript-eslint/ban-ts-comment
      // @ts-ignore-error  this should exist before running the server
      // but it may not exist just yet.
      // eslint-disable-next-line import/no-unresolved
      await import("../build/server/index.js")
  return build as unknown as ServerBuild
}

app.all(
  "*",
  createRequestHandler({
    getLoadContext: (_req: Request, res: Response) => ({
      // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
      cspNonce: res.locals.cspNonce,
      serverBuild: getBuild(),
    }),
    mode: env.NODE_ENV,
    build: getBuild,
  }) as RequestHandler,
)

const desiredPort = env.SERVER_PORT
const portToUse = await getPort({
  port: portNumbers(desiredPort, desiredPort + 100),
})
const portAvailable = desiredPort === portToUse
if (!portAvailable && env.NODE_ENV !== "development") {
  console.log(`⚠️ Port ${desiredPort} is not available.`)
  process.exit(1)
}

const server = app.listen(portToUse, () => {
  if (!portAvailable) {
    console.warn(
      chalk.yellow(
        `⚠️  Port ${desiredPort} is not available, using ${portToUse} instead.`,
      ),
    )
  }
  console.log(`🚀  We have liftoff!`)
  const localUrl = `http://localhost:${portToUse}`
  let lanUrl: string | null = null
  const localIp = ipAddress() ?? "Unknown"
  // Check if the address is a private ip
  // https://en.wikipedia.org/wiki/Private_network#Private_IPv4_address_spaces
  // https://github.com/facebook/create-react-app/blob/d960b9e38c062584ff6cfb1a70e1512509a966e7/packages/react-dev-utils/WebpackDevServerUtils.js#LL48C9-L54C10
  if (/^10[.]|^172[.](1[6-9]|2[0-9]|3[0-1])[.]|^192[.]168[.]/.test(localIp)) {
    lanUrl = `http://${localIp}:${portToUse}`
  }

  console.log(
    `
${chalk.bold("Local:")}            ${chalk.cyan(localUrl)}
${lanUrl ? `${chalk.bold("On Your Network:")}  ${chalk.cyan(lanUrl)}` : ""}
${chalk.bold("Press Ctrl+C to stop")}
        `.trim(),
  )

  initCronjobs()
})

closeWithGrace(async () => {
  await new Promise((resolve, reject) => {
    server.close((e) => (e ? reject(e) : resolve("ok")))
  })
})