Skip to content

Sentry upgrade from 7.118.0 to 8.26.0 leak memory #13412

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

Closed
3 tasks done
flav-code opened this issue Aug 17, 2024 · 52 comments
Closed
3 tasks done

Sentry upgrade from 7.118.0 to 8.26.0 leak memory #13412

flav-code opened this issue Aug 17, 2024 · 52 comments
Assignees
Labels
Package: node Issues related to the Sentry Node SDK

Comments

@flav-code
Copy link

flav-code commented Aug 17, 2024

Is there an existing issue for this?

How do you use Sentry?

Self-hosted/on-premise

Which SDK are you using?

@sentry/node

SDK Version

8.26.0

Framework Version

No response

Link to Sentry event

No response

Reproduction Example/SDK Setup

my sentry init in 7.118.0

    init({
        dsn: process.env.SENTRY_DSN,
        environment: process.env.NODE_ENV || "none",
        release: require("../../package.json").version,
        serverName: `${cluster.bot}-c=${cluster.id}-first=${cluster.first}-last=${cluster.last}` || "none",
        integrations: [
            new Integrations.Postgres({ module: require("pg") }),
            new Integrations.Modules(),
            new Integrations.FunctionToString(),
            new Integrations.LinkedErrors(),
            new Integrations.Console(),
            new Integrations.Http({ breadcrumbs: true, tracing: true }),
            rewriteFramesIntegration({ root: path.join(__dirname, "..") }),
        ],
        // Performance Monitoring
        tracesSampleRate: 1.0, //  Capture 100% of the transactions
    });

my sentry init in 8.6.0

    Sentry.init({
        dsn: process.env.SENTRY_DSN,
        environment: process.env.NODE_ENV || "none",
        release: require("../../package.json").version,
        serverName: `${cluster.bot}-c=${cluster.id}-first=${cluster.first}-last=${cluster.last}` || "none",
        integrations: [
            Sentry.modulesIntegration(),
            Sentry.functionToStringIntegration(),
            Sentry.linkedErrorsIntegration(),
            Sentry.consoleIntegration(),
            Sentry.httpIntegration({ breadcrumbs: true }),
            Sentry.rewriteFramesIntegration({ root: path.join(__dirname, "..") }),
            Sentry.onUnhandledRejectionIntegration(),
            Sentry.onUncaughtExceptionIntegration(),
            Sentry.redisIntegration(),
            Sentry.postgresIntegration(),
        ],
        // To avoid sending too much data to Sentry, we can reduce the sample rate of traces and profiles
        tracesSampleRate: 1.0,
        profilesSampleRate: 1.0,
    });

Steps to Reproduce

I'll try removing some of the integrations to see what's causing the problem.

Expected Result

A normal memory usage

Image

Actual Result

anormal memory usage

Image

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Aug 17, 2024
@github-actions github-actions bot added the Package: node Issues related to the Sentry Node SDK label Aug 17, 2024
@Lms24
Copy link
Member

Lms24 commented Aug 19, 2024

Hey @flav-code thanks for writing in!

We'll look into your issue next week as this week is Hackweek at Sentry (see #13421).

@lforst
Copy link
Member

lforst commented Aug 26, 2024

Hi, @flav-code would you be able to provide a memory snapshot with the Node/v8 profiler so that we can look at what is holding the references causing the leak? Feel free to also shoot us an email or twitter dm if you don't want to publicly share it. Thanks!

@flav-code
Copy link
Author

I can't give you an snapshot because I have a lot of private information.

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Aug 26, 2024
@flav-code
Copy link
Author

I made a snapshot, I'll examine it and show you

@flav-code
Copy link
Author

Image

@flav-code
Copy link
Author

Set, Span and NonRecordingSpan are from Sentry

@lforst
Copy link
Member

lforst commented Aug 26, 2024

Yeah that looks like Sentry. Would you mind digging around a bit ant examine what holds the references to the spans?

@flav-code
Copy link
Author

Image

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Aug 26, 2024
@flav-code
Copy link
Author

Image

@amakhrov
Copy link

Set, Span and NonRecordingSpan are from Sentry

This indicates that Sentry objects are retained in memory. It doesn't mean they are causing this retention though!
E.g. the expanded Span object has the distance (from root) of 15. The sentryRootSpan - distance of 12. It means something else is holding the reference to it, something that is closer to the root.
You might want to explore the bottom part of the memory profiler - the Retainers section

@flav-code
Copy link
Author

Image

@amakhrov
Copy link

timerListMap[180000] - my understanding is that the app has started a timer for 180s (3min).
Any chance it's your app code rather than Sentry?

@flav-code
Copy link
Author

I don't think so. Before switching to sentry v8, someone warned me about the memory leak.

@lforst
Copy link
Member

lforst commented Aug 27, 2024

I have a few questions to narrow this down further. I am not ruling out that our SDK is causing the leak:

  • What backend framework are you using - if at all?
  • Are you manually creating spans somewhere?
  • Can you share any more code that is relevant to Sentry?
  • What Node.js version are you using? We have found that there are memory leaks in Node.js with regards to timers?
  • Can you double-check whether you have any setTimeouts or setIntervals?
  • One way we have found things may "leak" is if you create a span and don't end it. That way children get attached and they will be held indefinitely.

@lforst
Copy link
Member

lforst commented Aug 28, 2024

That setup looks good 👌 Can you share more of the debug logs? It would be good to see logs up until and including when you do things like send requests and database queries and similar. Thanks!

@flav-code
Copy link
Author

there was a query to my db but it doesn't seem to appear

Image

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Aug 28, 2024
@lforst
Copy link
Member

lforst commented Aug 28, 2024

Would you mind sharing the start of your application up to a certain point in text format? Thanks!

@flav-code
Copy link
Author

flav-code commented Aug 28, 2024

Do you have discord ?
It would be easier to talk and send logs

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Aug 28, 2024
@lforst
Copy link
Member

lforst commented Aug 28, 2024

Yes! Feel free to join https://discord.com/invite/sentry and ping @lforst

@lforst
Copy link
Member

lforst commented Sep 10, 2024

After some back and fourth we have discovered that the memory leak in this issue happens due to the httpIntegration creating a NonRecordingSpan for a very long-lived websocket request. The span is never ended because the request basically never ends for the entire lifetime of the process and stuff (ie. child spans) keep getting attached to that span.

The workaround for now is to do the following:

Sentry.init({
  integrations: [
    Sentry.httpIntegration({
      ignoreOutgoingRequests(url, request) {
        return true;
      },
    }),
  ],
})

Thanks for the collaboration @flav-code!!


Action items (varying degrees of possible):

  • Stop attaching stuff to spans
  • Create an upper bound for stuff being attached to spans

@flav-code
Copy link
Author

No problem 👍

@mydea
Copy link
Member

mydea commented Mar 18, 2025

Hey, I am closing this because I believe we found the issue and provided a workaround for the time being. Please re-open if you still encounter the problem - thank you!

@mydea mydea closed this as completed Mar 18, 2025
@flav-code
Copy link
Author

yes, it's not as bad as before

@lforst
Copy link
Member

lforst commented Apr 16, 2025

Note that this is one of the cases that will be solved once we switch to span streaming. So in like 5 years.

@flav-code
Copy link
Author

5 years ???

@lforst
Copy link
Member

lforst commented Apr 19, 2025

I should have added a sarcasm indicator 😬 It's coming but it's taking longer than we (I) would like. Hopefully sooner than 5 years.

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

No branches or pull requests

7 participants