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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RUMM-2196: Add OS and device information to RUM events #945

Conversation

0xnm
Copy link
Member

@0xnm 0xnm commented Jun 8, 2022

What does this PR do?

This change and OS information (name, version, major version), and device information (type, model, name, brand) to the RUM events. Essentially it is a continuation of the work started in DataDog/rum-events-format#62 with a goal to have a better control on the device/OS information compared to parsing User Agent header approach (which requires continuous maintenance from backend team and has difficulties in the device categorization).

WIth that change SDK will send the following:

  • OS Name - will always be Android.

  • OS Version - full OS version, like 8.1.0, will always be the value of Build.VERSION.RELEASE property.

  • OS Major Version - only the major part of the semver format, like 8 for the OS version value above (or value of the version itself if non-semver format is used).

  • Device model - technical name of the device, like SG-M8888, will always be the value of Build.MODEL property.

  • Device brand - the value of Build.BRAND property capitalized.

  • Device name - normally a full name, which is a concatenation of model and brand (done like that to be backward compatible as most as possible with existing values of device.name facet).

  • Device type - can be mobile, tablet, tv or other. The borderline between mobile and tablet is quite blurry, especially considering that some tables come with SIM-cards (hence telephony), so the major property to distinguish the two is the smallest screen width. Most of Stackoverflow answers suggest to use value of 600 (large), but 800 (xlarge) is used instead, because in the last years screen size of the phones got bigger on average (but this means that also Nexus 7 will fall under Other category, because it has large display, not xlarge and no cellular connectivity). TV category detection relies on the some features, but it can be that both TV sticks and real TVs fall into this category (not only real TVs).

It may be that it will be some difference in the device.type values or device.name compared with existing ones, but I expect the difference to be quite small. Anyway, we will also see what are the values sent with a next release and tune the classification logic.

P.S. PR looks big, but in reality the main content is just in the https://github.com/DataDog/dd-sdk-android/pull/945/files#diff-1f56ceca14027b5fe4de05bfa4b764f1d02c15791fc0653eec8097056e70d27d.

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)

@xgouchet xgouchet added the size-huge This PR is huge sized label Jun 8, 2022
@0xnm 0xnm marked this pull request as ready for review June 8, 2022 15:22
@0xnm 0xnm requested a review from a team as a code owner June 8, 2022 15:22
@codecov-commenter
Copy link

codecov-commenter commented Jun 15, 2022

Codecov Report

Merging #945 (9f681b7) into develop (e5ddff4) will increase coverage by 0.34%.
The diff coverage is 96.23%.

@@             Coverage Diff             @@
##           develop     #945      +/-   ##
===========================================
+ Coverage    82.86%   83.20%   +0.34%     
===========================================
  Files          265      267       +2     
  Lines         9035     9214     +179     
  Branches      1458     1484      +26     
===========================================
+ Hits          7486     7666     +180     
+ Misses        1145     1142       -3     
- Partials       404      406       +2     
Impacted Files Coverage Δ
...n/com/datadog/android/core/internal/CoreFeature.kt 93.04% <80.00%> (-0.32%) ⬇️
...core/internal/system/DefaultAndroidInfoProvider.kt 84.44% <84.44%> (ø)
.../android/core/internal/net/DataOkHttpUploaderV2.kt 98.77% <100.00%> (ø)
...datadog/android/core/internal/system/DeviceType.kt 100.00% <100.00%> (ø)
...id/core/internal/system/NoOpAndroidInfoProvider.kt 100.00% <100.00%> (ø)
...adog/android/error/internal/CrashReportsFeature.kt 100.00% <100.00%> (ø)
...in/com/datadog/android/log/internal/LogsFeature.kt 100.00% <100.00%> (ø)
.../main/kotlin/com/datadog/android/rum/RumMonitor.kt 75.00% <100.00%> (+0.53%) ⬆️
...lin/com/datadog/android/rum/internal/RumFeature.kt 92.93% <100.00%> (ø)
...ndroid/rum/internal/domain/scope/RumActionScope.kt 98.43% <100.00%> (+0.15%) ⬆️
... and 13 more

@0xnm 0xnm merged commit 3b1c057 into develop Jun 15, 2022
@0xnm 0xnm deleted the nogorodnikov/rumm-2196/add-os-and-device-information-to-rum-events branch June 15, 2022 14:07
@xgouchet xgouchet added this to the 1.14.0 milestone Dec 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size-huge This PR is huge sized
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants