search

addthis / metrics-reporter-config

Apache License 2.0
74 stars 52 forks source link

Allow configuration of any type of metrics reporter #13

Closed tomvandenberge closed 10 years ago

tomvandenberge commented 10 years ago

I wanted to use metrics-reporter-config with a metrics reporter that is not (currently) supported. It turned out that this is not possible to do without modifying the code. The 5 metrics reporters that are currently supported are hard-wired into the code. Adding support for another reporter would require adding another piece of code for that reporter.

I found a quite simple way to allow configuration of any type of metrics reporter, without having to modify the code. The basic idea is to eliminate the compile-time dependencies on the reporters, and turn them into runtime dependencies. In the yaml file, it looks like this:

reporters:  
  - 
    !!com.addthis.metrics.reporter.config.CsvReporterConfig
    outdir: '/tmp/metrics/csv-dir'
    period: 2
    timeunit: 'SECONDS'

So instead of the currently hard-coded collection of reporters (csv, console, ganglia, graphite and riemann), you can configure any type of reporter by listing it under reporters. The only additional requirement is that a yaml tag is added with the fully qualified class name of the reporter.

The reporters are only required to implement the (new) interface com.addthis.metrics.reporter.config.ConfiguredReporter. All that this interface specifies is the enable method that is already implemented by all existing reporters.

Implementors of ConfiguredReporter are responsible for the correct configuration of the reporter, and to start it when enable() is called.

With this approach, the specific reporter properties and methods in ReporterConfig (csv, console, etc) are not necessary anymore. I have left them, to ensure backward compatibility. This means that all existing yaml files will keep working. Maybe it's good to deprecate the fields/methods, and remove them in a future version.

I have implemented a reporter for KairosDB, and it works fine. Please let me know what you think of these changes, and if you'd like to pull them in.

Regards, Tom

cburroughs commented 10 years ago

I like it! Glad it's working for the KariosDB reporter. Cleaning up the compile-time depdency mess that existed just so people could optionally not use something at run-time should make everything nicer for 3rd party reporters. I expect to support the exisitng (and any new core ones) indefinitly using the 'hard-wired' depdencies for backwards compatibility and since the !! is a bit ugly. I've pulled your changes in as 92cc5be94add1dadb80e20195b6f147a6b92b15d Thanks!

One minor note, we dont' use @author tags for the same philosophical reason as the Subversion community http://subversion.apache.org/docs/community-guide/conventions.html#other-conventions. They usually are just auto-dumped by IDEs so I removed them along with other style changes. If that's not okay with you please let me know.