----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35053/#review87472 -----------------------------------------------------------
samza-api/src/main/java/org/apache/samza/metrics/Histogram.java <https://reviews.apache.org/r/35053/#comment139797> Do we need add counter in the Histogram? In codahale, there is a counter. Not sure if we need it or not. samza-api/src/main/java/org/apache/samza/metrics/Histogram.java <https://reviews.apache.org/r/35053/#comment139794> construct a Histogram, not Timer samza-api/src/main/java/org/apache/samza/metrics/Histogram.java <https://reviews.apache.org/r/35053/#comment139795> name of this histogram samza-api/src/main/java/org/apache/samza/metrics/Histogram.java <https://reviews.apache.org/r/35053/#comment139796> name of this Histogram, not Timer samza-api/src/main/java/org/apache/samza/metrics/Meter.java <https://reviews.apache.org/r/35053/#comment139799> histogram -> meter samza-api/src/main/java/org/apache/samza/metrics/Meter.java <https://reviews.apache.org/r/35053/#comment139800> also add mark(long value) ? samza-api/src/main/java/org/apache/samza/metrics/Meter.java <https://reviews.apache.org/r/35053/#comment139801> I think we need to reset the clock here. Otherwise, it is calculating the mean from job-starting time to calling-getMeanRate time. What we really want is something like the mean value of the last 1 mins/5 mins, right? samza-api/src/main/java/org/apache/samza/metrics/Meter.java <https://reviews.apache.org/r/35053/#comment139802> timer -> meter also added unit tests for the histagram and meter. - Yan Fang On June 4, 2015, 7:22 a.m., Luis De Pombo wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/35053/ > ----------------------------------------------------------- > > (Updated June 4, 2015, 7:22 a.m.) > > > Review request for samza. > > > Repository: samza > > > Description > ------- > > SAMZA-683: add meter and histogram support in the metrics reporter > > > Diffs > ----- > > samza-api/src/main/java/org/apache/samza/metrics/Histogram.java > PRE-CREATION > samza-api/src/main/java/org/apache/samza/metrics/Meter.java PRE-CREATION > samza-api/src/main/java/org/apache/samza/metrics/MetricsRegistry.java > 5a00d018688066d13c56810ce9ac3150064f7ffb > samza-api/src/main/java/org/apache/samza/metrics/MetricsVisitor.java > 75abfe707885becd058a77cf2f30a3d2cfeea3fa > > samza-api/src/main/java/org/apache/samza/metrics/ReadableMetricsRegistryListener.java > 739d68f8ef223ceca0727aa542c0e7ce432c720b > samza-api/src/main/java/org/apache/samza/util/NoOpMetricsRegistry.java > 3df855c2f79345c47355a3d61c785565bf381a83 > samza-core/src/main/scala/org/apache/samza/metrics/MetricsRegistryMap.scala > 40ffee2bfa64a7aaa5fc68399fd6de3a4651a0de > > samza-core/src/main/scala/org/apache/samza/metrics/reporter/JmxReporter.scala > e9661023a04f39d059d879fea2140cb57af3b546 > > samza-core/src/main/scala/org/apache/samza/metrics/reporter/MetricsSnapshotReporter.scala > b6696f823e11a0c2134fc830178e47c3ee857378 > > samza-yarn/src/main/scala/org/apache/samza/webapp/ApplicationMasterRestServlet.scala > 09f4dc32a4b18aeb3accb856c360ea2f95c82673 > > Diff: https://reviews.apache.org/r/35053/diff/ > > > Testing > ------- > > > Thanks, > > Luis De Pombo > >
