Skip to content

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

Merged
denrase merged 3 commits intotemp/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 intotemp/8.14.1from
fix/remove-free-memory-win

Conversation

@denrase
Copy link
Copy Markdown
Collaborator

@denrase denrase commented Mar 17, 2025

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

codecov bot commented Mar 17, 2025

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
Copy link
Copy Markdown
Collaborator Author

denrase commented Mar 17, 2025

Comment regarding free memory removal: #2774 (review)

Copy link
Copy Markdown
Contributor

@buenaflor buenaflor left a comment

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
157 checks passed
@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 <denrase@gmail.com>
Co-authored-by: getsentry-bot <bot@sentry.io>
Co-authored-by: getsentry-bot <bot@getsentry.com>
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 <denrase@gmail.com>
Co-authored-by: getsentry-bot <bot@sentry.io>
Co-authored-by: getsentry-bot <bot@getsentry.com>
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