Skip to content

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.

3 participants