Refactor reporters to remove duplication#35
Refactor reporters to remove duplication#35BRMatt wants to merge 4 commits intoeric:masterfrom memberful:refactor-reporters
Conversation
First step towards removing duplication in the reporters
Some reporters such as logger and riemann need to record the type of the metric. This type appears to be an underscored version of the class name, so for now that's the default implementation.
|
Thanks for the pull request! I'm currently being cautious about adding report-related things to the metrics classes. A couple things I've noticed, but haven't had a chance to jump on are:
My thought so far was to implement some... I don't know the right word here so I'll call them "adapters"... that would implement some of the behaviors I've seen that are fairly consistent, like turning a registry into a flat list of For some reporters (like the librato metrics one, and a theoretical statsd one), instead of reporting the Metriks.meter('testing').mark
Metriks::FlatteningAdapter.new(Metriks.default).each do |metric_name, value|
puts "#{metric_name}: #{value}"
endwould output: For reporters, like the logger reporter, it would need a different adapter that kept the hierarchy of the metric itself (because we are reporting all of the metrics for a given named metric on a single line). These are still just fairly unpolished ideas, so I'm definitely open to other directions if they make more sense. |
Ah yes, I saw the ticket for moving reporters to their own gems and figured it might be useful to have them decoupled before they're split up. Happened to have a few hours to kill on the train the other day so figured I might try it out! :)
That sounds like an interesting idea, having looked through the reporters you support I can see what you mean about the different formats. When you say consistent behaviours, do you mean things like prepending some prefix to metric names, and not reporting certain metrics(#20), or are there are other more subtle similarities?
Ah, so instead of sending two consecutive
Agreed, metrics definitely shouldn't be aware of reporting, but shouldn't the metric classes be able to serialize/export their own values? Maybe in a more generic method like |
|
May be useful to look at how etsy/statsd implemented the interface between core and backends? I imagine there's a lot of overlap in the requirements. /cc @mheffner |
This removes a lot of the duplication between reporters for extracting a metric's values. Gathering the values has been moved into a
Metriks::Exportablemodule which serializes the various values into a hash.