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

Adding sentry to Remix increases latency by a lot #12285

Closed
3 tasks done
aon opened this issue May 29, 2024 · 10 comments · Fixed by #12110
Closed
3 tasks done

Adding sentry to Remix increases latency by a lot #12285

aon opened this issue May 29, 2024 · 10 comments · Fixed by #12110
Assignees
Labels
Package: remix Issues related to the Sentry Remix SDK Type: Bug

Comments

@aon
Copy link

aon commented May 29, 2024

Is there an existing issue for this?

How do you use Sentry?

Sentry Saas (sentry.io)

Which SDK are you using?

@sentry/remix

SDK Version

8.6.0

Framework Version

2.9.1

Link to Sentry event

No response

SDK Setup

No response

Steps to Reproduce

  1. Create a remix project with express as backend
  2. Add sentry to your project following instructions in https://docs.sentry.io/platforms/javascript/guides/remix/manual-setup/#installation
  3. Add a loader to a page
  4. Build and start application
  5. See how latency increases

Expected Result

As we're using a localhost environment, latency should be in the order of 1 ms.

image

Actual Result

Latency is in the order of 200/300ms.

image

@aon aon added the Type: Bug label May 29, 2024
@github-actions github-actions bot added the Package: remix Issues related to the Sentry Remix SDK label May 29, 2024
@aon
Copy link
Author

aon commented May 29, 2024

@AbhiPrasad
Copy link
Member

@aon if you disable performance monitoring by removing tracesSampleRate, does the latency still apply?

@aon
Copy link
Author

aon commented May 29, 2024

Removing tracesSampleRate removes the latency as well.

@AbhiPrasad
Copy link
Member

AbhiPrasad commented May 29, 2024

So it seems the issue is with performance monitoring. Hopefully this is addressed by #12110 then cc @onurtemizkan

@aon
Copy link
Author

aon commented May 29, 2024

Hopefully then. Thanks for the quick answer! Let me know how could I test this @onurtemizkan or how could I help out.

@onurtemizkan
Copy link
Collaborator

Thanks for reporting this @aon!

This is weird, I have tried to reproduce this locally but the Content Download stayed in microseconds in my test app. I think this will probably be fixed by #12110 (since we're removing most of the express server custom logic), but I'm still curious about what can be the culprit here.

Could you provide us with a minimal reproduction in the meantime? Also alternatively you can build from the branch https://github.com/getsentry/sentry-javascript/tree/onur/remix-otel-migration, and test to see if that update helps.

@aon
Copy link
Author

aon commented May 31, 2024

Hi @onurtemizkan! I created it here https://github.com/aon/minimum-sentry-remix. Please let me know your findings. Thanks!

@aon
Copy link
Author

aon commented May 31, 2024

BTW @onurtemizkan I've tried building the branch and I couldn't.

❯ yarn build
lerna notice cli v7.1.1

    ✔  @sentry/types:build:transpile (2s)

    ✖  @sentry/utils:build:transpile
       Usage Error: Couldn't find a script named "ts-node".

       $ yarn run [--inspect] [--inspect-brk] [-T,--top-level] [-B,--binaries-only] <scriptName> ...


    ✔  @sentry-internal/replay-worker:build:transpile (5s)

    ✖  @sentry/types:build:types
       Usage Error: Couldn't find a script named "downlevel-dts".

       $ yarn run [--inspect] [--inspect-brk] [-T,--top-level] [-B,--binaries-only] <scriptName> ...
       ERROR: "build:types:downlevel" exited with 1.



    ✖  @sentry-internal/replay-worker:build:types
       Usage Error: Couldn't find a script named "downlevel-dts".

       $ yarn run [--inspect] [--inspect-brk] [-T,--top-level] [-B,--binaries-only] <scriptName> ...
       ERROR: "build:types:downlevel" exited with 1.



 —————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————

 >  Lerna (powered by Nx)   Ran targets build:transpile, build:types, build:bundle for 31 projects (6s)

    ✔    2/5 succeeded [0 read from cache]

    ✖    3/5 targets failed, including the following:
         - @sentry/utils:build:transpile
         - @sentry/types:build:types
         - @sentry-internal/replay-worker:build:types

   Hint: Try "nx view-logs" to get structured, searchable errors logs in your browser.

@onurtemizkan
Copy link
Collaborator

Thanks for the reproduction @aon!

Yes, I can see this issue exists, and also can confirm that it's fixed on the PR #12110's branch!

@AbhiPrasad
Copy link
Member

We've released the changes with https://github.com/getsentry/sentry-javascript/releases/tag/8.10.0. To start using it, add autoInstrumentRemix: true to your server-side Sentry.init call!

https://docs.sentry.io/platforms/javascript/guides/remix/manual-setup/#server-side-configuration

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Package: remix Issues related to the Sentry Remix SDK Type: Bug
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants