Enable oshi ProcessMetrics in javaagent (and refactor oshi instrument…

…ation) (open-telemetry#5281) * Enable oshi ProcessMetrics in javaagent (and refactor oshi instrumentation) * Get rid of double async instrument registration logs * spotless
RashmiRam · May 23, 2022 · e9c4be6 · e9c4be6
1 parent 4f67a02
commit e9c4be6
Show file tree

Hide file tree

Showing 16 changed files with 259 additions and 149 deletions.
diff --git a/instrumentation/oshi/javaagent/build.gradle.kts b/instrumentation/oshi/javaagent/build.gradle.kts
@@ -17,5 +17,11 @@ dependencies {
 
   library("com.github.oshi:oshi-core:5.3.1")
 
-  testImplementation("com.google.guava:guava")
+  testImplementation(project(":instrumentation:oshi:testing"))
+}
+
+tasks {
+  withType<Test>().configureEach {
+    jvmArgs("-Dotel.instrumentation.oshi.experimental-metrics.enabled=true")
+  }
 }
diff --git a/...nt/src/main/java/io/opentelemetry/javaagent/instrumentation/oshi/MetricsRegistration.java b/...nt/src/main/java/io/opentelemetry/javaagent/instrumentation/oshi/MetricsRegistration.java
@@ -0,0 +1,30 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.javaagent.instrumentation.oshi;
+
+import io.opentelemetry.instrumentation.api.config.Config;
+import io.opentelemetry.instrumentation.oshi.ProcessMetrics;
+import io.opentelemetry.instrumentation.oshi.SystemMetrics;
+import java.util.concurrent.atomic.AtomicBoolean;
+
+public final class MetricsRegistration {
+
+  private static final AtomicBoolean registered = new AtomicBoolean();
+
+  public static void register() {
+    if (registered.compareAndSet(false, true)) {
+      SystemMetrics.registerObservers();
+
+      // ProcessMetrics don't follow the spec
+      if (Config.get()
+          .getBoolean("otel.instrumentation.oshi.experimental-metrics.enabled", false)) {
+        ProcessMetrics.registerObservers();
+      }
+    }
+  }
+
+  private MetricsRegistration() {}
+}
diff --git a/.../main/java/io/opentelemetry/javaagent/instrumentation/oshi/SystemInfoInstrumentation.java b/.../main/java/io/opentelemetry/javaagent/instrumentation/oshi/SystemInfoInstrumentation.java
@@ -11,7 +11,6 @@
 import static net.bytebuddy.matcher.ElementMatchers.isStatic;
 import static net.bytebuddy.matcher.ElementMatchers.named;
 
-import io.opentelemetry.instrumentation.oshi.SystemMetrics;
 import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
 import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
 import net.bytebuddy.asm.Advice;
@@ -44,7 +43,7 @@ public static class GetCurrentPlatformEnumAdvice {
 
     @Advice.OnMethodEnter(suppress = Throwable.class)
     public static void onEnter() {
-      SystemMetrics.registerObservers();
+      MetricsRegistration.register();
     }
   }
 }
