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(performance): txn summary "has" queries should fall back #73767

Closed
wants to merge 1 commit into from

Conversation

mjq
Copy link
Member

@mjq mjq commented Jul 3, 2024

Currently, has queries in the Transaction Summary filter causes an empty Duration Breakdown, although samples are returned. This happens because the metrics dataset only contains a subset of tags (to make cardinality manageable), so only those tags will return metrics data with has queries.

Since the tags for each metric are inconsistent, instead remove support for has queries to the metrics dataset, which will cause a fallback to the transactions dataset instead. That way queries will return metric results from the indexed transaction data instead of empty results.

Fixes #73591

Currently, `has` queries in the Transaction Summary filter causes an empty
Duration Breakdown, although samples are returned. This happens because the
metrics dataset only contains a subset of tags (to make cardinality
manageable), so only those tags will return metrics data with `has` queries.

Since the tags for each metric are inconsistent, instead remove support for
`has` queries to the metrics dataset, falling back to the transactions dataset
instead. That way queries will return metric results from the indexed
transaction data instead of empty results.

Fixes #73591
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Jul 3, 2024
Copy link

codecov bot commented Jul 3, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.09%. Comparing base (773a299) to head (54cb915).
Report is 841 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #73767      +/-   ##
==========================================
- Coverage   78.09%   78.09%   -0.01%     
==========================================
  Files        6654     6648       -6     
  Lines      297526   297576      +50     
  Branches    51189    51211      +22     
==========================================
+ Hits       232344   232383      +39     
- Misses      58902    58914      +12     
+ Partials     6280     6279       -1     
Files Coverage Δ
src/sentry/search/events/builder/metrics.py 88.48% <100.00%> (-0.03%) ⬇️

... and 48 files with indirect coverage changes

@getsantry
Copy link
Contributor

getsantry bot commented Jul 25, 2024

This pull request has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you add the label WIP, I will leave it alone unless WIP is removed ... forever!


"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

@getsantry getsantry bot added the Stale label Jul 25, 2024
@getsantry getsantry bot closed this Aug 5, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Aug 21, 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 Stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

has: filter not working on Transaction Summary page
1 participant