blacklocus / metrics-cloudwatch

A reporter for codahale metrics to Amazon CloudWatch
Apache License 2.0
61 stars 27 forks source link

CloudWatchReporter is tightly coupled with ScheduledReporter #20

Open jlhood opened 7 years ago

jlhood commented 7 years ago

CloudWatchReporter has a lot of great functionality, like the dimension support, but it currently extends ScheduledReporter, so there's limited control over when metrics are published to CloudWatch. I'd like to break out the functionality to publish metrics to CloudWatch into a separate class that can be composed with a ScheduledReporter subclass, but also used standalone.

That way I can use the standalone class to report metrics on a per-request basis. I need this for a serverless application I'm working on since I can't rely on the container continuing to exist and would instead like to create a new MetricRegistry for each request and then publish any metrics collected at the completion of that request.

I've forked the repo and intend to make this change myself and send a PR, but I'm opening this issue to discuss the best way to do this.

jlhood commented 7 years ago

Ideally I'd just change CloudWatchReporter to no longer extend ScheduledReporter and just implement Reporter and work in a standalone way. Then I'd create a ScheduledCloudWatchReporter object that extends ScheduledReporter and have it take a CloudWatchReporter in its constructor and delegate to it in the report() override method.

However that change would not be backwards-compatible so that wouldn't be something I could send a PR for. A backwards-compatible approach would be to create a class like NonScheduledCloudWatchReporter or AdhocCloudWatchReporter (better name suggestions would be appreciated) and move most of the CloudWatchReporter logic there. Then update CloudWatchReporter to take that class in its constructor so it continues to work.

I'm going to code up that approach and send a PR when I have it working. Just wanted to post my thoughts on it since I'm new to the codebase. If you have comments or suggestions, or there's something I'm missing, let me know. Thanks!