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

Prevent OOM by disabling TransactionPerformanceCollector for now #2498

Merged
merged 4 commits into from
Jan 26, 2023

Conversation

adinauer
Copy link
Member

📜 Description

Extracted TransactionPerformanceCollector interface and renamed previous TransactionPerformanceCollector to DefaultTransactionPerformanceCollector. Use NoOpTransactionPerformanceCollector as default in SentryOptions

💡 Motivation and Context

Quick fix for #2496 disabling TransactionPerformanceCollector by replacing it with a NoOp variant.

💚 How did you test it?

  • run spring boot sample
  • wrk -t2 -c100 -d30s http://user:password@localhost:8080/person/9
  • visualvm heapdump analysis

📝 Checklist

  • I reviewed the submitted code
  • I added tests to verify the changes
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled
  • I updated the docs if needed
  • No breaking changes

🔮 Next steps

  • figure out why performanceDataMap isn't cleaned up and keeps growing, then re-enable DefaultTransactionPerformanceCollector

@adinauer adinauer changed the title Quickfix to prevent transaction performance collector OOM Prevent OOM by disabling TransactionPerformanceCollector for now Jan 26, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Jan 26, 2023

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 354.38 ms 385.70 ms 31.32 ms
Size 1.73 MiB 2.33 MiB 620.49 KiB

Previous results on branch: fix/quickfix-oom-transaction-performance-collector

Startup times

Revision Plain With Sentry Diff
5fcf409 306.80 ms 357.96 ms 51.16 ms
239f3a1 344.41 ms 386.34 ms 41.93 ms
39da621 339.21 ms 384.84 ms 45.63 ms

App size

Revision Plain With Sentry Diff
5fcf409 1.73 MiB 2.33 MiB 620.49 KiB
239f3a1 1.73 MiB 2.33 MiB 620.49 KiB
39da621 1.73 MiB 2.33 MiB 620.49 KiB

@codecov
Copy link

codecov bot commented Jan 26, 2023

Codecov Report

Base: 80.13% // Head: 80.12% // Decreases project coverage by -0.01% ⚠️

Coverage data is based on head (93a5699) compared to base (fb1c50c).
Patch coverage: 90.90% of modified lines in pull request are covered.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #2498      +/-   ##
============================================
- Coverage     80.13%   80.12%   -0.01%     
- Complexity     3917     3920       +3     
============================================
  Files           321      322       +1     
  Lines         14788    14793       +5     
  Branches       1951     1951              
============================================
+ Hits          11850    11853       +3     
- Misses         2170     2171       +1     
- Partials        768      769       +1     
Impacted Files Coverage Δ
...sentry/DefaultTransactionPerformanceCollector.java 90.00% <90.00%> (ø)
...io/sentry/NoOpTransactionPerformanceCollector.java 100.00% <100.00%> (ø)
sentry/src/main/java/io/sentry/SentryOptions.java 79.37% <100.00%> (+0.04%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@adinauer adinauer merged commit e199f18 into main Jan 26, 2023
@adinauer adinauer deleted the fix/quickfix-oom-transaction-performance-collector branch January 26, 2023 15:48
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants