search

posix4e / rust-metrics

Multi reporter metrics library (carbon, graphite, postgresql, prometheus)
Other
99 stars 19 forks source link

API change ideas #55

Closed waywardmonkeys closed 8 years ago

waywardmonkeys commented 8 years ago

I've been trying to iteratively simplify the API, but it has proved to be rather hard to do in pieces and might require just a big set of changes.

1) Remove Registry and just let the Reporter store the metrics that it will report on. 2) Remove the enum Metric and add a trait Metric which we will impl from our various metrics. 3) Have the Reporter store an Arc<Metric> instead of the Metric directly. 4) Switch from pattern matching on Metric::* to using double dispatch so we have a report method on each metric type which calls back to a type specific function on the reporter. (so, Meter::report(reporter) would call reporter.report_meter which would do the right thing. Now, reporting in a loop is just iterating over the metrics and calling report on them...) 5) Separate reporting from spawning a thread so that a reporting cycle can be triggered independently of the thread being created. (This will let us improve testing.)

waywardmonkeys commented 8 years ago

Having the Reporter instance store metrics on its own lets it optimize that for itself and its own needs ... so things that don't need labels don't have to know about them. And the Carbon reporter will be able to improve how it does prefixing since it can then prefix on adding the metric rather than at every report cycle.

waywardmonkeys commented 8 years ago

https://github.com/waywardmonkeys/rust-metrics/commit/65f6b0b68e4135a86232e554b1292a69c8f4b88e is a start on some of this.

posix4e commented 8 years ago

Looking good On Sun, Jul 17, 2016 at 7:13 PM Bruce Mitchener notifications@github.com wrote:

waywardmonkeys@65f6b0b https://github.com/waywardmonkeys/rust-metrics/commit/65f6b0b68e4135a86232e554b1292a69c8f4b88e is a start on some of this.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/posix4e/rust-metrics/issues/55#issuecomment-233218350, or mute the thread https://github.com/notifications/unsubscribe-auth/AAxN2zS5FF6WVKwSXFGWDjGf9VlkWhfdks5qWuFigaJpZM4JOXRz .

Sent from my mobile 4045076749 Alex Newman

posix4e commented 8 years ago

+1 on getting rid of registry btw