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

fix: flatten searchable os distribution fields #81297

Merged
merged 3 commits into from
Nov 28, 2024

Conversation

JoshuaMoelans
Copy link
Member

@JoshuaMoelans JoshuaMoelans commented Nov 26, 2024

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

@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Nov 26, 2024
Copy link

codecov bot commented Nov 26, 2024

Codecov Report

Attention: Patch coverage is 85.71429% with 1 line in your changes missing coverage. Please review.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/sentry/rules/conditions/event_attribute.py 85.71% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #81297      +/-   ##
==========================================
+ Coverage   80.35%   80.36%   +0.01%     
==========================================
  Files        7221     7220       -1     
  Lines      319593   319764     +171     
  Branches    20783    20783              
==========================================
+ Hits       256800   256988     +188     
+ Misses      62398    62381      -17     
  Partials      395      395              

@JoshuaMoelans JoshuaMoelans marked this pull request as ready for review November 27, 2024 09:41
@JoshuaMoelans JoshuaMoelans changed the title (draft) flatten searchable os distribution fields fix: flatten searchable os distribution fields Nov 27, 2024
@kahest kahest requested a review from narsaynorath November 27, 2024 11:22
@kahest
Copy link
Member

kahest commented Nov 27, 2024

@narsaynorath added you as reviewer since you're familiar with #69865 but pls add other folks as needed 🙏

Copy link
Member

@narsaynorath narsaynorath left a comment

Choose a reason for hiding this comment

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

Looks good to me. iirc when we last made this change it was because nested contexts weren't assigned a key properly in clickhouse, so since we're reducing the level of nesting, this should match up properly

@JoshuaMoelans JoshuaMoelans merged commit 6d8d341 into master Nov 28, 2024
49 checks passed
@JoshuaMoelans JoshuaMoelans deleted the joshua/feat/flatten_os_distribution_search branch November 28, 2024 09:26
jan-auer added a commit that referenced this pull request Nov 28, 2024
* master: (219 commits)
  fix: flatten searchable os distribution fields (#81297)
  chore(profiling): Remvoe unused profile functions metrics hook (#81396)
  fix(prompts): Properly return false instead of undefined when prompt data is null (#81404)
  fix(insights): broken screen rendering doc link (#81257)
  fix(rpc): Only groupby when needed (#81403)
  feat(grouping): Tally frame types while building exception grouping components (#81341)
  fix(similarity): Limit > 30 system frame check to Java (#81385)
  feat(alerts): Adds EAP spans results consumer configs (#81365)
  ref(insights): simplify domain view header by using tab links (#81324)
  fix(issues): Add projectId for flag onboarding on click (#81387)
  chore(flamegraphs): Remove unused legacy flamegraph code path (#81381)
  fix(performance): No table overflow + glitchy behaviour (#81378)
  feat(widget-builder): Add feature flag for redesign (#81377)
  feat(profiling): Clean up continuous profiling ui and compat flags (#81260)
  feat(visibility): Clamp date range for `TagStore` queries (#81363)
  test(taskbroker): Add CLI command for sending taskbroker tasks (#81319)
  feat(dashboards): Add ff for favouriting dashboards (#81368)
  fix(trace) match event_id by error (#81370)
  fix(insights): add missing slash on performance moving banner (#81364)
  ref(models): Include event id in `Event` repr (#81345)
  ...
andrewshie-sentry pushed a commit 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 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
@github-actions github-actions bot locked and limited conversation to collaborators Dec 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Scope: Backend Automatically applied to PRs that change backend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants