-
-
Notifications
You must be signed in to change notification settings - Fork 170
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
feat: add linux distributions to os context #963
base: master
Are you sure you want to change the base?
Conversation
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #963 +/- ##
==========================================
- Coverage 86.53% 82.55% -3.98%
==========================================
Files 40 53 +13
Lines 3736 7860 +4124
Branches 0 1232 +1232
==========================================
+ Hits 3233 6489 +3256
- Misses 503 1260 +757
- Partials 0 111 +111 |
The failing Mingw test is unrelated to the change (a change in the GHA runner image requires us to install |
…solving to memcpy
700a65a
to
44866bb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a few minor nits, looking good!
again, failing test here is due to: #968 |
Users of the Native SDK also want to search for the Linux distributions their events came from: getsentry/sentry-native#943 The corresponding PRs to * develop docs: getsentry/develop#1227 * relay: getsentry/relay#3443 * Native SDK: getsentry/sentry-native#963
Users of the Native SDK also want to search for the Linux distributions their events came from: getsentry/sentry-native#943 The corresponding PRs to * develop docs: getsentry/develop#1227 * relay: getsentry/relay#3443 * Native SDK: getsentry/sentry-native#963
PR is on hold until the context format is clarified. Nested context formats with three layers are unacceptable for the backend to search, so we either change the format in the SDK or map it locally in the sentry backend. Since the format was at the core of how to add this feature from the start, it is best to change the format in the SDK (and with it, in the develop-docs, relay, and sentry backend). This is currently deprioritized. |
…4292) (continuation of #3443 ) <!-- Describe your PR here. --> To make these fields searchable, we had to change from a nested to a flattened approach. This is provided in sentry-native ([relevant PR](getsentry/sentry-native#963)). The update is also tracked on the docs side: getsentry/sentry-docs#11936
<!-- Describe your PR here. --> update to #69865 To search the distribution fields, we cannot have them in a hierarchy `os.distribution.name`, we instead need them flattened to `os.distribution_name` This is part of reworking the following native-SDK PR: getsentry/sentry-native#963 The update is also tracked on the docs side: getsentry/sentry-docs#11936 And on Relay: getsentry/relay#4292
Fixes #943.
This adds Linux distribution meta-data to the os context as follows:
Only
name
andversion
should be indexed.Further steps required before we merge/release this:
name
andversion
: Make os.distribution searchable sentry#69865distribution_name
instead ofdistribution.name
, etc.)master
: fix: 404 links in develop-docs sentry-docs#10768) (feat(native): update contexts interface to flatten distribution keys sentry-docs#11936)os.context
relay#4292)