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

Custom percentile reporting #41

Closed
wants to merge 4 commits into from

Conversation

bmorton
Copy link

@bmorton bmorton commented Jun 27, 2013

I set out to add an option to the Librato reporter so that custom/additional percentiles could be reported. I ended up doing some refactoring for the lists of metrics supported for each metric type in various Reporters to reduce some duplication.

I added some tests I thought were helpful and added documentation where it made sense. I'd love some feedback on this, so let me know!

Thanks!

Brian Morton added 4 commits June 26, 2013 22:27
* Add reportable_metrics class method to all metric types
* Add reportable_snapshot_metrics class method to all metric types
* Allow desired percentiles to be passed in for snapshot metrics
…ethods.

* Add VALID_PERCENTILES constant to Snapshot class
* Reference VALID_PERCENTILES to create a convenience method for checking validity
* Add method for taking a list of percentiles and turning them into valid
methods to be called on Snapshot objects
* Update reporters to use the reportable_metrics and reportable_snapshot_metrics class methods
@bmorton
Copy link
Author

bmorton commented Jun 27, 2013

Hmm, apparently 1.8.7 doesn't like assert_not_empty. Additionally, I started looking through some other pull requests and see that there are other pull requests similar to mine.

Have you started pulling out reporters yet? Is a better approach to pull out the LibratoMetrics reporter?

@josephruscio
Copy link

@bmorton I can't speak for @eric, but I think the eventual goal is to pull the Librato reporter out. We haven't had bandwidth to do that yet unfortunately and I don't know if the actual reporter interface has stabilized yet e.g. #37 ?

@eric
Copy link
Owner

eric commented Jul 8, 2013

I made some progress on #37 over the weekend, but still have some unresolved issues on how I'm going to roll it out.

@bmorton bmorton closed this Oct 9, 2021
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.

None yet

3 participants