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

Filters: add support for filters with multiple values #718

Merged
merged 4 commits into from
Aug 23, 2024

Conversation

matyax
Copy link
Contributor

@matyax matyax commented Aug 23, 2024

Aimed to fix #691 , this PR adds support for all fields and labels with multiple values.

Fields demo:

Demo.mov

Labels demo:

Demo.mov

@matyax matyax requested a review from a team as a code owner August 23, 2024 10:12
@matyax
Copy link
Contributor Author

matyax commented Aug 23, 2024

I did this all with fields, but didn't check labels. Looking now 🧐

@matyax
Copy link
Contributor Author

matyax commented Aug 23, 2024

Added support for labels too.

@matyax matyax changed the title Filters: add support for fields with multiple values Filters: add support for filters with multiple values Aug 23, 2024
@gtk-grafana
Copy link
Contributor

This works, but the value breakdown UIs don't support including multiple yet.

@matyax
Copy link
Contributor Author

matyax commented Aug 23, 2024

Oh absolutely. But you can get there by starting with exclusions.

@gtk-grafana
Copy link
Contributor

It's a good way to keep the size of the PR down. I'm fine with merging this as is and following up in the UI (which I think will be a bigger PR)

@matyax
Copy link
Contributor Author

matyax commented Aug 23, 2024

Agree. If you would like to follow that up, since you recently refactored it, go for it. Otherwise I'll follow this up. Thanks!

@gtk-grafana
Copy link
Contributor

Might be nice to use a multi select variable instead of taking up all this space for variables? Can you create 2 two follow up issues (you can assign the changing of the queries in the breakdowns to allow for multiple includes) so we don't lose track over the weekend?

Copy link
Contributor

@gtk-grafana gtk-grafana left a comment

Choose a reason for hiding this comment

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

LGTM, works well locally

@matyax
Copy link
Contributor Author

matyax commented Aug 23, 2024

Created #723 and #724

@matyax matyax enabled auto-merge (squash) August 23, 2024 12:30
@matyax matyax merged commit 3c181f8 into main Aug 23, 2024
4 checks passed
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.

Detected level: add support for multiple selected levels or hide the option
2 participants