Add memory usage to contexts #2133

denrase · 2024-06-25T14:15:20Z

📜 Description

Add memory usage to contexts

💡 Motivation and Context

Closes #1826

💚 How did you test it?

Sample app

📝 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

github-actions · 2024-06-25T14:15:30Z

iOS Performance metrics 🚀

	Plain	With Sentry	Diff
Startup time	1240.94 ms	1263.18 ms	22.24 ms
Size	8.33 MiB	9.63 MiB	1.29 MiB

Baseline results on branch: main

Startup times

Revision	Plain	With Sentry	Diff
`e3ef570`	1220.08 ms	1239.66 ms	19.58 ms
`1b66358`	1260.81 ms	1282.08 ms	21.27 ms
`64af39c`	1243.98 ms	1257.69 ms	13.71 ms
`f275487`	1291.65 ms	1339.92 ms	48.26 ms
`2d3b03d`	1258.19 ms	1272.69 ms	14.50 ms
`bdd1a23`	1225.78 ms	1235.82 ms	10.04 ms
`408bdab`	1239.63 ms	1257.61 ms	17.98 ms
`0db91cc`	1267.63 ms	1279.69 ms	12.06 ms
`4ad2751`	1247.67 ms	1249.20 ms	1.53 ms
`11fb408`	1256.14 ms	1283.51 ms	27.37 ms

App size

Revision	Plain	With Sentry	Diff
`e3ef570`	8.32 MiB	9.38 MiB	1.06 MiB
`1b66358`	8.10 MiB	9.08 MiB	1004.35 KiB
`64af39c`	8.28 MiB	9.34 MiB	1.06 MiB
`f275487`	8.32 MiB	9.38 MiB	1.05 MiB
`2d3b03d`	8.10 MiB	9.07 MiB	1000.83 KiB
`bdd1a23`	8.29 MiB	9.39 MiB	1.10 MiB
`408bdab`	8.28 MiB	9.34 MiB	1.06 MiB
`0db91cc`	8.15 MiB	9.15 MiB	1018.56 KiB
`4ad2751`	8.29 MiB	9.39 MiB	1.10 MiB
`11fb408`	8.10 MiB	9.08 MiB	1004.36 KiB

Previous results on branch: feat/memory-usage

Startup times

Revision	Plain	With Sentry	Diff
`e8d8880`	1244.51 ms	1264.81 ms	20.30 ms
`ce54ed1`	1222.17 ms	1248.96 ms	26.79 ms
`1ab1f7e`	1244.94 ms	1263.58 ms	18.65 ms
`cf5df45`	1245.84 ms	1262.33 ms	16.50 ms
`fcbcade`	1234.31 ms	1253.57 ms	19.26 ms

App size

Revision	Plain	With Sentry	Diff
`e8d8880`	8.33 MiB	9.63 MiB	1.30 MiB
`ce54ed1`	8.33 MiB	9.62 MiB	1.29 MiB
`1ab1f7e`	8.33 MiB	9.62 MiB	1.29 MiB
`cf5df45`	8.33 MiB	9.63 MiB	1.29 MiB
`fcbcade`	8.33 MiB	9.63 MiB	1.30 MiB

github-actions · 2024-06-25T14:16:03Z

	Messages
📖	Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Generated by 🚫 dangerJS against c1d87f6

codecov · 2024-06-25T14:17:34Z

Codecov Report

Attention: Patch coverage is 67.85714% with 18 lines in your changes missing coverage. Please review.

Project coverage is 88.30%. Comparing base (f172c4d) to head (c1d87f6).

Files	Patch %	Lines
...c/event_processor/enricher/io_platform_memory.dart	61.70%	18 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2133      +/-   ##
==========================================
+ Coverage   86.46%   88.30%   +1.84%     
==========================================
  Files         166      225      +59     
  Lines        5850     7773    +1923     
==========================================
+ Hits         5058     6864    +1806     
- Misses        792      909     +117

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

buenaflor · 2024-06-25T15:08:32Z

We could use these fields if they make sense.

memory_size
Optional. Total system memory available in bytes.

free_memory
Optional. Free system memory in bytes.

usable_memory
Optional. Memory usable for the app in bytes.

see https://develop.sentry.dev/sdk/event-payloads/contexts/

buenaflor · 2024-06-25T18:14:59Z

app_memory might also be useful

app_memory
Optional. Amount of memory used by the application in bytes.

depending on the meaning of app_memory both current and max rss could fit here. we'd need more clarification for that

buenaflor · 2024-06-25T18:27:17Z

Okay it's supposed to be the memory at the time of the event so currentRss should fit here

buenaflor

left some comments,

I'm not sure though how we should categorize maxRss

CHANGELOG.md

buenaflor · 2024-06-28T11:24:25Z

dart/lib/src/event_processor/enricher/io_enricher_event_processor.dart

+          _bytesToHumanReadableFileSize(ProcessInfo.currentRss),
+      'maxResidentSetSize': _bytesToHumanReadableFileSize(ProcessInfo.maxRss),


