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

feat: add linux distributions to os context #963

Merged
merged 36 commits into from
Dec 4, 2024

Conversation

supervacuus
Copy link
Collaborator

@supervacuus supervacuus commented Mar 7, 2024

Fixes #943.

This adds Linux distribution meta-data to the os context as follows:

Screenshot 2024-03-07 at 14 55 00

Only name and version should be indexed.

Further steps required before we merge/release this:

Copy link

github-actions bot commented Mar 7, 2024

Fails
🚫 Please consider adding a changelog entry for the next release.
Messages
📖 Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Instructions and example for changelog

Please add an entry to CHANGELOG.md to the "Unreleased" section. Make sure the entry includes this PR's number.

Example:

## Unreleased

- add linux distributions to os context ([#963](https://github.com/getsentry/sentry-native/pull/963))

If none of the above apply, you can opt out of this check by adding #skip-changelog to the PR description.

Generated by 🚫 dangerJS against 1699009

Copy link

codecov bot commented Mar 7, 2024

Codecov Report

Attention: Patch coverage is 90.47619% with 10 lines in your changes missing coverage. Please review.

Project coverage is 82.55%. Comparing base (235f837) to head (1699009).
Report is 70 commits behind head on master.

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     

@supervacuus
Copy link
Collaborator Author

The failing Mingw test is unrelated to the change (a change in the GHA runner image requires us to install zlib for MinGW).

@supervacuus supervacuus force-pushed the feat/add_linux_distros_to_os_context branch from 700a65a to 44866bb Compare March 20, 2024 18:05
@supervacuus
Copy link
Collaborator Author

@markushi, @Swatinem, can one of you give this a thorough look?

Copy link
Member

@markushi markushi left a 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!

src/sentry_os.c Outdated Show resolved Hide resolved
tests/unit/test_os_release.c Outdated Show resolved Hide resolved
src/sentry_os.c Show resolved Hide resolved
src/sentry_os.c Outdated Show resolved Hide resolved
src/sentry_os.c Show resolved Hide resolved
@supervacuus
Copy link
Collaborator Author

again, failing test here is due to: #968

narsaynorath pushed a commit to getsentry/sentry that referenced this pull request May 17, 2024
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
cmanallen pushed a commit to getsentry/sentry that referenced this pull request May 21, 2024
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
@supervacuus supervacuus mentioned this pull request May 22, 2024
5 tasks
@supervacuus supervacuus marked this pull request as draft June 17, 2024 09:33
@supervacuus
Copy link
Collaborator Author

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.

@JoshuaMoelans JoshuaMoelans marked this pull request as ready for review November 26, 2024 09:12
JoshuaMoelans added a commit to getsentry/relay that referenced this pull request Nov 28, 2024
…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
JoshuaMoelans added a commit to getsentry/sentry that referenced this pull request Nov 28, 2024
<!-- 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
andrewshie-sentry pushed a commit to getsentry/sentry that referenced this pull request Dec 2, 2024
<!-- 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
JoshuaMoelans added a commit to getsentry/sentry that referenced this pull request Dec 4, 2024
<!-- Describe your PR here. -->

As part of the following [sentry-native
PR](getsentry/sentry-native#963) we want to be
able to search on these recently added fields by having them autofill.

Originally added (unflattened) in
#69865
Flattened them for searchability in
#81297
Copy link
Member

@JoshuaMoelans JoshuaMoelans left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

@supervacuus supervacuus merged commit 3baf0c7 into master Dec 4, 2024
31 checks passed
@supervacuus supervacuus deleted the feat/add_linux_distros_to_os_context branch December 4, 2024 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Linux distro meta-data to OS context.
4 participants