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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Report App Memory Usage #2027

Merged
merged 4 commits into from
Aug 3, 2022
Merged

Conversation

kevinrenskers
Copy link
Contributor

馃摐 Description

Just like we're reporting free memory (see #1962), we are now reporting app memory usage.

馃挕 Motivation and Context

Closes #999

馃挌 How did you test it?

Unit tests, manual test.

馃摑 Checklist

  • I reviewed the submitted code
  • I added tests to verify the changes
  • I updated the docs if needed
  • Review from the native team if needed
  • No breaking changes

馃敭 Next steps

@kevinrenskers
Copy link
Contributor Author

kevinrenskers commented Aug 1, 2022

Screen Shot 2022-08-01 at 12 25 38

I'm not sure why "Free Memory" is shown as a human readable value, but "App Memory" is shown as a raw value. Is there a magic key I should use rather than app_memory? Looking at https://develop.sentry.dev/sdk/event-payloads/contexts/#device-context I don't see anything.

Should we translate this client-side? That seems a bit weird. So I guess we need to create a ticket for the backend/ui team to format this value?

* master:
  feat: Include app permissions with event (#1984)

# Conflicts:
#	Sources/Sentry/SentryClient.m
#	Tests/SentryTests/SentryClientTests.swift
@brustolin brustolin changed the title Report App Memory Usage feat: Report App Memory Usage Aug 3, 2022
@github-actions
Copy link

github-actions bot commented Aug 3, 2022

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

Generated by 馃毇 dangerJS against f42496d

@brustolin
Copy link
Contributor

Instead of device context, we should store this information in the app context.

@kevinrenskers
Copy link
Contributor Author

Done

@kevinrenskers kevinrenskers merged commit 4fb00e9 into master Aug 3, 2022
@kevinrenskers kevinrenskers deleted the feat/999-app-memory-usage branch August 3, 2022 21:00
@bruno-garcia
Copy link
Member

So I guess we need to create a ticket for the backend/ui team to format this value?

Was there a follow up on this?

This was done here: getsentry/sentry#20909

@bruno-garcia
Copy link
Member

Could be worth writing on the develop docs how to format data so that it's presented like that. Or where/how to change Sentry to do so (e.g: link to this PR?)

@kevinrenskers
Copy link
Contributor Author

Dhiogo said "we are solving the memory format server side".

kevinrenskers added a commit that referenced this pull request Aug 4, 2022
* master:
  feat: add culture context to event (#2036)
  feat: Report App Memory Usage (#2027)
  ci: run profile generator 4x per day (#2038)
  build(deps): bump actions/stale from 5.1.0 to 5.1.1 (#2030)

# Conflicts:
#	Sentry.xcodeproj/project.pbxproj
kevinrenskers added a commit that referenced this pull request Aug 4, 2022
* master:
  feat: add culture context to event (#2036)
  feat: Report App Memory Usage (#2027)
  ci: run profile generator 4x per day (#2038)
  build(deps): bump actions/stale from 5.1.0 to 5.1.1 (#2030)

# Conflicts:
#	Sources/Sentry/SentryClient.m
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.

Report App Memory Usage
4 participants