RUM-13468: Add totalRam, logicalCpuCount and isLowRam to DeviceInfo

[marco-saia-datadog]

What does this PR do?

Adds new attributes to the Device Info:

• totalRam: The total RAM in megabytes
• processorCount: Number of logical CPU cores available for scheduling on the device at runtime, as reported by the operating system.
• isLowRam: Whether the device is considered a low RAM device (from ActivityManager.isLowRamDevice)

Additional Notes

• The relevant schemas have been modified locally
• PR in rum-events-format: [FEAT] Added new attributes to RUM Device Info rum-events-format#323
• PR in rum-events-format: RUM-12199: Device Capabilities Attributes Refactoring rum-events-format#337 (refactoring)

Review checklist (to be filled by reviewers)

• Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
• Make sure you discussed the feature or bugfix with the maintaining team in an Issue
• Make sure each commit and the PR mention the Issue number (cf the CONTRIBUTING doc)

[codecov-commenter]

Codecov Report

❌ Patch coverage is 78.65169% with 19 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.24%. Comparing base (369ea6c) to head (8eb4230).
⚠️ Report is 347 commits behind head on develop.

Files with missing lines	Patch %	Lines
...core/internal/system/DefaultAndroidInfoProvider.kt	18.75%	13 Missing ⚠️
...tadog/android/core/internal/NoOpContextProvider.kt	0.00%	3 Missing ⚠️
...id/core/internal/system/NoOpAndroidInfoProvider.kt	0.00%	3 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #3024      +/-   ##
===========================================
+ Coverage    71.20%   71.24%   +0.04%     
===========================================
  Files          922      922              
  Lines        34099    34172      +73     
  Branches      5776     5776              
===========================================
+ Hits         24277    24344      +67     
- Misses        8190     8204      +14     
+ Partials      1632     1624       -8     

Files with missing lines	Coverage Δ
...tlin/com/datadog/android/api/context/DeviceInfo.kt	100.00% <100.00%> (ø)
...og/android/core/internal/DatadogContextProvider.kt	97.92% <100.00%> (+0.14%)	⬆️
...android/log/internal/domain/DatadogLogGenerator.kt	97.71% <100.00%> (+0.03%)	⬆️
...g/android/rum/internal/DatadogLateCrashReporter.kt	90.24% <100.00%> (+0.14%)	⬆️
...ndroid/rum/internal/domain/scope/RumActionScope.kt	97.13% <100.00%> (+0.04%)	⬆️
...roid/rum/internal/domain/scope/RumResourceScope.kt	90.65% <100.00%> (+0.16%)	⬆️
.../android/rum/internal/domain/scope/RumViewScope.kt	93.99% <100.00%> (-0.24%)	⬇️
...ernal/domain/scope/RumVitalAppLaunchEventHelper.kt	89.00% <100.00%> (+0.34%)	⬆️
...ndroid/telemetry/internal/TelemetryEventHandler.kt	89.58% <100.00%> (+2.85%)	⬆️
...trace/internal/domain/event/BaseSpanEventMapper.kt	70.45% <100.00%> (+7.04%)	⬆️
... and 3 more

... and 24 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[0xnm]

LGTM, just some minor cleanup is needed.

[0xnm]

I believe these two are nullable because the API we use to get this info may not return the data?

UPD: I see, it is due to the exception handling.

[0xnm]

neat! we indeed can use it now since min API is 23.

[0xnm]

Afaik this was to avoid losing precision in the floating point values between serialization/deserialization roundtrip, so it is not needed for the integer values. Same for other deserialized assertions below.

[marco-saia-datadog]

Actually, I added this because the recursive comparison seems to break with integer values, but I am happy to revisit it. Maybe I'll clue you in with the error stacktrace, you might have some useful insight to share. I'll follow up soon.

[marco-saia-datadog]

In RumEventDeserializerTest.kt using:

@ForgeConfiguration(value = Configurator::class, seed = 0xd1285cd0106L)

The forged event has:

logicalCpuCount = 1181136908
totalRam = -479744429
 
// Also, would it make sense to only forge positive values?
// I kept it like this to cover all possible values and make sure 
// we never get runtime errors as long as these values are Integers

And I get this error:

Can't find any field or property with name 'value'.
Error when introspecting properties was :
- No getter for property 'value' in java.lang.Integer 
Error when introspecting fields was :
- Unable to obtain the value of the field <'value'> from <-479744429>
org.assertj.core.util.introspection.IntrospectionError: 
Can't find any field or property with name 'value'.
Error when introspecting properties was :
- No getter for property 'value' in java.lang.Integer 
Error when introspecting fields was :
- Unable to obtain the value of the field <'value'> from <-479744429>
	at app//org.assertj.core.util.introspection.PropertyOrFieldSupport.getSimpleValue(PropertyOrFieldSupport.java:88)
	at app//org.assertj.core.api.recursive.comparison.RecursiveComparisonDifferenceCalculator.determineDifferences(RecursiveComparisonDifferenceCalculator.java:295)
	at app//org.assertj.core.api.recursive.comparison.RecursiveComparisonDifferenceCalculator.determineDifferences(RecursiveComparisonDifferenceCalculator.java:186)
	at app//org.assertj.core.api.RecursiveComparisonAssert.determineDifferencesWith(RecursiveComparisonAssert.java:1199)
	at app//org.assertj.core.api.RecursiveComparisonAssert.isEqualTo(RecursiveComparisonAssert.java:160)
	at app//com.datadog.android.rum.utils.assertj.DeserializedLongTaskEventAssert.isEqualTo(DeserializedLongTaskEventAssert.kt:28)
	at app//com.datadog.android.rum.internal.domain.event.RumEventDeserializerTest.M deserialize a serialized RUM LongTaskEvent W deserialize()(RumEventDeserializerTest.kt:156)
	at java.base@21.0.8/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(Unknown Source)

