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

Initial pass on context data #3521

Merged
merged 54 commits into from
Jun 27, 2016
Merged

Initial pass on context data #3521

merged 54 commits into from
Jun 27, 2016

Conversation

mitsuhiko
Copy link
Member

@mitsuhiko mitsuhiko commented Jun 17, 2016

Refs #3510

  • Adds 'contexts' attribute to payloads[1]
  • Adds Browser, Device, Runtime, and OS context schemas
  • Autofill Browser/OS context from javascript's useragents
  • Disable sentry_useragents for javascript/cocoa
  • Show summary of contexts[2] for javascript/cocoa
  • Disable gravatars for user contexts
  • Push user into context chunks, and lower priority below event entries[3]

[1] context schemas

"contexts": {
    "device": {
        "name": "iPad",
        "model": "iPad5,1"
    },
    "os": {
        "name": "iOS",
        "version": "8.1"
    }
}

[2] context summary

screenshot 2016-06-27 10 36 00

[3] expanded contexts

screenshot 2016-06-27 10 49 50

@getsentry/infrastructure @getsentry/cocoa @getsentry/ui

@dcramer
Copy link
Member

dcramer commented Jun 20, 2016

IMO we should break up the context file. This has gotten to be a pattern lately and it's making some of the modules unwieldy. I dont know how deep we'll go into custom rendering for various styles of context, but as soon as these become more than "here are the field names" it really sucks to have to dig into a giant file to make changes.

tl;dr I'm a big proponent of "one class per file" in most situations

@@ -711,6 +712,7 @@ def create_partitioned_queues(name):
'sentry.interfaces.Csp': 'sentry.interfaces.csp.Csp',
'sentry.interfaces.AppleCrashReport': 'sentry.interfaces.applecrash.AppleCrashReport',
'sentry.interfaces.Breadcrumbs': 'sentry.interfaces.breadcrumbs.Breadcrumbs',
'sentry.interfaces.Contexts': 'sentry.interfaces.contexts.Contexts',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't add the full path

def to_json(self):
return self.data

def flatten_index_value(self, value):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

whats this mean? might be worthwhile to add some docstrings on the base type to explain input/output

@dcramer dcramer force-pushed the feature/contexts branch 2 times, most recently from 4ea483e to b1f7310 Compare June 23, 2016 01:26
@dcramer
Copy link
Member

dcramer commented Jun 23, 2016

We're still going to tweak the UI, but leaving a few notes on that here:

  • If we keep it a tabular grid (or if we have one, which we likely will) we probably should push it down below entries. This will including pushing user below, but arguably is how it should be.
  • We should consider a summary view of the context data towards the top of the page, in a small compact space. The alternative is to throw it into the sidebar, but we haven't figured out a good solution to ensure thats representative yet.
  • We might want to consider (can be a followup) having aggregate-wide information for these things, beyond tags. For example, if we start capturing more hardware stats, it'd be nice to have a breakdown of "90% of these errors had <50mb of free memory".

@dcramer
Copy link
Member

dcramer commented Jun 23, 2016

Some notes:

  • Always three summary context boxes
  • Which three boxes depends on "experience" (web server, web client, desktop, mobile)
  • When data isnt available we show ? + "Unknown X" + instructions if applicable (user)

Experiences:

  • Web (Server): User, Runtime, OS
  • Web (Client): User, Browser, OS
  • Desktop: User, Device, OS (TBD)
  • Mobile: User, Device, OS

In the future it's possible we'll make these configurable, or we'll just do it based on what's available.

@dcramer dcramer force-pushed the feature/contexts branch 2 times, most recently from 45f633b to aa50d23 Compare June 27, 2016 16:56
@dcramer
Copy link
Member

dcramer commented Jun 27, 2016

@dcramer
Copy link
Member

dcramer commented Jun 27, 2016

/cc @bretthoerner since this will be relevant for Android

@dcramer dcramer merged commit cddff4b into master Jun 27, 2016
@dcramer dcramer deleted the feature/contexts branch June 27, 2016 21:34
@github-actions github-actions bot locked and limited conversation to collaborators Dec 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants