Update units of time based metrics from millis to seconds for Java17 … (

#12084)
open-telemetry · Aug 28, 2024 · ab79ad4 · ab79ad4
1 parent 936ce57
commit ab79ad4
Show file tree

Hide file tree

Showing 13 changed files with 57 additions and 35 deletions.
diff --git a/.../main/java/io/opentelemetry/instrumentation/runtimemetrics/java17/internal/Constants.java b/.../main/java/io/opentelemetry/instrumentation/runtimemetrics/java17/internal/Constants.java
@@ -18,7 +18,7 @@ private Constants() {}
   public static final String ONE = "1";
   public static final String HERTZ = "Hz";
   public static final String BYTES = "By";
-  public static final String MILLISECONDS = "ms";
+  public static final String SECONDS = "s";
   public static final String COMMITTED = "committed";
   public static final String RESERVED = "reserved";
   public static final String INITIAL_SIZE = "initialSize";

diff --git a/...in/java/io/opentelemetry/instrumentation/runtimemetrics/java17/internal/DurationUtil.java b/...in/java/io/opentelemetry/instrumentation/runtimemetrics/java17/internal/DurationUtil.java
@@ -12,13 +12,12 @@
  * any time.
  */
 public final class DurationUtil {
-  private static final double NANOS_PER_MILLI = 1e6;
+  private static final double NANOS_PER_SECOND = 1e9;
 
-  /** Returns the duration as milliseconds, with fractional part included. */
-  @SuppressWarnings("TimeUnitMismatch")
-  public static double toMillis(Duration duration) {
+  /** Returns the duration as seconds, with fractional part included. */
+  public static double toSeconds(Duration duration) {
     double epochSecs = (double) duration.getSeconds();
-    return epochSecs * 1000 + duration.getNano() / NANOS_PER_MILLI;
+    return epochSecs + duration.getNano() / NANOS_PER_SECOND;
   }
 
   private DurationUtil() {}

diff --git a/.../io/opentelemetry/instrumentation/runtimemetrics/java17/internal/cpu/LongLockHandler.java b/.../io/opentelemetry/instrumentation/runtimemetrics/java17/internal/cpu/LongLockHandler.java
@@ -35,7 +35,7 @@ public LongLockHandler(Meter meter, ThreadGrouper grouper) {
         meter
             .histogramBuilder(METRIC_NAME)
             .setDescription(METRIC_DESCRIPTION)
-            .setUnit(Constants.MILLISECONDS)
+            .setUnit(Constants.SECONDS)
             .build();
   }
 
@@ -73,7 +73,7 @@ public PerThreadLongLockHandler(DoubleHistogram histogram, String threadName) {
     @Override
     public void accept(RecordedEvent recordedEvent) {
       if (recordedEvent.hasField(EVENT_THREAD)) {
-        histogram.record(DurationUtil.toMillis(recordedEvent.getDuration()), attributes);
+        histogram.record(DurationUtil.toSeconds(recordedEvent.getDuration()), attributes);
       }
       // What about the class name in MONITOR_CLASS ?
       // We can get a stack trace from the thread on the event

diff --git a/...entation/runtimemetrics/java17/internal/garbagecollection/G1GarbageCollectionHandler.java b/...entation/runtimemetrics/java17/internal/garbagecollection/G1GarbageCollectionHandler.java
@@ -6,7 +6,7 @@
 package io.opentelemetry.instrumentation.runtimemetrics.java17.internal.garbagecollection;
 
 import io.opentelemetry.api.common.Attributes;
-import io.opentelemetry.api.metrics.LongHistogram;
+import io.opentelemetry.api.metrics.DoubleHistogram;
 import io.opentelemetry.api.metrics.Meter;
 import io.opentelemetry.instrumentation.runtimemetrics.java17.JfrFeature;
 import io.opentelemetry.instrumentation.runtimemetrics.java17.internal.Constants;
@@ -27,21 +27,20 @@ public final class G1GarbageCollectionHandler implements RecordedEventHandler {
           "G1 Young Generation",
           Constants.ATTR_GC_ACTION,
           Constants.END_OF_MINOR_GC);
-  private final LongHistogram histogram;
+  private final DoubleHistogram histogram;
 
   public G1GarbageCollectionHandler(Meter meter) {
     histogram =
         meter
             .histogramBuilder(Constants.METRIC_NAME_GC_DURATION)
             .setDescription(Constants.METRIC_DESCRIPTION_GC_DURATION)
-            .setUnit(Constants.MILLISECONDS)
-            .ofLongs()
+            .setUnit(Constants.SECONDS)
             .build();
   }
 
   @Override
   public void accept(RecordedEvent ev) {
-    histogram.record(ev.getLong(Constants.DURATION), ATTR);
+    histogram.record(ev.getDouble(Constants.DURATION), ATTR);
   }
 
   @Override

diff --git a/...ntation/runtimemetrics/java17/internal/garbagecollection/OldGarbageCollectionHandler.java b/...ntation/runtimemetrics/java17/internal/garbagecollection/OldGarbageCollectionHandler.java
@@ -6,7 +6,7 @@
 package io.opentelemetry.instrumentation.runtimemetrics.java17.internal.garbagecollection;
 
 import io.opentelemetry.api.common.Attributes;
-import io.opentelemetry.api.metrics.LongHistogram;
+import io.opentelemetry.api.metrics.DoubleHistogram;
 import io.opentelemetry.api.metrics.Meter;
 import io.opentelemetry.instrumentation.runtimemetrics.java17.JfrFeature;
 import io.opentelemetry.instrumentation.runtimemetrics.java17.internal.Constants;
@@ -22,16 +22,15 @@
 public final class OldGarbageCollectionHandler implements RecordedEventHandler {
   private static final String EVENT_NAME = "jdk.OldGarbageCollection";
 
-  private final LongHistogram histogram;
+  private final DoubleHistogram histogram;
   private final Attributes attributes;
 
   public OldGarbageCollectionHandler(Meter meter, String gc) {
     histogram =
         meter
             .histogramBuilder(Constants.METRIC_NAME_GC_DURATION)
             .setDescription(Constants.METRIC_DESCRIPTION_GC_DURATION)
-            .setUnit(Constants.MILLISECONDS)
-            .ofLongs()
+            .setUnit(Constants.SECONDS)
             .build();
     // Set the attribute's GC based on which GC is being used.
     attributes =
@@ -41,7 +40,7 @@ public OldGarbageCollectionHandler(Meter meter, String gc) {
 
   @Override
   public void accept(RecordedEvent ev) {
-    histogram.record(ev.getLong(Constants.DURATION), attributes);
+    histogram.record(ev.getDouble(Constants.DURATION), attributes);
   }
 
   @Override

diff --git a/...ation/runtimemetrics/java17/internal/garbagecollection/YoungGarbageCollectionHandler.java b/...ation/runtimemetrics/java17/internal/garbagecollection/YoungGarbageCollectionHandler.java
@@ -6,7 +6,7 @@
 package io.opentelemetry.instrumentation.runtimemetrics.java17.internal.garbagecollection;
 
 import io.opentelemetry.api.common.Attributes;
-import io.opentelemetry.api.metrics.LongHistogram;
+import io.opentelemetry.api.metrics.DoubleHistogram;
 import io.opentelemetry.api.metrics.Meter;
 import io.opentelemetry.instrumentation.runtimemetrics.java17.JfrFeature;
 import io.opentelemetry.instrumentation.runtimemetrics.java17.internal.Constants;
@@ -22,16 +22,15 @@
 public final class YoungGarbageCollectionHandler implements RecordedEventHandler {
   private static final String EVENT_NAME = "jdk.YoungGarbageCollection";
 
-  private final LongHistogram histogram;
+  private final DoubleHistogram histogram;
   private final Attributes attributes;
 
   public YoungGarbageCollectionHandler(Meter meter, String gc) {
     histogram =
         meter
             .histogramBuilder(Constants.METRIC_NAME_GC_DURATION)
             .setDescription(Constants.METRIC_DESCRIPTION_GC_DURATION)
-            .setUnit(Constants.MILLISECONDS)
-            .ofLongs()
+            .setUnit(Constants.SECONDS)
             .build();
     // Set the attribute's GC based on which GC is being used.
     // G1 young collection is already handled by G1GarbageCollectionHandler.
@@ -42,7 +41,7 @@ public YoungGarbageCollectionHandler(Meter meter, String gc) {
 
   @Override
   public void accept(RecordedEvent ev) {
-    histogram.record(ev.getLong(Constants.DURATION), attributes);
+    histogram.record(ev.getDouble(Constants.DURATION), attributes);
   }
 
   @Override

diff --git a/...ntelemetry/instrumentation/runtimemetrics/java17/internal/network/NetworkReadHandler.java b/...ntelemetry/instrumentation/runtimemetrics/java17/internal/network/NetworkReadHandler.java
@@ -40,7 +40,7 @@ public NetworkReadHandler(Meter meter, ThreadGrouper nameNormalizer) {
         meter
             .histogramBuilder(Constants.METRIC_NAME_NETWORK_DURATION)
             .setDescription(Constants.METRIC_DESCRIPTION_NETWORK_DURATION)
-            .setUnit(Constants.MILLISECONDS)
+            .setUnit(Constants.SECONDS)
             .build();
   }
 
@@ -81,7 +81,7 @@ public PerThreadNetworkReadHandler(
     @Override
     public void accept(RecordedEvent ev) {
       bytesHistogram.record(ev.getLong(BYTES_READ), attributes);
-      durationHistogram.record(DurationUtil.toMillis(ev.getDuration()), attributes);
+      durationHistogram.record(DurationUtil.toSeconds(ev.getDuration()), attributes);
     }
   }
 }
diff --git a/...telemetry/instrumentation/runtimemetrics/java17/internal/network/NetworkWriteHandler.java b/...telemetry/instrumentation/runtimemetrics/java17/internal/network/NetworkWriteHandler.java
@@ -58,7 +58,7 @@ public NetworkWriteHandler(Meter meter, ThreadGrouper nameNormalizer) {
         meter
             .histogramBuilder(Constants.METRIC_NAME_NETWORK_DURATION)
             .setDescription(Constants.METRIC_DESCRIPTION_NETWORK_DURATION)
-            .setUnit(Constants.MILLISECONDS)
+            .setUnit(Constants.SECONDS)
             .build();
   }
 
@@ -99,7 +99,7 @@ private PerThreadNetworkWriteHandler(
     @Override
     public void accept(RecordedEvent ev) {
       bytesHistogram.record(ev.getLong(BYTES_WRITTEN), attributes);
-      durationHistogram.record(DurationUtil.toMillis(ev.getDuration()), attributes);
+      durationHistogram.record(DurationUtil.toSeconds(ev.getDuration()), attributes);
     }
   }
 }
diff --git a/...est/java/io/opentelemetry/instrumentation/runtimemetrics/java17/G1GcMemoryMetricTest.java b/...est/java/io/opentelemetry/instrumentation/runtimemetrics/java17/G1GcMemoryMetricTest.java
@@ -20,7 +20,7 @@
 import static io.opentelemetry.instrumentation.runtimemetrics.java17.internal.Constants.METRIC_NAME_GC_DURATION;
 import static io.opentelemetry.instrumentation.runtimemetrics.java17.internal.Constants.METRIC_NAME_MEMORY;
 import static io.opentelemetry.instrumentation.runtimemetrics.java17.internal.Constants.METRIC_NAME_MEMORY_AFTER;
-import static io.opentelemetry.instrumentation.runtimemetrics.java17.internal.Constants.MILLISECONDS;
+import static io.opentelemetry.instrumentation.runtimemetrics.java17.internal.Constants.SECONDS;
 import static org.assertj.core.api.Assertions.assertThat;
 
 import io.opentelemetry.api.common.Attributes;
@@ -88,7 +88,7 @@ void shouldHaveGcDurationMetrics() {
         metric ->
             metric
                 .hasName(METRIC_NAME_GC_DURATION)
-                .hasUnit(MILLISECONDS)
+                .hasUnit(SECONDS)
                 .hasDescription(METRIC_DESCRIPTION_GC_DURATION)
                 .satisfies(
                     data ->

diff --git a/.../src/test/java/io/opentelemetry/instrumentation/runtimemetrics/java17/JfrCpuLockTest.java b/.../src/test/java/io/opentelemetry/instrumentation/runtimemetrics/java17/JfrCpuLockTest.java
@@ -5,7 +5,7 @@
 
 package io.opentelemetry.instrumentation.runtimemetrics.java17;
 
-import static io.opentelemetry.instrumentation.runtimemetrics.java17.internal.Constants.MILLISECONDS;
+import static io.opentelemetry.instrumentation.runtimemetrics.java17.internal.Constants.SECONDS;
 import static org.assertj.core.api.Assertions.assertThat;
 
 import java.time.Duration;
@@ -51,7 +51,7 @@ void shouldHaveLockEvents() throws Exception {
         metric ->
             metric
                 .hasName("jvm.cpu.longlock")
-                .hasUnit(MILLISECONDS)
+                .hasUnit(SECONDS)
                 .hasHistogramSatisfying(histogram -> {}));
   }
 }
diff --git a/...est/java/io/opentelemetry/instrumentation/runtimemetrics/java17/PsGcMemoryMetricTest.java b/...est/java/io/opentelemetry/instrumentation/runtimemetrics/java17/PsGcMemoryMetricTest.java
@@ -23,7 +23,7 @@
 import static io.opentelemetry.instrumentation.runtimemetrics.java17.internal.Constants.METRIC_NAME_MEMORY;
 import static io.opentelemetry.instrumentation.runtimemetrics.java17.internal.Constants.METRIC_NAME_MEMORY_AFTER;
 import static io.opentelemetry.instrumentation.runtimemetrics.java17.internal.Constants.METRIC_NAME_MEMORY_LIMIT;
-import static io.opentelemetry.instrumentation.runtimemetrics.java17.internal.Constants.MILLISECONDS;
+import static io.opentelemetry.instrumentation.runtimemetrics.java17.internal.Constants.SECONDS;
 import static org.assertj.core.api.Assertions.assertThat;
 
 import io.opentelemetry.api.common.Attributes;
@@ -93,7 +93,7 @@ void shouldHaveGcDurationMetrics() {
         metric ->
             metric
                 .hasName(METRIC_NAME_GC_DURATION)
-                .hasUnit(MILLISECONDS)
+                .hasUnit(SECONDS)
                 .hasDescription(METRIC_DESCRIPTION_GC_DURATION)
                 .satisfies(
                     data ->

diff --git a/...java/io/opentelemetry/instrumentation/runtimemetrics/java17/SerialGcMemoryMetricTest.java b/...java/io/opentelemetry/instrumentation/runtimemetrics/java17/SerialGcMemoryMetricTest.java
@@ -11,7 +11,7 @@
 import static io.opentelemetry.instrumentation.runtimemetrics.java17.internal.Constants.END_OF_MINOR_GC;
 import static io.opentelemetry.instrumentation.runtimemetrics.java17.internal.Constants.METRIC_DESCRIPTION_GC_DURATION;
 import static io.opentelemetry.instrumentation.runtimemetrics.java17.internal.Constants.METRIC_NAME_GC_DURATION;
-import static io.opentelemetry.instrumentation.runtimemetrics.java17.internal.Constants.MILLISECONDS;
+import static io.opentelemetry.instrumentation.runtimemetrics.java17.internal.Constants.SECONDS;
 import static org.assertj.core.api.Assertions.assertThat;
 
 import io.opentelemetry.api.common.Attributes;
@@ -43,7 +43,7 @@ void shouldHaveGcDurationMetrics() {
         metric ->
             metric
                 .hasName(METRIC_NAME_GC_DURATION)
-                .hasUnit(MILLISECONDS)
+                .hasUnit(SECONDS)
                 .hasDescription(METRIC_DESCRIPTION_GC_DURATION)
                 .satisfies(
                     data ->

diff --git a/...ava/io/opentelemetry/instrumentation/runtimemetrics/java17/internal/DurationUtilTest.java b/...ava/io/opentelemetry/instrumentation/runtimemetrics/java17/internal/DurationUtilTest.java
@@ -0,0 +1,26 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.instrumentation.runtimemetrics.java17.internal;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import java.time.Duration;
+import org.junit.jupiter.api.Test;
+
+class DurationUtilTest {
+
+  @Test
+  void shouldConvertDurationToSeconds() {
+    // Given
+    Duration duration = Duration.ofSeconds(7, 144);
+
+    // When
+    double seconds = DurationUtil.toSeconds(duration);
+
+    // Then
+    assertEquals(7.000000144, seconds);
+  }
+}