Add additional app and device fields #881
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This stores the number of milliseconds that the application has been running, based on the time Bugsnag was started and the time the event happened This also removes the Expo specific implementation of this, so it also uses the version from core
This adds the 'manufacturer' and 'model' fields to Android and the total device memory to both iOS and Android
djskinner
approved these changes
Jun 8, 2020
bengourley
suggested changes
Jun 15, 2020
There was a problem hiding this comment.
In addition to the inline comments, as we discussed, the app duration functionality should be shared between node and the browser. I think since expo's app plugin collects more info, it's fine for app.duration to be populated there.
I think the new plugin should be called @bugsnag/plugin-app-duration.
imjoehaines
force-pushed
the
add-app-and-device-fields
branch
from
be47855
to
65bea8d
Compare
imjoehaines
force-pushed
the
add-app-and-device-fields
branch
from
65bea8d
to
64fd89e
Compare
bengourley
approved these changes
Jun 22, 2020
imjoehaines
force-pushed
the
add-app-and-device-fields
branch
2 times, most recently
from
b22fe62
to
bc2fb4a
Compare
bengourley
approved these changes
Jun 23, 2020
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds a number of additional fields to the
appanddevicenotify keys, specifically:orientation— the orientation of the browser using theScreen.orientationAPI. This is fairly widely supported, but not universally (e.g. Safari doesn't support this). There is also fallback which compares the browser width & height, to support more devicestotalMemory— the total memory available on the device, fetched with theexpo-devicemodule, which is a new dependencyosName— this usesos.platform, so reports the kernel name. This is still a good indicator and is much simpler and less error prone than trying to detect the OSosVersion— this usesos.release, which is also kernel informationtotalMemory— the total memory available to the node process (os.totalmem)freeMemory— the free memory available to the node process (os.freemem)Two new plugins have been added:
durationfield toevent.app, like the existingplugin-expo-appdurationfield toevent.app, like the existingplugin-expo-appThese plugins are currently identical so it may make sense to merge them into a
plugin-duration, which could then be used in Expo too. I'm not sure if we'd want to keep them separate though, so that there's room for them to diverge in future if we add more app fields to the browser or Node