Skip to content

Fix: Improve platform memory collection on windows/linux#2798

Merged
denrase merged 3 commits into
temp/8.14.1from
fix/remove-free-memory-win
Mar 18, 2025
Merged

Fix: Improve platform memory collection on windows/linux#2798
denrase merged 3 commits into
temp/8.14.1from
fix/remove-free-memory-win

Conversation

@denrase

@denrase denrase commented Mar 17, 2025

Copy link
Copy Markdown
Collaborator

📜 Description

A user reported an issue where his http calls are way slower if wrapped in sentry transactions. Turns out this is because we call external binaries with exec for every transaction to get physical and free memory size. Turns out this has a large overhead. Also the binary used on windows was deprecated and may therefore not be present. So we have the large overhead without the benefit.

This PR introduces the following to mitigate/fix all of the above:

  • Disable platform memory collection per default per options.
  • If enabled:
    • Cache physical memory indefinitely
    • Cache free memory for one minute
    • On windows, fallback to powershell if wmci is not available.
  • Avoid copying of contexts.

Update: free memory has been removed currently on Desktop

💡 Motivation and Context

Relates to #2760
Relates to onepub-dev/system_info#12

💚 How did you test it?

Ran locally in a win 11 vm. Unit tests.

📝 Checklist

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

🔮 Next steps

@denrase
denrase changed the base branch from main to temp/8.14.1 March 17, 2025 13:20
@codecov

codecov Bot commented Mar 17, 2025

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 66.10169% with 20 lines in your changes missing coverage. Please review.

Project coverage is 88.91%. Comparing base (eeded53) to head (9e07db4).
Report is 1 commits behind head on temp/8.14.1.

Files with missing lines Patch % Lines
...c/event_processor/enricher/io_platform_memory.dart 35.48% 20 Missing ⚠️
Additional details and impacted files
@@               Coverage Diff               @@
##           temp/8.14.1    #2798      +/-   ##
===============================================
- Coverage        88.97%   88.91%   -0.06%     
===============================================
  Files              263      263              
  Lines             8931     8951      +20     
===============================================
+ Hits              7946     7959      +13     
- Misses             985      992       +7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@denrase

denrase commented Mar 17, 2025

Copy link
Copy Markdown
Collaborator Author

Comment regarding free memory removal: #2774 (review)

@buenaflor buenaflor left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good 👍

@denrase
denrase merged commit e24b9dd into temp/8.14.1 Mar 18, 2025
@denrase
denrase deleted the fix/remove-free-memory-win branch March 18, 2025 10:14
@getsentry getsentry deleted a comment from github-actions Bot Mar 18, 2025
buenaflor added a commit that referenced this pull request Mar 24, 2025
* Fix: Improve platform memory collection on windows/linux (#2798)

* deps: bump Android from `7.22.1` to `7.22.4` (#2810)

* update android deps

* Update CHANGELOG.md

* Fix adding runtime to contexts (#2813)

* Fix CHANGELOG formatting

* release: 8.14.1

* Update CHANGELOG

* Use options.platform instead of options.platformChecker

---------

Co-authored-by: Denis Andrašec <[email protected]>
Co-authored-by: getsentry-bot <[email protected]>
Co-authored-by: getsentry-bot <[email protected]>
buenaflor added a commit that referenced this pull request Apr 24, 2025
* Fix: Improve platform memory collection on windows/linux (#2798)

* deps: bump Android from `7.22.1` to `7.22.4` (#2810)

* update android deps

* Update CHANGELOG.md

* Fix adding runtime to contexts (#2813)

* Fix CHANGELOG formatting

* release: 8.14.1

* fix: `options.diagnosticLevel` not affecting logs (#2856)

* v9: Set log level to `warning` by default  (#2836)

* update

* update test

* update init native sdk test

* Update CHANGELOG

* Add additional test

* Update CHANGELOG

* Update CHANGELOG

* Update

* Update

* Fix test

* Fix test

* Fix analyze

* Remove prod scheme

* Update mocks

* Update mocks

* Improve performance of frames tracking (#2854)

* Improve performance

* Update tests

* Improve performance

* Formatting

* Separate function

* Remove separate function

* Update

* Improve frames tracking performance

* update naming

* Update

* update

* update

* Edge case

* Update mocks

* formatting

* Improvements

* Analyze

* Update mocks

* Update mocks

* Clean up `getSpan()` log (#2865)

* Remove unnecessary log and document the behaviour

* Typo

* Typo

* Add CHANGELOG entry

* release: 8.14.2

* Update

* Update

---------

Co-authored-by: Denis Andrašec <[email protected]>
Co-authored-by: getsentry-bot <[email protected]>
Co-authored-by: getsentry-bot <[email protected]>
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