----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34854/#review87934 -----------------------------------------------------------
core/pom.xml <https://reviews.apache.org/r/34854/#comment140354> Versions should go in the root pom. Also, io.dropwizard.metrics are the same thing as com.codahale.metrics (presumably newer because the latest version is higher and it's now what's mentioned on their website). We already have some dependencides on the latter, and it would be good to keep this all consistent. Can you replace the existing com.codahale.metrics dependencies with io.dropwizard.metrics? And given that we'll have a bunch of these dependencies, it's probably a good idea to create a variable for the version (like we do with some others such as tomcat). For example, "dropwizard.metrics.version" or something. core/src/main/java/org/apache/oozie/util/MetricsInstrumentation.java <https://reviews.apache.org/r/34854/#comment140355> All of these property names should be public static final String class variables core/src/main/java/org/apache/oozie/util/MetricsInstrumentation.java <https://reviews.apache.org/r/34854/#comment140360> Let's just create a String for metricsServerName.trim().toLowerCase() so we don't have to do it so many times core/src/main/java/org/apache/oozie/util/MetricsInstrumentation.java <https://reviews.apache.org/r/34854/#comment140356> All configs show up in the admin/configuration endpoint, so there's no reason to log it this way. If you want to have debug logging with this info, that's still fine, but let's make it more consistent with other log statements. e.g. "Publishing external monitoring to [{0}] every [{1}] seconds" etc core/src/main/java/org/apache/oozie/util/MetricsInstrumentation.java <https://reviews.apache.org/r/34854/#comment140357> This can just be LOG.error( core/src/main/java/org/apache/oozie/util/MetricsInstrumentation.java <https://reviews.apache.org/r/34854/#comment140359> LOG.error( webapp/pom.xml <https://reviews.apache.org/r/34854/#comment140361> From the JIRA, we're excluding this because of a license problem. Can you add a comment in the XML here mentioning that so we don't accidently un-exclude it later? - Robert Kanter On June 15, 2015, 12:49 p.m., Narayan Periwal wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/34854/ > ----------------------------------------------------------- > > (Updated June 15, 2015, 12:49 p.m.) > > > Review request for oozie. > > > Bugs: OOZIE-2251 > https://issues.apache.org/jira/browse/OOZIE-2251 > > > Repository: oozie-git > > > Description > ------- > > We have been logging so many important matrices in oozie-instrumentation.log > . These information is very useful for oozie functional monitoring. But it is > always difficult to get the meaning from flat file. If we expose this > information on some graphing tool, We can get the lot of meaning out of it > and can take some actions based on it. > > > Diffs > ----- > > core/pom.xml 7877773 > > core/src/main/java/org/apache/oozie/service/MetricsInstrumentationService.java > 2a00a73 > core/src/main/java/org/apache/oozie/util/Instrumentation.java 4eb6386 > core/src/main/java/org/apache/oozie/util/MetricsInstrumentation.java > 185b67e > core/src/main/resources/oozie-default.xml 4dc127e > docs/src/site/twiki/AG_Install.twiki 0ce2609 > webapp/pom.xml e42e219 > > Diff: https://reviews.apache.org/r/34854/diff/ > > > Testing > ------- > > Done > > > Thanks, > > Narayan Periwal > >
