-
Notifications
You must be signed in to change notification settings - Fork 46
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
Percentile filter #73
base: master
Are you sure you want to change the base?
Conversation
Enhanced code to filter quantiles based on user choice Included --cql-ssl option to crated encrypted native connection Added latest version of jars as the older version are having some vulnerabilities
Used netty 4.1.42.Final to be inline with another PR
…le-filter The master branch contains some important fixes.
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.
I'm thinking that it would be better to exclude the quantiles in the output writer layer, ie. by just not writing them to the socket, rather than modifying every code path that deals with quantiles.
common/src/main/java/com/zegelin/cassandra/exporter/CollectorFunctions.java
Outdated
Show resolved
Hide resolved
common/src/main/java/com/zegelin/cassandra/exporter/CollectorFunctions.java
Outdated
Show resolved
Hide resolved
common/src/main/java/com/zegelin/cassandra/exporter/CollectorFunctions.java
Outdated
Show resolved
Hide resolved
common/src/main/java/com/zegelin/cassandra/exporter/Harvester.java
Outdated
Show resolved
Hide resolved
standalone/src/main/java/com/zegelin/cassandra/exporter/Application.java
Outdated
Show resolved
Hide resolved
Check out We could filter all of them here:
|
|
||
|
||
Iterator<Interval> itr = e.samplingCounting.getIntervals().iterator(); | ||
ArrayList<Interval> filtered = Lists.newArrayList(); |
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.
Creating a new list here isn't necessary. Lets just use Iterables.filter(...)
.
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.
As you suggested, I will apply filter it in TextFormatMetricFormatWriter and revert the above change.
Applied quantile filtering while writing metrics into socket. Corrected formating. Reverted netty jar version changes.
Appended the new line at the end of files which removed while restoring previous changes from repo.
@zegelin thanks for your review and suggestion. I have made the changes, please review. |
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.
Some minor code formatting changes need. Otherwise, looks good to me!
I'll raise a separate improvement ticket to look into some kind of auto linter/formatter so that I don't have to keep bugging you to add a few spaces :)
standalone/src/main/java/com/zegelin/cassandra/exporter/Application.java
Outdated
Show resolved
Hide resolved
common/src/main/java/com/zegelin/cassandra/exporter/cli/HarvesterOptions.java
Outdated
Show resolved
Hide resolved
@zegelin , thanks for your review. I have fixed formatting issues. |
@zegelin, can you please approve and merge my pull request if you don't see any issues? |
3b88272
to
84cb7cd
Compare
Enhanced code to filter quantiles based on user choice
Included --cql-ssl option to crated encrypted native connection
Added latest version of jars as the older version are having some
vulnerabilities