are there any alternatives for web?

Found this, but there's not much documentation: https://api.dart.dev/stable/2.18.0/dart-html/MemoryInfo/usedJSHeapSize.html

this is based on this https://developer.mozilla.org/en-US/docs/Web/API/Performance/memory

it's deprecated and not all browsers support it, dunno if it's worth the effort to add this if we have to remove it again at some point

Yeah, i removed it after reading the docs and added some info.

buenaflor · 2024-06-28T11:25:32Z

dart/lib/src/event_processor/enricher/io_enricher_event_processor.dart

@@ -40,6 +40,11 @@ class IoEnricherEventProcessor implements EnricherEventProcessor {
    );

    contexts['dart_context'] = _getDartContext();
+    contexts['process_info'] = <String, dynamic>{
+      'currentResidentSetSize':


let's add currentRss to app_memory

How does currentRss compare to what the native side sets? Does it override what the native side sets?

Updated the code and we only set these values if there are no native SDKs

I don't think android sets it, I know iOS for sure does @denrase

Updated so we set appMemory to ProcessInfo.currentRss, it it's not already set, regardless of native integration. The other values are merged, so this is additional information.

On Linux/Windows we now also add free and total system memory, if possible.

denrase · 2024-07-01T10:01:28Z

@buenaflor Used the mapping below, as these seemed the most appropriate accodring to the documentation. WDYT?

Used

app_memory -> currentRss
Optional. Amount of memory used by the application in bytes.
usable_memory -> maxRss
Optional. Memory usable for the app in bytes.

Values below are not used, as we can't say anything about the system memory as far as I can see.

memory_size
Optional. Total system memory available in bytes.
free_memory
Optional. Free system memory in bytes.

denrase · 2024-07-01T10:05:24Z

There's a package to get more system info we could look into: https://pub.dev/packages/system_info2

denrase · 2024-07-01T10:07:47Z

Here's how it checks for physical memory info on different platforms.

https://github.com/onepub-dev/system_info/blob/8a9bf6b8eb7c86a09b3c3df4bf6d7fa5a6b50732/lib/src/platform/memory.dart#L73

buenaflor · 2024-07-01T10:28:10Z

usable_memory -> maxRss
Optional. Memory usable for the app in bytes.

hm not sure if this completely fits since maxRss captures the highest amount used thus far

buenaflor · 2024-07-01T10:34:16Z

There's a package to get more system info we could look into: https://pub.dev/packages/system_info2

can those commands be run in production?

dart/lib/src/event_processor/enricher/io_platform_memory.dart

github-actions · 2024-07-01T13:51:43Z

Android Performance metrics 🚀

	Plain	With Sentry	Diff
Startup time	390.06 ms	458.00 ms	67.94 ms
Size	6.35 MiB	7.35 MiB	1019.02 KiB

Baseline results on branch: main

Startup times

Revision	Plain	With Sentry	Diff
`d189e01`	328.67 ms	397.12 ms	68.45 ms
`7f75f32`	347.36 ms	419.58 ms	72.22 ms
`457a85b`	312.37 ms	376.67 ms	64.31 ms
`2331d89`	352.45 ms	417.34 ms	64.89 ms
`0118295`	365.71 ms	438.56 ms	72.85 ms
`eb1a7c1`	332.98 ms	381.55 ms	48.57 ms
`ef2f368`	350.06 ms	429.44 ms	79.38 ms
`e5b744f`	302.70 ms	342.17 ms	39.47 ms
`c73ab67`	353.82 ms	408.71 ms	54.90 ms
`4477d2e`	392.75 ms	472.69 ms	79.94 ms

App size

Revision	Plain	With Sentry	Diff
`d189e01`	6.16 MiB	7.14 MiB	1009.90 KiB
`7f75f32`	6.26 MiB	7.20 MiB	959.18 KiB
`457a85b`	6.06 MiB	7.09 MiB	1.03 MiB
`2331d89`	5.94 MiB	6.96 MiB	1.02 MiB
`0118295`	6.33 MiB	7.26 MiB	947.07 KiB
`eb1a7c1`	5.94 MiB	6.92 MiB	1005.76 KiB
`ef2f368`	5.94 MiB	6.89 MiB	975.81 KiB
`e5b744f`	6.06 MiB	7.09 MiB	1.03 MiB
`c73ab67`	6.15 MiB	7.13 MiB	999.97 KiB
`4477d2e`	6.33 MiB	7.26 MiB	950.38 KiB

Previous results on branch: feat/memory-usage

Startup times

Revision	Plain	With Sentry	Diff
`e8d8880`	427.13 ms	496.06 ms	68.93 ms
`cf5df45`	405.76 ms	472.82 ms	67.06 ms
`ce54ed1`	416.06 ms	507.80 ms	91.74 ms
`1ab1f7e`	387.86 ms	451.34 ms	63.48 ms
`fcbcade`	407.13 ms	482.32 ms	75.19 ms

App size

Revision	Plain	With Sentry	Diff
`e8d8880`	6.35 MiB	7.35 MiB	1.00 MiB
`cf5df45`	6.35 MiB	7.35 MiB	1018.79 KiB
`ce54ed1`	6.35 MiB	7.35 MiB	1019.03 KiB
`1ab1f7e`	6.35 MiB	7.35 MiB	1019.02 KiB
`fcbcade`	6.35 MiB	7.35 MiB	1.00 MiB

buenaflor

left some comments, regarding maxRss we can also name it ourselves, not sure if usable_memory is the best fit. wdyt?

dart/lib/src/event_processor/enricher/io_platform_memory.dart

CHANGELOG.md

denrase · 2024-07-08T13:54:00Z

can those commands be run in production?

Testes in debug in am Windows VM and tests pass so far.

buenaflor

one last fix pls

CHANGELOG.md

Add memory usage to contexts

cf5df45

add cl entry

eeb797f

denrase added 2 commits June 25, 2024 16:31

format

7c05a02

run dart fix

d3302e8

denrase marked this pull request as ready for review June 25, 2024 14:33

denrase requested review from krystofwoldrich, stefanosiano and buenaflor as code owners June 25, 2024 14:33

buenaflor reviewed Jun 28, 2024

View reviewed changes

denrase added 5 commits July 1, 2024 10:49

Set ProcessInfo.currentRss as appMemory

b210da5

read memoryInfo.usedJSHeapSize for html app_memory

37ea59c

remove memoty info enrichment on web

d10dbbe

update changelog

6088536

set usable memory

89657ee

read total & free memory on linux/windows

ba8a048

ueman reviewed Jul 1, 2024

View reviewed changes

dart/lib/src/event_processor/enricher/io_platform_memory.dart Outdated Show resolved Hide resolved

denrase added 2 commits July 1, 2024 15:52

only run on vm

e8d8880

use correct section for windows commands

fcbcade

buenaflor reviewed Jul 1, 2024

View reviewed changes

dart/lib/src/event_processor/enricher/io_platform_memory.dart Outdated Show resolved Hide resolved

CHANGELOG.md Outdated Show resolved Hide resolved

denrase added 6 commits July 2, 2024 09:40

update changelog

eae3175

log error

e2d07f0

use platform checker

1ab1f7e

always load appMemory

f880f63

format

ce54ed1

Merge branch 'main' into feat/memory-usage

3a2ce0a

denrase requested review from buenaflor and ueman July 8, 2024 13:54

buenaflor requested changes Jul 8, 2024

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

fix changelog

c1d87f6

denrase requested a review from buenaflor July 8, 2024 14:19

buenaflor approved these changes Jul 8, 2024

View reviewed changes

denrase merged commit 9f9dd52 into main Jul 9, 2024
130 checks passed

denrase deleted the feat/memory-usage branch July 9, 2024 08:34

		_bytesToHumanReadableFileSize(ProcessInfo.currentRss),
		'maxResidentSetSize': _bytesToHumanReadableFileSize(ProcessInfo.maxRss),

Add memory usage to contexts #2133

Add memory usage to contexts #2133

Conversation

denrase commented Jun 25, 2024

📜 Description

💡 Motivation and Context

💚 How did you test it?

📝 Checklist

github-actions bot commented Jun 25, 2024 • edited Loading

iOS Performance metrics 🚀

Baseline results on branch: main

Startup times

App size

Previous results on branch: feat/memory-usage

Startup times

App size

github-actions bot commented Jun 25, 2024 • edited Loading

codecov bot commented Jun 25, 2024 • edited Loading

Codecov Report

buenaflor commented Jun 25, 2024

buenaflor commented Jun 25, 2024

buenaflor commented Jun 25, 2024 • edited Loading

buenaflor left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

denrase Jul 1, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

denrase commented Jul 1, 2024 • edited Loading

denrase commented Jul 1, 2024

denrase commented Jul 1, 2024

buenaflor commented Jul 1, 2024

buenaflor commented Jul 1, 2024

github-actions bot commented Jul 1, 2024 • edited Loading

Android Performance metrics 🚀

Baseline results on branch: main

Startup times

App size

Previous results on branch: feat/memory-usage

Startup times

App size

buenaflor left a comment • edited Loading

Choose a reason for hiding this comment

denrase commented Jul 8, 2024

buenaflor left a comment

Choose a reason for hiding this comment

github-actions bot commented Jun 25, 2024 •

edited

Loading

github-actions bot commented Jun 25, 2024 •

edited

Loading

codecov bot commented Jun 25, 2024 •

edited

Loading

buenaflor commented Jun 25, 2024 •

edited

Loading

denrase Jul 1, 2024 •

edited

Loading

denrase commented Jul 1, 2024 •

edited

Loading

github-actions bot commented Jul 1, 2024 •

edited

Loading

buenaflor left a comment •

edited

Loading