eigengo / monitor

Library for monitoring the Typesafe stack-based applications.
Apache License 2.0
127 stars 24 forks source link

This is the initial deposit for `output-codahalemetrics` for Codahale Metrics. #94

Closed vonnagy closed 10 years ago

vonnagy commented 11 years ago

This will satisfy #80 when completed.

Do not merge yet as this needs a bit of discussion around the resolution for #91 which is also an issues with the AkkaIOStatsdCounterInterface.

This is the initial deposit for output-metrics for Codahale Metrics (http://metrics.codahale.com) support. It follows the same general pattern of output-stats.

As described in #91, I also looking for a bit of feedback on naming the metrics as Codahale is a bit more restrictive then those being sent to DataDog. An example of the current output for an actor system named system and prefix named harness would be as follows:

System wide harness.actor-count

System Actors harness.server.system.io-tcp.duration harness.server.system.io-tcp.delivered harness.server.system.io-tcp.delivered.bind

User Actors harness.server.user.system.cluster.duration harness.server.user.system.cluster.delivered harness.server.user.system.cluster.delivered.leaderchanged

janm399 commented 11 years ago

Great; I'll take a look later on today. We will soon be moving away from Travis CI to our own Jenkins to implement a better CI / CD approach.

I'll see if I can sort out the Travis CI build errors, but we are all getting consistent successful builds on our own machines, and consistent failures on Travis CI.

janm399 commented 11 years ago

What do you think about calling the module output-codahalemetrics? Just ...-metrics might be ambiguous in the future.

Now, to code.

Intuitively, it would seem that an obvious refactoring could be moving the messages that are sent to the actors (subtypes of the MetricsStatistic) to the output module so that we can reuse them. But in this case, I think that the intuition is wrong. The messages sent to the actors map to the the statsd messages exactly, but they don't map exactly to the Codahale Metrics. So, it would make more sense to define the CM messages to match the CM API, and to perform the required conversions in the implementations of the CounterInterface.

I was going to write that I'm not sure what benefit we get from providing two implementations of the CounterInterface: one that uses (for the lack of a better word here) blocking execution, and one that wraps the calls in an actor that ultimately lives in another ActorSystem. I can't quite see what should be the optimal solution, so let's keep both. (And same applies to Datadog / statsd.)

vonnagy commented 11 years ago

As for the blocking implementation of CounterInterface as defined in MetricsCounterInterface, I only really included that since the statsd module had it as well. I am also not sure of the benefits of this so am open to remove it if we determine it is not needed.

I will rename the module and namespaces to use codahalemetrics instead of metrics as that makes it clearer.

janm399 commented 11 years ago

I'm at a client today, so I'll take a look tomorrow.

janm399 commented 11 years ago

I'm not forgetting you--I promise--but I've had to deal with other stuff today. Tomorrow, I'm out again, but back on Friday.

vonnagy commented 11 years ago

No worries. I am out the rest of the week for Thanksgiving here in the US, but I will check in on any comments you might have.

vonnagy commented 11 years ago

@janm399 I was adding some documentation and then will wait until the PR gets reviewed.

janm399 commented 11 years ago

OK. I'm back from the madness of Scala eXchange and CodeMesh, so I'll dive into some Scala over the weekend. Thanks for your contributions & patience. Let's now rock it.

janm399 commented 10 years ago

I've tidied up the code and merged in version 0.3; can you now contribute to the documentation?

vonnagy commented 10 years ago

Yes, I already have most of the documentation complete so hopefully be tomorrow I should have it ready to pull. Thanks.