This repository was archived by the owner on Oct 25, 2023. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 53
Enable Topic Exclusion #57
Open
abraham-leal
wants to merge
19
commits into
Dabz:master
Choose a base branch
from
abraham-leal:enable-exclude-topics
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Dabz
reviewed
Nov 24, 2020
Dabz
reviewed
Nov 24, 2020
Dabz
reviewed
Nov 24, 2020
Dabz
reviewed
Nov 24, 2020
Dabz
reviewed
Nov 24, 2020
Dabz
reviewed
Nov 24, 2020
Alright, please let me know if you have any comments on the latest commit. If not, I'll still wait until the Metrics API can handle the Unary Filter well to open it for merging. |
Merged with v2 branch, should be ready for merge after v2 is on master. |
Hey @Dabz, this PR is ready for review when you have time. I'm still doing some perf testing to make sure the new functionality doesn't slow down the exporter much, but the logic will probably stay the same if I do any performance improvements. :) |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resolves #54.
What changed?
The query will now be optimized to either:
To filter results of a query, excludeTopicsRegex is now supported to not display results in the /metrics endpoint if it matches the regex provided. This regex field is a list, allowing the matching of multiple regex.
NOTE: Regex filtering does NOT improve the query, just removes the output.
Is there any testing?
Yes, unit testing has been implemented in modified classes, namely:
Why is this a Draft PR?
The Confluent Metrics API currently is having some trouble with the Unary operator (used to optimize the query filtering).
This PR is not ready for merging until that filter quality is resolved and some more testing can happen.
Open for discussion
First time contributing, so please critique my variable naming / style to conform to some other codebase implied standards.
I tried to adhere to them, hopefully there aren't that many hiccups. Some files are changed just because I found some typos.