Caused by: org.assertj.core.util.introspection.IntrospectionError: Unable to obtain the value of the field <'value'> from <-479744429>
	at app//org.assertj.core.util.introspection.FieldSupport.readSimpleField(FieldSupport.java:248)
	at app//org.assertj.core.util.introspection.FieldSupport.fieldValue(FieldSupport.java:202)
	at app//org.assertj.core.util.introspection.PropertyOrFieldSupport.getSimpleValue(PropertyOrFieldSupport.java:70)
	... 87 more

Caused by: java.lang.reflect.InaccessibleObjectException: Unable to make field private final int java.lang.Integer.value accessible: module java.base does not "opens java.lang" to unnamed module @72a34537
	at java.base/java.lang.reflect.AccessibleObject.throwInaccessibleObjectException(Unknown Source)
	at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(Unknown Source)
	at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(Unknown Source)
	at java.base/java.lang.reflect.Field.checkCanSetAccessible(Unknown Source)
	at java.base/java.lang.reflect.Field.setAccessible(Unknown Source)
	at org.assertj.core.util.introspection.FieldUtils.getField(FieldUtils.java:67)
	at org.assertj.core.util.introspection.FieldUtils.readField(FieldUtils.java:141)
	at org.assertj.core.util.introspection.FieldSupport.readSimpleField(FieldSupport.java:208)

I'll debug this further and get back to you, but please let me know if this is a known issue or if you have any insight! :)

[marco-saia-datadog]

I think I have found a fix!

When serializing and deserializing the forged event with JsonParser, the Integer values are converted to Number values, specifically to LazilyParsedNumber.

Internally, assertj encounters the attribute (e.g totalRam), and calls:

Set<String> actualFieldsNames = Objects.getFieldsNames(dualValue.actual.getClass());

Which returns ["value"].

It then tries to access the value field on both the actual and expected event, causing the Runtime Error above, since Integer.value is private.

Here is the quick fix:

val lazilyParsedIntPredicate = BiPredicate { v1: LazilyParsedNumber, v2: Integer -> v1.toInt() == v2.toInt() }

assertThat(actual.device)
  .usingRecursiveComparison()
  .withEqualsForFields(lazilyParsedIntPredicate, "totalRam", "logicalCpuCount")
  .ignoringFields("batteryLevel", "brightnessLevel")
  .isEqualTo(expected.device)

I'll refine this and update the PR :)

[marco-saia-datadog]

I have pushed new changes, and added a common extension:

/**
 * Custom predicate for comparing Gson's LazilyParsedNumber with Integer fields.
 *
 * See [withGsonIntEqualsForFields].
 */
private val lazilyParsedIntPredicate = BiPredicate { v1: LazilyParsedNumber, v2: Int -> v1.toInt() == v2 }

/**
 * Adds support for comparing Gson's LazilyParsedNumber with Integer fields
 * during AssertJ recursive comparison.
 *
 * Gson deserializes Integer values as LazilyParsedNumber. During recursive
 * comparison, AssertJ may attempt reflective field comparison (e.g. Integer.value),
 * which fails because the field is private. This forces value-based comparison instead.
 */
fun<T : RecursiveComparisonAssert<T>> RecursiveComparisonAssert<T>.withGsonIntEqualsForFields(
    vararg fieldNames: String
): RecursiveComparisonAssert<T> {
    return this.withEqualsForFields(lazilyParsedIntPredicate, *fieldNames)
}

[0xnm]

lgtm, there is just one line leftover from debugging

[marco-saia-datadog]

Thanks @0xnm! I have removed the left-over debugging line, and rebased the PR. Let's see if all checks pass, then one final approval and I believe we can merge this 🙏

[marco-saia-datadog]

@0xnm The checks are failing with:

e: DatadogLogGenerator.kt:228:9 Cannot find a parameter with this name: logicalCpuCount
e: DatadogLogGenerator.kt:229:9 Cannot find a parameter with this name: totalRam
e: DatadogLogGenerator.kt:230:9 Cannot find a parameter with this name: isLowRam

Seems like a schema issue, but shouldn't it be updated automatically?

Update

I have checked the pipeline configuration, and it seems like we rely on caching using $CI_COMMIT_REF_SLUG as the key.

Is it possible that the CI is still picking up the old schema, since the branch hasn't changed after I have updated rum-events-format?

Also, shouldn't we be using $CI_COMMIT_SHA as the cache key?

[0xnm]

There is some strange caching issue.

Also, shouldn't we be using $CI_COMMIT_SHA as the cache key?

This will point to the current commit, while we want to point to the last known branch HEAD.