-
Notifications
You must be signed in to change notification settings - Fork 16
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
Multiple include: Levels & Labels #1074
Conversation
…include-ui__levels
…include-ui__levels
// We need at least one inclusive filter | ||
if (this.options.type === 'indexed') { | ||
if (filteredFilters.length < 1) { | ||
filteredFilters = filters; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So if we have multiple include filters containing the exclude key, we would run the query with all of them. Maybe this is a reason to exclude the "primary" filter from breakdown views so we know that we're never excluding the only include filter.
But folks might want write a regex include e.g. cluster=~"us-.+"
, and then want to exclude stuff in the UI... but right now I believe this will behave as the current version, including will hide all other options, and excluding will remove that panel from the view (if they have no other inclusion label filters).
This comment was marked as outdated.
This comment was marked as outdated.
…include-ui__levels
* feat: multiple field includes --------- Co-authored-by: Matias Chomicki <matyax@gmail.com>
src/Components/ServiceScene/Breakdowns/LabelValuesBreakdownScene.tsx
Outdated
Show resolved
Hide resolved
src/Components/ServiceScene/Breakdowns/LabelValuesBreakdownScene.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work! I'd appreciate if you can address/respond to the open comments before merging. Thanks!
Made significant changes to how it works.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested the usual happy paths and looks ready to go 👌
Providing the ability to add multiple include filters in a value breakdown.
Related: