Get CodaHale metrics working by starting up the stream consumers #1586
Conversation
| Command command = new Command(); | ||
| command.execute(); | ||
|
|
||
| Thread.sleep(1000); |
There was a problem hiding this comment.
surely you can use await here instead of sleep from https://github.com/awaitility/awaitility
such as
Gauge<Long> successGauge = metricRegistry.getGauges().get("test.test.countSuccess");
await().atMost(1, SECONDS).until(successGauge::getValue, isEqualTo(1L));
There was a problem hiding this comment.
I could, but then I introduce an extra dependency for this specific test. You could argue that a utility like this should be applied across all of the unit tests. My personal opinions is that the current state isn't enhanced by that much by a wholesale change to this style, but I'm happy to be convinced otherwise
|
|
||
| Thread.sleep(1000); | ||
|
|
||
| assertThat((Long) metricRegistry.getGauges().get("test.test.countSuccess").getValue(), is(1L)); |
There was a problem hiding this comment.
Hmm ... should this also test the other gauges ?
how about a counter from each stream ?
RollingCommandEventCounterStream
RollingCommandLatencyDistributionStream
RollingCommandUserLatencyDistributionStream
RollingCommandMaxConcurrencyStream
Which stream to timeouts happen on ?
There was a problem hiding this comment.
Yes, more test coverage here would be great. Would you like to submit a PR?
Fix #1581
Issue was twofold: