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(metrics): Use bulk_reverse_reslove in metrics #52115

Merged
merged 6 commits into from
Jul 4, 2023

Conversation

RaduW
Copy link
Contributor

@RaduW RaduW commented Jul 4, 2023

This PR uses bulk_reverse_reslove to avoid N+1 query problems.
(this is a cleaned-up version of #52007)

Will rebase once PR #51791 is merged.
fixes: #51227

@RaduW RaduW requested a review from a team as a code owner July 4, 2023 08:58
@RaduW RaduW requested a review from a team July 4, 2023 08:58
@RaduW RaduW requested a review from a team as a code owner July 4, 2023 08:58
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Jul 4, 2023
@codecov
Copy link

codecov bot commented Jul 4, 2023

Codecov Report

Merging #52115 (1c43854) into master (bd53a6c) will increase coverage by 0.00%.
The diff coverage is 90.69%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #52115   +/-   ##
=======================================
  Coverage   79.31%   79.32%           
=======================================
  Files        4901     4901           
  Lines      205111   205139   +28     
  Branches    35063    35070    +7     
=======================================
+ Hits       162689   162720   +31     
+ Misses      37460    37456    -4     
- Partials     4962     4963    +1     
Impacted Files Coverage Δ
src/sentry/sentry_metrics/utils.py 87.01% <77.77%> (-2.99%) ⬇️
...ry/api/endpoints/organization_measurements_meta.py 90.47% <100.00%> (ø)
...c/sentry/release_health/release_monitor/metrics.py 86.11% <100.00%> (+1.26%) ⬆️
src/sentry/snuba/metrics/datasource.py 95.95% <100.00%> (+0.05%) ⬆️
src/sentry/utils/pytest/metrics.py 86.11% <100.00%> (+0.19%) ⬆️

... and 2 files with indirect coverage changes

Copy link
Member

@obostjancic obostjancic left a comment

Choose a reason for hiding this comment

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

Looks great!

src/sentry/sentry_metrics/utils.py Outdated Show resolved Hide resolved
@RaduW RaduW merged commit 34b3834 into master Jul 4, 2023
55 checks passed
@RaduW RaduW deleted the feat/metrics/use_bulk_reverse_resolve2 branch July 4, 2023 12:21
@RaduW RaduW restored the feat/metrics/use_bulk_reverse_resolve2 branch July 4, 2023 12:23
@RaduW RaduW deleted the feat/metrics/use_bulk_reverse_resolve2 branch July 4, 2023 12:24
@RaduW RaduW added the Trigger: Revert add to a merged PR to revert it (skips CI) label Jul 4, 2023
@getsentry-bot
Copy link
Contributor

PR reverted: b14d14a

getsentry-bot added a commit that referenced this pull request Jul 4, 2023
This reverts commit 34b3834.

Co-authored-by: RaduW <5281987+RaduW@users.noreply.github.com>
RaduW added a commit that referenced this pull request Jul 4, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Jul 20, 2023
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 Trigger: Revert add to a merged PR to revert it (skips CI)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use bulk_reverse_resolve in metrics layer
4 participants