Convert the ValueObserver instruments to use the LastValue aggegratio…

…n. (#1689) * Convert the ValueObserver instruments to use the LastValue aggegration. * update for rebase
open-telemetry · Oct 12, 2020 · ff73ae8 · ff73ae8
1 parent 487e57e
commit ff73ae8
Show file tree

Hide file tree

Showing 4 changed files with 21 additions and 40 deletions.
diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/ViewRegistry.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/ViewRegistry.java
@@ -63,8 +63,8 @@ private static Aggregation getRegisteredAggregation(InstrumentDescriptor descrip
       case UP_DOWN_COUNTER:
         return Aggregations.sum();
       case VALUE_RECORDER:
-      case VALUE_OBSERVER:
         return Aggregations.minMaxSumCount();
+      case VALUE_OBSERVER:
       case SUM_OBSERVER:
       case UP_DOWN_SUM_OBSERVER:
         return Aggregations.lastValue();

diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/Aggregations.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/Aggregations.java
@@ -222,13 +222,15 @@ public MetricData.Type getDescriptorType(
           return instrumentValueType == InstrumentValueType.LONG
               ? MetricData.Type.MONOTONIC_LONG
               : MetricData.Type.MONOTONIC_DOUBLE;
+        case VALUE_OBSERVER:
         case UP_DOWN_SUM_OBSERVER:
           return instrumentValueType == InstrumentValueType.LONG
               ? MetricData.Type.NON_MONOTONIC_LONG
               : MetricData.Type.NON_MONOTONIC_DOUBLE;
         default:
           // Do not change this unless the limitations of the current LastValueAggregator are fixed.
-          throw new IllegalArgumentException("Unsupported instrument/value types");
+          throw new IllegalArgumentException(
+              "Unsupported instrument/value types: " + instrumentType + "/" + instrumentValueType);
       }
     }
 

diff --git a/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/DoubleValueObserverSdkTest.java b/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/DoubleValueObserverSdkTest.java
@@ -13,16 +13,14 @@
 import io.opentelemetry.sdk.common.InstrumentationLibraryInfo;
 import io.opentelemetry.sdk.internal.TestClock;
 import io.opentelemetry.sdk.metrics.data.MetricData;
-import io.opentelemetry.sdk.metrics.data.MetricData.SummaryPoint;
-import io.opentelemetry.sdk.metrics.data.MetricData.ValueAtPercentile;
+import io.opentelemetry.sdk.metrics.data.MetricData.DoublePoint;
 import io.opentelemetry.sdk.resources.Resource;
-import java.util.Arrays;
 import java.util.Collections;
-import java.util.List;
 import org.junit.jupiter.api.Test;
 
 /** Unit tests for {@link DoubleValueObserverSdk}. */
 class DoubleValueObserverSdkTest {
+
   private static final long SECOND_NANOS = 1_000_000_000;
   private static final Resource RESOURCE =
       Resource.create(Attributes.of(stringKey("resource_key"), "resource_value"));
@@ -66,7 +64,7 @@ void collectMetrics_NoRecords() {
                 "testObserver",
                 "My own DoubleValueObserver",
                 "ms",
-                MetricData.Type.SUMMARY,
+                MetricData.Type.NON_MONOTONIC_DOUBLE,
                 Collections.emptyList()));
   }
 
@@ -84,15 +82,13 @@ void collectMetrics_WithOneRecord() {
                 "testObserver",
                 "",
                 "1",
-                MetricData.Type.SUMMARY,
+                MetricData.Type.NON_MONOTONIC_DOUBLE,
                 Collections.singletonList(
-                    SummaryPoint.create(
+                    DoublePoint.create(
                         testClock.now() - SECOND_NANOS,
                         testClock.now(),
                         Labels.of("k", "v"),
-                        1,
-                        12.1d,
-                        valueAtPercentiles(12.1d, 12.1d)))));
+                        12.1d))));
     testClock.advanceNanos(SECOND_NANOS);
     assertThat(doubleValueObserver.collectAll())
         .containsExactly(
@@ -102,18 +98,12 @@ void collectMetrics_WithOneRecord() {
                 "testObserver",
                 "",
                 "1",
-                MetricData.Type.SUMMARY,
+                MetricData.Type.NON_MONOTONIC_DOUBLE,
                 Collections.singletonList(
-                    SummaryPoint.create(
+                    DoublePoint.create(
                         testClock.now() - SECOND_NANOS,
                         testClock.now(),
                         Labels.of("k", "v"),
-                        1,
-                        12.1d,
-                        valueAtPercentiles(12.1d, 12.1d)))));
-  }
-
-  private static List<ValueAtPercentile> valueAtPercentiles(double min, double max) {
-    return Arrays.asList(ValueAtPercentile.create(0, min), ValueAtPercentile.create(100, max));
+                        12.1d))));
   }
 }
diff --git a/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/LongValueObserverSdkTest.java b/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/LongValueObserverSdkTest.java
@@ -13,12 +13,9 @@
 import io.opentelemetry.sdk.common.InstrumentationLibraryInfo;
 import io.opentelemetry.sdk.internal.TestClock;
 import io.opentelemetry.sdk.metrics.data.MetricData;
-import io.opentelemetry.sdk.metrics.data.MetricData.SummaryPoint;
-import io.opentelemetry.sdk.metrics.data.MetricData.ValueAtPercentile;
+import io.opentelemetry.sdk.metrics.data.MetricData.LongPoint;
 import io.opentelemetry.sdk.resources.Resource;
-import java.util.Arrays;
 import java.util.Collections;
-import java.util.List;
 import org.junit.jupiter.api.Test;
 
 /** Unit tests for {@link LongValueObserverSdk}. */
@@ -66,7 +63,7 @@ void collectMetrics_NoRecords() {
                 "testObserver",
                 "My own LongValueObserver",
                 "ms",
-                MetricData.Type.SUMMARY,
+                MetricData.Type.NON_MONOTONIC_LONG,
                 Collections.emptyList()));
   }
 
@@ -84,15 +81,13 @@ void collectMetrics_WithOneRecord() {
                 "testObserver",
                 "",
                 "1",
-                MetricData.Type.SUMMARY,
+                MetricData.Type.NON_MONOTONIC_LONG,
                 Collections.singletonList(
-                    SummaryPoint.create(
+                    LongPoint.create(
                         testClock.now() - SECOND_NANOS,
                         testClock.now(),
                         Labels.of("k", "v"),
-                        1,
-                        12,
-                        valueAtPercentiles(12, 12)))));
+                        12))));
     testClock.advanceNanos(SECOND_NANOS);
     assertThat(longValueObserver.collectAll())
         .containsExactly(
@@ -102,18 +97,12 @@ void collectMetrics_WithOneRecord() {
                 "testObserver",
                 "",
                 "1",
-                MetricData.Type.SUMMARY,
+                MetricData.Type.NON_MONOTONIC_LONG,
                 Collections.singletonList(
-                    SummaryPoint.create(
+                    LongPoint.create(
                         testClock.now() - SECOND_NANOS,
                         testClock.now(),
                         Labels.of("k", "v"),
-                        1,
-                        12,
-                        valueAtPercentiles(12, 12)))));
-  }
-
-  private static List<ValueAtPercentile> valueAtPercentiles(double min, double max) {
-    return Arrays.asList(ValueAtPercentile.create(0, min), ValueAtPercentile.create(100, max));
+                        12))));
   }
 }