-
Notifications
You must be signed in to change notification settings - Fork 75
RUM-13468: Add totalRam, processorCount and isLowRamDevice to DeviceInfo
#3024
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’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
RUM-13468: Add totalRam, processorCount and isLowRamDevice to DeviceInfo
#3024
Conversation
a5339c4 to
899ef0f
Compare
totalRam, processorCount and isLowRamDevice to DeviceInfo
65945bf to
55a5484
Compare
|
🎯 Code Coverage * Fix with Cursor requires Datadog plugin ≥v2.17.0 🔗 Commit SHA: a4e01bd | Docs | Datadog PR Page | Was this helpful? Give us feedback! |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #3024 +/- ##
===========================================
+ Coverage 71.25% 71.26% +0.01%
===========================================
Files 866 866
Lines 31734 31807 +73
Branches 5360 5360
===========================================
+ Hits 22610 22665 +55
- Misses 7596 7614 +18
Partials 1528 1528
🚀 New features to boost your workflow:
|
55a5484 to
c356214
Compare
c356214 to
a4e01bd
Compare
| "processor_count": { | ||
| "type" : "number", | ||
| "description": "Number of device processors", | ||
| "readOnly": true | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably the naming is not the best one, because processor is a chip containing multiple cores. We probably want to send the number of physical cores, right?
| override val processorCount: Int by lazy(LazyThreadSafetyMode.PUBLICATION) { | ||
| Runtime.getRuntime().availableProcessors() | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure if we want to send the number of processors (chips) or cores, but this method returns the number of physical cores and not the number of chips.
| } | ||
| } | ||
|
|
||
| override val processorCount: Int by lazy(LazyThreadSafetyMode.PUBLICATION) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unless we have any performance concerns, we can use default mode
| override val processorCount: Int by lazy(LazyThreadSafetyMode.PUBLICATION) { | |
| override val processorCount: Int by lazy { |
| } | ||
|
|
||
| @Suppress("UnsafeThirdPartyFunctionCall") // Called within a try/catch block | ||
| override val totalRam: Int? by lazy(LazyThreadSafetyMode.PUBLICATION) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| override val totalRam: Int? by lazy(LazyThreadSafetyMode.PUBLICATION) { | |
| override val totalRam: Int? by lazy { |
| } | ||
|
|
||
| @Suppress("UnsafeThirdPartyFunctionCall") // Called within a try/catch block | ||
| override val isLowRamDevice: Boolean? by lazy(LazyThreadSafetyMode.PUBLICATION) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| override val isLowRamDevice: Boolean? by lazy(LazyThreadSafetyMode.PUBLICATION) { | |
| override val isLowRamDevice: Boolean? by lazy { |
| Runtime.getRuntime().availableProcessors() | ||
| } | ||
|
|
||
| @Suppress("UnsafeThirdPartyFunctionCall") // Called within a try/catch block |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor: it is better to move this annotation right to the call site rather than to keep it for the whole method. I guess here it will be getMemoryInfo and ActivityManager.MemoryInfo() constructor can be declared as safe in YAML config.
| val processorCount: Int = forge.anInt() | ||
| val totalRam: Int = forge.anInt() | ||
| val isLowRamDevice: Boolean = forge.aBool() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| val processorCount: Int = forge.anInt() | |
| val totalRam: Int = forge.anInt() | |
| val isLowRamDevice: Boolean = forge.aBool() | |
| val processorCount = forge.anInt() | |
| val totalRam = forge.aNullable { anInt() } | |
| val isLowRamDevice = forge.aNullable { aBool() } |
| totalRam = forge.anInt(), | ||
| isLowRamDevice = forge.aBool() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| totalRam = forge.anInt(), | |
| isLowRamDevice = forge.aBool() | |
| totalRam = forge.aNullable { anInt() }, | |
| isLowRamDevice = forge.aNullable { aBool() } |
| processorCount = datadogContext.deviceInfo.processorCount, | ||
| totalRam = datadogContext.deviceInfo.totalRam, | ||
| isLowRamDevice = datadogContext.deviceInfo.isLowRamDevice |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we need to add assertions for these new properties in tests for RumViewScope, RumResourceScope, etc.
totalRam, processorCount and isLowRamDevice to DeviceInfototalRam, processorCount and isLowRamDevice to DeviceInfo
What does this PR do?
Adds new attributes to the Device Info:
ActivityManager.isLowRamDevice)Additional Notes
Review checklist (to be filled by reviewers)