----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53297/#review155061 -----------------------------------------------------------
samza-core/src/main/scala/org/apache/samza/util/MetricsReporterLoader.scala (line 40) <https://reviews.apache.org/r/53297/#comment224908> We're moving away from Scala. All new files should be Java. samza-core/src/main/scala/org/apache/samza/util/Util.scala (line 335) <https://reviews.apache.org/r/53297/#comment224907> This edit looks like a mistake. Did this file need to be modified at all? samza-rest/src/main/java/org/apache/samza/rest/SamzaRestService.java (line 60) <https://reviews.apache.org/r/53297/#comment224910> The Resources will eventually emit metrics too, so I think this value is too specific. samza-rest/src/main/java/org/apache/samza/rest/SamzaRestService.java (line 73) <https://reviews.apache.org/r/53297/#comment224909> What's the purpose of this static factory? The typical reason is to construct the object differently (e.g. a different subclass) based on some parameter. I don't see any value of the approach here. If there is some value, then the constructor should be made private. samza-rest/src/main/java/org/apache/samza/rest/SamzaRestService.java (line 75) <https://reviews.apache.org/r/53297/#comment224911> I don't think the MetricsConfig constructure takes a subset. I think it takes the root and expects to find the "metrics" prefix - Jake Maes On Oct. 31, 2016, 9:18 p.m., Shanthoosh Venkataraman wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/53297/ > ----------------------------------------------------------- > > (Updated Oct. 31, 2016, 9:18 p.m.) > > > Review request for samza and Jake Maes. > > > Repository: samza > > > Description > ------- > > This patch aims at enabling users to define custom reporters to send metrics > from the monitors. Configurations required for the definition of the metrics > reporters follows the same convention as of the samza jobs. > > > Diffs > ----- > > docs/learn/documentation/versioned/rest/monitors.md > 46678bbe5fed99f767c3324dc9578ee1a64cec66 > samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala > e0468ee89c89fd720834461771ebb36475475bcb > > samza-core/src/main/scala/org/apache/samza/metrics/ContainerProcessManagerMetrics.scala > f24beb1e099dd44b15b475e0a4a7f70560c6965e > samza-core/src/main/scala/org/apache/samza/util/MetricsReporterLoader.scala > PRE-CREATION > samza-core/src/main/scala/org/apache/samza/util/Util.scala > c4836f202f7eda1d4e71eac94fd48e46207b0316 > samza-rest/src/main/java/org/apache/samza/rest/SamzaRestConfig.java > 47b0663637f6db187d86961377ee3ee203b73fdb > samza-rest/src/main/java/org/apache/samza/rest/SamzaRestService.java > 2a3e83a24a5343bb53b93fc9d0a647c1b253714b > > samza-rest/src/test/java/org/apache/samza/rest/resources/TestSamzaRestService.java > PRE-CREATION > > samza-yarn/src/main/scala/org/apache/samza/job/yarn/SamzaAppMasterMetrics.scala > 8a5b4aaea6e11a5af999f12d50e5b6135dbc70ca > > Diff: https://reviews.apache.org/r/53297/diff/ > > > Testing > ------- > > Unit tests are done to verify the intended functionality. > > > Thanks, > > Shanthoosh Venkataraman > >
