[
https://issues.apache.org/jira/browse/SAMZA-349?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14070442#comment-14070442
]
Chris Riccomini commented on SAMZA-349:
---------------------------------------
bq. Personally I think that resetting every 60 seconds makes much more sense,
e.g. because it allows you to sum the rate of events in several parallel
processes, it's not so dependent on whether you're using 32-bit or 64-bit
numbers, etc. So I'm in favour of this patch, I just wanted to call out the
potential compatibility issue.
Yea, that's why I did it the way it is. I also agree it makes more sense to
reset, though. It's possible for us to make the reset configurable, but that
might lead to more confusion.
bq. Also, since we only refresh the metrics in MetricsSnapshotReporter (kafka
metrics stream) but do not reset in the JMX report, will this be a little
confusing for users?
I think it should be OK. The metrics reporters are all independent
implementations, so it should be OK to alter how we handle metrics on a
per-reporter basis.
> MetricsSnapshotReporter does not refresh metrics every 60 seconds
> -----------------------------------------------------------------
>
> Key: SAMZA-349
> URL: https://issues.apache.org/jira/browse/SAMZA-349
> Project: Samza
> Issue Type: Bug
> Reporter: Yan Fang
> Assignee: Yan Fang
> Attachments: SAMZA-349.patch
>
>
> If my understanding is correct, the metrics we provide are for every 60
> seconds and all counters will be reset every 60 seconds. Current the
> MetricsSnapshotReporter seems missing this implementation. It sends out the
> metrics every 60 seconds but does not reset the counter value.
> {code}
> registry.getGroup(group).foreach {
> case (name, metric) =>
> metric.visit(new MetricsVisitor {
> def counter(counter: Counter) = groupMsg.put(name,
> counter.getCount: java.lang.Long)
> def gauge[T](gauge: Gauge[T]) = groupMsg.put(name,
> gauge.getValue.asInstanceOf[Object])
> })
> }
> {code}
--
This message was sent by Atlassian JIRA
(v6.2#6252)
