[GitHub] billoley commented on a change in pull request #291: ACCUMULO-4693: Add instance-specific ProcessName to Hadoop2 metrics

[gi...@git.apache.org]

billoley commented on a change in pull request #291: ACCUMULO-4693: Add instance-specific ProcessName to Hadoop2 metrics
URL: https://github.com/apache/accumulo/pull/291#discussion_r136417788
 
 

 ##########
 File path: server/base/src/main/java/org/apache/accumulo/server/metrics/MetricsSystemHelper.java
 ##########
 @@ -16,20 +16,65 @@
  */
 package org.apache.accumulo.server.metrics;
 
+import java.util.HashMap;
+import java.util.Map;
+
 import org.apache.hadoop.metrics2.MetricsSystem;
 import org.apache.hadoop.metrics2.lib.DefaultMetricsSystem;
+import org.apache.hadoop.metrics2.source.JvmMetrics;
+import org.apache.hadoop.metrics2.source.JvmMetricsInfo;
 
 /**
  *
  */
 public class MetricsSystemHelper {
 
+  private static Map<String,String> serviceNameMap = new HashMap<>();
+  private static String processName = "Unknown";
+
+  static {
+    serviceNameMap.put("master", "Master");
+    serviceNameMap.put("tserver", "TabletServer");
+    serviceNameMap.put("monitor", "Monitor");
+    serviceNameMap.put("gc", "GarbageCollector");
+    serviceNameMap.put("tracer", "Tracer");
+    serviceNameMap.put("shell", "Shell");
+  }
+
+  public static void configure(String application) {
+    String serviceName = application;
+    if (MetricsSystemHelper.serviceNameMap.containsKey(application)) {
+      serviceName = MetricsSystemHelper.serviceNameMap.get(application);
+    }
+
+    // a system property containing 'instance' can be used if more than one TabletServer is started on a host
+    String serviceInstance = "";
+    if (serviceName.equals("TabletServer")) {
+      for (Map.Entry<Object,Object> p : System.getProperties().entrySet()) {
+        if (((String) p.getKey()).contains("instance")) {
+          // get rid of all non-alphanumeric characters and then prefix with a -
+          serviceInstance = "-" + ((String) p.getValue()).replaceAll("[^a-zA-Z0-9]", "");
 
 Review comment:
   I like making the accumulo.service.instance just a number instead of 1_, 2_, 3_, since it can be used for other things (metrics) without parsing.  The reason for the _ is three entries in generic_logger.xml.   I propose that when -Daccumulo.service.instance=X is defined, we also add -Dinstance.sep=_ (or some other named property) right next to it.  generic_logger.xml can use both of these properties.  This part of the launch script would only be linked with the logging config and not the code.   A slight downside is another property on the command line.  Log4j2 would provide several options to avoid this (scripting, environment variables), but we're still on Log4j1.2.  
 
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services