feat(contexts): Normalize known os.name to render UI context icons` #8231
Conversation
mitsuhiko
left a comment
mitsuhiko
left a comment
There was a problem hiding this comment.
I really think we should map from raw_description to name etc. instead of reusing name for this. I see value in the original data.
bruno-garcia
changed the title
|
@mitsuhiko in order to do that I've sent a PR on the SDK first to send the data via Updated the PR to use |
jan-auer
left a comment
jan-auer
left a comment
There was a problem hiding this comment.
LGTM in general, just note the one comment on indexed_fields.
src/sentry/interfaces/contexts.py
Outdated
| indexed_fields = { | ||
| '': u'{name} {version}', | ||
| 'name': u'{name}', | ||
| 'raw_description': u'{raw_description}', |
There was a problem hiding this comment.
I think we should not create tags for raw_description
src/sentry/utils/os_normalization.py
Outdated
|
|
||
| # If there's no name and version, attempts to infer from raw_description | ||
| def normalize(data): | ||
| raw_description = data.get('raw_description', None) |
There was a problem hiding this comment.
nit: data.get('foo') will default to None, so you could omit the second parameter.
src/sentry/utils/os_normalization.py
Outdated
| windows = re.compile('^(Microsoft )?Windows (NT )?(?P<version>\d+\.\d+\.\d+).*$') | ||
| # Environment.OSVersion or RuntimeInformation.OSDescription (uname) | ||
| # on Mono and CoreCLR on macOS, iOS, Linux, etc | ||
| uname = re.compile('^(?P<name>[a-zA-Z]+) (?P<version>\d+\.\d+\.\d+(\.[1-9]+)?).*$') |
There was a problem hiding this comment.
nit: Most regexes in sentry have this naming style _windows_re, _uname_re
There was a problem hiding this comment.
I would add (?i) to make it case insensitive (the windows one)
src/sentry/utils/os_normalization.py
Outdated
| uname = re.compile('^(?P<name>[a-zA-Z]+) (?P<version>\d+\.\d+\.\d+(\.[1-9]+)?).*$') | ||
|
|
||
|
|
||
| # If there's no name and version, attempts to infer from raw_description |
There was a problem hiding this comment.
nit: Move this comment into the function above the line.
src/sentry/utils/os_normalization.py
Outdated
| windows = re.compile('^(Microsoft )?Windows (NT )?(?P<version>\d+\.\d+\.\d+).*$') | ||
| # Environment.OSVersion or RuntimeInformation.OSDescription (uname) | ||
| # on Mono and CoreCLR on macOS, iOS, Linux, etc | ||
| uname = re.compile('^(?P<name>[a-zA-Z]+) (?P<version>\d+\.\d+\.\d+(\.[1-9]+)?).*$') |
There was a problem hiding this comment.
I would add (?i) to make it case insensitive (the windows one)
|
👷 cmon travis |
…8231) Following efforts to send more contextual data from the C# SDK, this PR intends to improve the visualization of the data extracted from the clients.
Following efforts to send more contextual data from the C# SDK, this PR intends to improve the visualization of the data extracted from the clients.