Merge pull request DataDog#1962 from DataDog/mcculls/useSystemAccessForMetricsJMX

mcculls · web-flow · commit 20473c0b54eb · 2020-10-09T12:09:38.000+01:00
Use SystemAccess to query JMX in health metrics
diff --git a/dd-trace-core/src/main/java/datadog/trace/core/monitor/CPUTimer.java b/dd-trace-core/src/main/java/datadog/trace/core/monitor/CPUTimer.java
@@ -1,12 +1,10 @@
 package datadog.trace.core.monitor;
 
 import com.timgroup.statsd.StatsDClient;
-import java.lang.management.ManagementFactory;
-import java.lang.management.ThreadMXBean;
+import datadog.trace.core.util.SystemAccess;
 
 public class CPUTimer extends Timer {
 
-  private final ThreadMXBean threadMXBean = ManagementFactory.getThreadMXBean();
   private final StatsDClient statsd;
 
   private final String name;
@@ -23,21 +21,25 @@ public class CPUTimer extends Timer {
   @Override
   public Recording start() {
     super.start();
-    this.start = threadMXBean.getCurrentThreadCpuTime();
+    this.start = SystemAccess.getCurrentThreadCpuTime();
     return this;
   }
 
   @Override
   public void reset() {
-    long cpuNanos = threadMXBean.getCurrentThreadCpuTime();
-    this.cpuTime += (cpuNanos - start);
+    long cpuNanos = SystemAccess.getCurrentThreadCpuTime();
+    if (start > 0) {
+      this.cpuTime += (cpuNanos - start);
+    }
     this.start = cpuNanos;
     super.reset();
   }
 
   @Override
   public void stop() {
-    this.cpuTime += threadMXBean.getCurrentThreadCpuTime() - start;
+    if (start > 0) {
+      this.cpuTime += SystemAccess.getCurrentThreadCpuTime() - start;
+    }
     super.stop();
   }
 
diff --git a/dd-trace-core/src/main/java/datadog/trace/core/monitor/Monitoring.java b/dd-trace-core/src/main/java/datadog/trace/core/monitor/Monitoring.java
@@ -1,7 +1,5 @@
 package datadog.trace.core.monitor;
 
-import static java.lang.management.ManagementFactory.getThreadMXBean;
-
 import com.timgroup.statsd.NoOpStatsDClient;
 import com.timgroup.statsd.StatsDClient;
 import java.util.concurrent.TimeUnit;
@@ -57,10 +55,7 @@ public Recording newCPUTimer(final String name) {
     if (!enabled) {
       return NoOpRecording.NO_OP;
     }
-    if (getThreadMXBean().isCurrentThreadCpuTimeSupported()) {
-      return new CPUTimer(name, statsd, flushAfterNanos);
-    }
-    return newTimer(name);
+    return new CPUTimer(name, statsd, flushAfterNanos);
   }
 
   public Counter newCounter(final String name) {
diff --git a/dd-trace-core/src/main/java/datadog/trace/core/util/JmxSystemAccessProvider.java b/dd-trace-core/src/main/java/datadog/trace/core/util/JmxSystemAccessProvider.java
@@ -14,6 +14,7 @@
 final class JmxSystemAccessProvider implements SystemAccessProvider {
   private final ThreadMXBean threadMXBean = ManagementFactory.getThreadMXBean();
   private final RuntimeMXBean runtimeMXBean = ManagementFactory.getRuntimeMXBean();
+  private final boolean cpuTimeSupported = threadMXBean.isCurrentThreadCpuTimeSupported();
 
   public static final JmxSystemAccessProvider INSTANCE = new JmxSystemAccessProvider();
 
@@ -23,7 +24,7 @@ final class JmxSystemAccessProvider implements SystemAccessProvider {
    */
   @Override
   public long getThreadCpuTime() {
-    return threadMXBean.getCurrentThreadCpuTime();
+    return cpuTimeSupported ? threadMXBean.getCurrentThreadCpuTime() : Long.MIN_VALUE;
   }
 
   /**
diff --git a/dd-trace-core/src/main/java/datadog/trace/core/util/SystemAccess.java b/dd-trace-core/src/main/java/datadog/trace/core/util/SystemAccess.java
@@ -21,8 +21,8 @@ public static void disableJmx() {
 
   /** Enable JMX accesses */
   public static void enableJmx() {
-    if (!Config.get().isProfilingEnabled()) {
-      log.debug("Will not enable JMX access. Profiling is disabled.");
+    if (!Config.get().isProfilingEnabled() && !Config.get().isHealthMetricsEnabled()) {
+      log.debug("Will not enable JMX access. Profiling and metrics are both disabled.");
       return;
     }
     try {
diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/monitor/TimingTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/monitor/TimingTest.groovy
@@ -1,6 +1,9 @@
 package datadog.trace.core.monitor
 
 import com.timgroup.statsd.StatsDClient
+import datadog.trace.agent.test.utils.ConfigUtils
+import datadog.trace.api.Config
+import datadog.trace.core.util.SystemAccess
 import datadog.trace.util.test.DDSpecification
 import org.junit.Assert
 import org.junit.Assume
@@ -95,6 +98,7 @@ class TimingTest extends DDSpecification {
     Monitoring monitoring = new Monitoring(statsd, 100, MILLISECONDS)
     def timer = monitoring.newCPUTimer("my_timer")
     when:
+    ConfigUtils.withConfigOverride("${Config.HEALTH_METRICS_ENABLED}", "true", { SystemAccess.enableJmx() })
     Recording recording = timer.start()
     Thread.sleep(200)
     recording.close()
@@ -105,6 +109,8 @@ class TimingTest extends DDSpecification {
     1 * statsd.gauge("my_timer", { it > MILLISECONDS.toNanos(200) }, { it[0] == "stat:max" && it[1].startsWith("thread:") })
     1 * statsd.gauge("my_timer.cpu", { it > 0 }, { it[0].startsWith("thread:") })
     0 * _
+    cleanup:
+    SystemAccess.disableJmx()
   }
 }
 