diff --git a/...javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/oshi/OshiTest.groovy b/...javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/oshi/OshiTest.groovy
diff --git a/...ent/src/test/java/io/opentelemetry/javaagent/instrumentation/oshi/ProcessMetricsTest.java b/...ent/src/test/java/io/opentelemetry/javaagent/instrumentation/oshi/ProcessMetricsTest.java
@@ -0,0 +1,25 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.javaagent.instrumentation.oshi;
+
+import io.opentelemetry.instrumentation.oshi.AbstractProcessMetricsTest;
+import io.opentelemetry.instrumentation.testing.junit.AgentInstrumentationExtension;
+import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+class ProcessMetricsTest extends AbstractProcessMetricsTest {
+
+  @RegisterExtension
+  public static final InstrumentationExtension testing = AgentInstrumentationExtension.create();
+
+  @Override
+  protected void registerMetrics() {}
+
+  @Override
+  protected InstrumentationExtension testing() {
+    return testing;
+  }
+}
diff --git a/...gent/src/test/java/io/opentelemetry/javaagent/instrumentation/oshi/SystemMetricsTest.java b/...gent/src/test/java/io/opentelemetry/javaagent/instrumentation/oshi/SystemMetricsTest.java
@@ -0,0 +1,25 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.javaagent.instrumentation.oshi;
+
+import io.opentelemetry.instrumentation.oshi.AbstractSystemMetricsTest;
+import io.opentelemetry.instrumentation.testing.junit.AgentInstrumentationExtension;
+import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+class SystemMetricsTest extends AbstractSystemMetricsTest {
+
+  @RegisterExtension
+  public static final InstrumentationExtension testing = AgentInstrumentationExtension.create();
+
+  @Override
+  protected void registerMetrics() {}
+
+  @Override
+  protected InstrumentationExtension testing() {
+    return testing;
+  }
+}
diff --git a/instrumentation/oshi/library/build.gradle.kts b/instrumentation/oshi/library/build.gradle.kts
@@ -5,7 +5,5 @@ plugins {
 dependencies {
   library("com.github.oshi:oshi-core:5.3.1")
 
-  testImplementation("io.opentelemetry:opentelemetry-sdk-metrics")
-  testImplementation(project(":testing-common"))
-  testImplementation("org.assertj:assertj-core")
+  testImplementation(project(":instrumentation:oshi:testing"))
 }
diff --git a/...tion/oshi/library/src/main/java/io/opentelemetry/instrumentation/oshi/ProcessMetrics.java b/...tion/oshi/library/src/main/java/io/opentelemetry/instrumentation/oshi/ProcessMetrics.java
@@ -22,15 +22,15 @@ private ProcessMetrics() {}
   /** Register observers for java runtime metrics. */
   public static void registerObservers() {
     // TODO(anuraaga): registerObservers should accept an OpenTelemetry instance
-    Meter meter = GlobalOpenTelemetry.get().getMeterProvider().get(ProcessMetrics.class.getName());
+    Meter meter = GlobalOpenTelemetry.get().getMeterProvider().get("io.opentelemetry.oshi");
     SystemInfo systemInfo = new SystemInfo();
     OperatingSystem osInfo = systemInfo.getOperatingSystem();
     OSProcess processInfo = osInfo.getProcess(osInfo.getProcessId());
 
     meter
         .upDownCounterBuilder("runtime.java.memory")
         .setDescription("Runtime Java memory")
-        .setUnit("bytes")
+        .setUnit("By")
         .buildWithCallback(
             r -> {
               processInfo.updateAttributes();
@@ -41,12 +41,12 @@ public static void registerObservers() {
     meter
         .gaugeBuilder("runtime.java.cpu_time")
         .setDescription("Runtime Java CPU time")
-        .setUnit("seconds")
+        .setUnit("ms")
         .buildWithCallback(
             r -> {
               processInfo.updateAttributes();
-              r.record(processInfo.getUserTime() * 1000, Attributes.of(TYPE_KEY, "user"));
-              r.record(processInfo.getKernelTime() * 1000, Attributes.of(TYPE_KEY, "system"));
+              r.record(processInfo.getUserTime(), Attributes.of(TYPE_KEY, "user"));
+              r.record(processInfo.getKernelTime(), Attributes.of(TYPE_KEY, "system"));
             });
   }
 }
diff --git a/...ation/oshi/library/src/main/java/io/opentelemetry/instrumentation/oshi/SystemMetrics.java b/...ation/oshi/library/src/main/java/io/opentelemetry/instrumentation/oshi/SystemMetrics.java
@@ -30,8 +30,7 @@ private SystemMetrics() {}
   /** Register observers for system metrics. */
   public static void registerObservers() {
     // TODO(anuraaga): registerObservers should accept an OpenTelemetry instance
-    Meter meter =
-        GlobalOpenTelemetry.get().getMeterProvider().get("io.opentelemetry.instrumentation.oshi");
+    Meter meter = GlobalOpenTelemetry.get().getMeterProvider().get("io.opentelemetry.oshi");
     SystemInfo systemInfo = new SystemInfo();
     HardwareAbstractionLayer hal = systemInfo.getHardware();
 

diff --git a/...oshi/library/src/test/java/io/opentelemetry/instrumentation/oshi/AbstractMetricsTest.java b/...oshi/library/src/test/java/io/opentelemetry/instrumentation/oshi/AbstractMetricsTest.java
diff --git a/.../oshi/library/src/test/java/io/opentelemetry/instrumentation/oshi/ProcessMetricsTest.java b/.../oshi/library/src/test/java/io/opentelemetry/instrumentation/oshi/ProcessMetricsTest.java
@@ -5,30 +5,22 @@
 
 package io.opentelemetry.instrumentation.oshi;
 
-import static io.opentelemetry.sdk.testing.assertj.MetricAssertions.assertThat;
+import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension;
+import io.opentelemetry.instrumentation.testing.junit.LibraryInstrumentationExtension;
+import org.junit.jupiter.api.extension.RegisterExtension;
 
-import org.junit.jupiter.api.Test;
+class ProcessMetricsTest extends AbstractProcessMetricsTest {
 
-public class ProcessMetricsTest extends AbstractMetricsTest {
+  @RegisterExtension
+  public static final InstrumentationExtension testing = LibraryInstrumentationExtension.create();
 
-  @Test
-  public void test() {
+  @Override
+  protected void registerMetrics() {
     ProcessMetrics.registerObservers();
+  }
 
-    waitAndAssertMetrics(
-        metric ->
-            metric
-                .hasName("runtime.java.memory")
-                .hasUnit("bytes")
-                .hasLongSum()
-                .points()
-                .anySatisfy(point -> assertThat(point.getValue()).isPositive()),
-        metric ->
-            metric
-                .hasName("runtime.java.cpu_time")
-                .hasUnit("seconds")
-                .hasDoubleGauge()
-                .points()
-                .anySatisfy(point -> assertThat(point.getValue()).isPositive()));
+  @Override
+  protected InstrumentationExtension testing() {
+    return testing;
   }
 }
diff --git a/...n/oshi/library/src/test/java/io/opentelemetry/instrumentation/oshi/SystemMetricsTest.java b/...n/oshi/library/src/test/java/io/opentelemetry/instrumentation/oshi/SystemMetricsTest.java
@@ -5,34 +5,22 @@
 
 package io.opentelemetry.instrumentation.oshi;
 
-import static io.opentelemetry.sdk.testing.assertj.MetricAssertions.assertThat;
+import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension;
+import io.opentelemetry.instrumentation.testing.junit.LibraryInstrumentationExtension;
+import org.junit.jupiter.api.extension.RegisterExtension;
 
-import org.junit.jupiter.api.Test;
+class SystemMetricsTest extends AbstractSystemMetricsTest {
 
-public class SystemMetricsTest extends AbstractMetricsTest {
+  @RegisterExtension
+  public static final InstrumentationExtension testing = LibraryInstrumentationExtension.create();
 
-  @Test
-  public void test() {
+  @Override
+  protected void registerMetrics() {
     SystemMetrics.registerObservers();
+  }
 
-    waitAndAssertMetrics(
-        metric ->
-            metric
-                .hasName("system.memory.usage")
-                .hasUnit("By")
-                .hasLongSum()
-                .points()
-                .anySatisfy(point -> assertThat(point.getValue()).isPositive()),
-        metric ->
-            metric
-                .hasName("system.memory.utilization")
-                .hasUnit("1")
-                .hasDoubleGauge()
-                .points()
-                .anySatisfy(point -> assertThat(point.getValue()).isPositive()),
-        metric -> metric.hasName("system.network.io").hasUnit("By").hasLongSum(),
-        metric -> metric.hasName("system.network.packets").hasUnit("packets").hasLongSum(),
-        metric -> metric.hasName("system.network.errors").hasUnit("errors").hasLongSum(),
-        metric -> metric.hasName("system.disk.operations").hasUnit("operations").hasLongSum());
+  @Override
+  protected InstrumentationExtension testing() {
+    return testing;
   }
 }
diff --git a/instrumentation/oshi/testing/build.gradle.kts b/instrumentation/oshi/testing/build.gradle.kts
@@ -0,0 +1,7 @@
+plugins {
+  id("otel.java-conventions")
+}
+
+dependencies {
+  api(project(":testing-common"))
+}