ctubbsii 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_r136490769
##########
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:
@billoley Please consider that much of what you've just described is no
longer applicable to the current state of the development (master) branch. For
example, `generic_logger.xml` no longer exists, neither does
`accumulo.service.instance` property, or any automatic numbering being assigned
in the launch scripts. I think it may help to frame the discussion in the
context of the current state of the code, rather than the previous releases,
which had a very casual approach to the tight coupling between the server code
and the environment which launched it.
If we need to support injecting some environment into the metrics code, I
think Java system properties are the right way to go. These can be easily
scoped to particular Accumulo behaviors, and easily documented.
We have already added "accumulo.application", which is used (if set,
otherwise "unknown") to identify a server within the AccumuloMonitorAppender.
Perhaps it would be better to more narrowly scope this as
"accumulo.monitor.appender.application", and create a separate
"accumulo.metrics.service.id" (or similar), which is used "as is" (rather than
any parsing) by the metrics framework (if set).
The user can customize the values of these properties in their environment
in a way that suits them, and being separate properties, they can share the
same value or not, as determined by the user. If we need to, we can parse the
value of the metrics property if it needs to be constrained, but if it's just
an arbitrary string, and the only constraint is the user preferences, this can
be done entirely in the launch script by the user.
----------------------------------------------------------------
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:
[email protected]
With regards,
Apache Git Services
