RUMM-2195: Add OS and device information properties to the RUM schema#62
Conversation
| "gaming_console", | ||
| "bot", |
There was a problem hiding this comment.
Do we have an accurate way to detect those natively? Also on the naming, I might use gaming_system, but not sure about this.
There was a problem hiding this comment.
I guess gaming_console may be deducted by having Xbox, Playstation, etc. in the device name or brand.
Not sure about Bot usage (maybe synthetics or case of web crawler?), but we have such type on the backend, so I decided to keep it just in case.
There was a problem hiding this comment.
Shouldn't we only add them once we need it? I think for now we can only use mobile, tablet and tv.
There was a problem hiding this comment.
I added them all just in case if Browser SDK start using these properties as well, because this is what I see for example for the rum-app:
But I'm ok with shrinking this list as well.
There was a problem hiding this comment.
I agree with @0xnm , these are the values available in the backend and since the browser (also using this rum format) can run on those, we need to be compatible
There was a problem hiding this comment.
I'm fine with being exhaustive from the start or add it as needed. FYI we don't plan to provide those properties via the Browser SDK.
There was a problem hiding this comment.
this is the definition of this enum backend side
There was a problem hiding this comment.
i think you should try to replicate the backend enum values as much as possible nevermind i think it's fine this way as it keeps schema syntax consistent
| "required": [ | ||
| "type" | ||
| ], |
There was a problem hiding this comment.
Unless we upgrade the format version, all new fields are optionals
There was a problem hiding this comment.
os and device parent fields are optional, so this is required rule only for the new optional parent fields, shouldn't break anything
| "required": [ | ||
| "name", | ||
| "version", | ||
| "version_major" | ||
| ], |
There was a problem hiding this comment.
Unless we upgrade the format version, all new fields are optionals
| "properties": { | ||
| "name": { | ||
| "type": "string", | ||
| "description": "Operating system name, ex. Android, iOS", |
There was a problem hiding this comment.
nit: I believe it's not the right abbreviation but I might be wrong 🙃
| "description": "Operating system name, ex. Android, iOS", | |
| "description": "Operating system name, e.g. Android, iOS", |
| "gaming_console", | ||
| "bot", |
There was a problem hiding this comment.
Shouldn't we only add them once we need it? I think for now we can only use mobile, tablet and tv.
4b62d50 to
e9f491a
Compare
| "gaming_console", | ||
| "bot", |
There was a problem hiding this comment.
I agree with @0xnm , these are the values available in the backend and since the browser (also using this rum format) can run on those, we need to be compatible
5 participants
This change adds OS and device information properties to the RUM schema to stop relying on the User Agent parsing for mobile devices, because UA parsing is not quite reliable as a result of the wide range of UA formats which can be found across different Mobile/Tablet/TV/etc. devices.
This change is mostly backfill of the backend logic which extracts the following from the parsed UA:
os.nameos.versionos.version_majordevice.typedevice.namedevice.modeldevice.brandAll the fields of
osproperty are required in assumption that it is always possible to get those values in any runtime.Only
typefield ofdeviceis required, because SDK running on thedesktop, for example, probably may not have any info aboutname,model,brand(or they can be not very representative).device.typeis taken from what we have already in the dashboard, with the only difference thatAppliancetype is replaced by theTV, because we have now a stronger focus on the TV platforms and any other device fromappliancetype can be marked asother.