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

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

Open
aon opened this issue May 9, 2024 · 2 comments
Open

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

aon opened this issue May 9, 2024 · 2 comments

Comments

@aon
Copy link

aon commented May 9, 2024

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
Copy link

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

@aon
Copy link
Author

aon commented May 13, 2024

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")))
  })
})

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants