fix: Still write user-agent when os-context conflicts#229
Conversation
jan-auer
left a comment
jan-auer
left a comment
There was a problem hiding this comment.
Excellent description, thanks! G2G once updated 👍
On the side, is the reason we're not setting client_os all the time that we do not want to break JS events?
| version: Annotated::from(version), | ||
| ..OsContext::default() | ||
| }))); | ||
| contexts.insert( |
There was a problem hiding this comment.
Please add a variant Context::ClientOs(Box<OsContext>) and return "client_os" from fn Context::default_key. That way, we only maintain this logic in one place and make it more reusable.
There was a problem hiding this comment.
Oh and I'm realizing that our enum derive only lowercases fields, where it should probably snake_case. Would that be possible to change with this PR?
There was a problem hiding this comment.
I don't think that is a good idea. the Context enum deals with context types (not default keys) but client_os is not its own context type yet.
I tried to implement this and it creates multiple possible representations for the same JSON: {"contexts": {"os": ...}} will be deserialized into an OsContext but can be serialized from a ClientOs variant as well.
Co-Authored-By: Jan Michael Auer <jan.auer@sentry.io>
Yes, particularly search. |
* master: feat: Implement data scrubbers in Relay (#227) fix: Still write user-agent when os-context conflicts (#229) ref: change default to sentry.io from ingest.sentry.io test: Merge integration tests in Travis config (#228) test: Run Kafka on Travis (#226) ref: Make uaparser optional (#224) fix: Move insta to dev-dependencies doc: Add some Docker info to README deploy: Add a pipeline step to create Sentry release build: Do not push Docker images to Dockerhub (#223) build: Use 8 core machines on Cloud Build
2 participants
Problem
The
oscontext is defined as "the OS of the device that sent the event" in the docs. Previously the server backfilled theoscontext based on the User-Agent only for JS events.We recently changed this (getsentry/sentry#10679) to do this for all events from all platforms. Therefore we can no longer have certainty which device the OS context is referring to: For SDKs that send OS contexts, it likely refers to the device where the event originated, as the docs define. For SDKs that do not send OS contexts, it can be whatever is in the user-agent.
As a concrete example:
However, the way the UI displays things makes it confusing. There, the browser context and OS context are pulled together and shown in one line:
This makes users expect something that is the opposite of what the interface docs suggest: The OS context (the one stored in the
oskey) is expected to refer to the client device.Solution
client_osoverosfor this bar (keep fallback toosfor JS).This needs to be deployed AFTER the UI knows to prefer
client_osoverosfor this bar.