Skip to content

fix(events): Handle count_if(span.status) #96692

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 6 commits into from
Aug 1, 2025
Merged

fix(events): Handle count_if(span.status) #96692

merged 6 commits into from
Aug 1, 2025

Conversation

wmak
Copy link
Member

@wmak wmak commented Jul 29, 2025

@wmak wmak requested review from a team as code owners July 29, 2025 20:39
@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Jul 29, 2025
@codecov Codecov
Copy link

codecov bot commented Jul 29, 2025 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master   #96692      +/-   ##
==========================================
+ Coverage   80.00%   80.70%   +0.70%     
==========================================
  Files        8510     8498      -12     
  Lines      374625   373948     -677     
  Branches    24305    24266      -39     
==========================================
+ Hits       299714   301805    +2091     
+ Misses      74534    71766    -2768     
  Partials      377      377

DominikB2014
DominikB2014 approved these changes Jul 30, 2025
View reviewed changes
static/app/views/insights/types.tsx Outdated
@@ -498,7 +498,7 @@ type SpanResponseRaw = {
} & {
[Property in SpanNumberFields as `avg_compare(${Property},${string},${string},${string})`]: number;
} & {
[Property in SpanFields as `count_if(${Property},${string})`]: number;
[Property in SpanFields as `count_if(${Property},equals,${string})`]: number;
Copy link
Contributor

@DominikB2014 DominikB2014 Jul 30, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I think this should be

Suggested change
[Property in SpanFields as `count_if(${Property},equals,${string})`]: number;
[Property in SpanFields as `count_if(${Property},${'equals' | not_equals | etc.},${string})`]: number;

Base automatically changed from wmak/fix/handle-count_if-span.status to master July 31, 2025 16:47
@wmak wmak requested a review from a team as a code owner July 31, 2025 16:47
@wmak wmak requested a review from a team as a code owner July 31, 2025 18:50
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Jul 31, 2025
@github-actions GitHub Actions
Copy link
Contributor

🚨 Warning: This pull request contains Frontend and Backend changes!

It's discouraged to make changes to Sentry's Frontend and Backend in a single pull request. The Frontend and Backend are not atomically deployed. If the changes are interdependent of each other, they must be separated into two pull requests and be made forward or backwards compatible, such that the Backend or Frontend can be safely deployed independently.

Have questions? Please ask in the #discuss-dev-infra channel.

wmak added 2 commits July 31, 2025 14:55
@wmak
chore(insights): Update all if combinators to include operator
f7af13d 
- I'm updating count_if to match discover's api in #96689, so for
  consistency adding the operator to all our existing -if combinator
  usage before deprecating it
@wmak
fix: Missing spots to update
69fd725
@wmak wmak force-pushed the wmak/chore/update-all-ifcombinators-to-include-operator branch from c6f3b8a to 69fd725 Compare July 31, 2025 18:55
@wmak
Copy link
Member Author

wmak commented Jul 31, 2025

Had to force push cause changing the base branch was leaving some hanging backend changes

cursor[bot]

This comment was marked as outdated.

Sign in to view

cursor[bot]

This comment was marked as outdated.

Sign in to view

@wmak wmak enabled auto-merge (squash) August 1, 2025 15:35
@wmak wmak merged commit c1b7153 into master Aug 1, 2025
47 checks passed
@wmak wmak deleted the wmak/chore/update-all-ifcombinators-to-include-operator branch August 1, 2025 15:46
buenaflor pushed a commit that referenced this pull request Aug 4, 2025
@wmak @buenaflor
fix(events): Handle count_if(span.status) (#96692)
1648f74 
- I'm updating count_if to match discover's api in #96689, so for
consistency adding the operator to all our existing -if combinator usage
before deprecating it
andrewshie-sentry pushed a commit that referenced this pull request Aug 4, 2025
@wmak @andrewshie-sentry
fix(events): Handle count_if(span.status) (#96692)
a6139cd 
- I'm updating count_if to match discover's api in #96689, so for
consistency adding the operator to all our existing -if combinator usage
before deprecating it
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cursor cursor[bot] cursor[bot] left review comments

@DominikB2014 DominikB2014 DominikB2014 approved these changes

Assignees
No one assigned
Labels
Scope: Backend Automatically applied to PRs that change backend components Scope: Frontend Automatically applied to PRs that change frontend components
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@wmak @DominikB2014