From bc4f8f75a2ece3ad17e05a0b9a3bbbd1073316dc Mon Sep 17 00:00:00 2001
From: jack-berg <34418638+jack-berg@users.noreply.github.com>
Date: Wed, 17 Nov 2021 20:02:53 -0600
Subject: [PATCH] Make InstrumentSelect#getInstrumentType() nullable (#3893)

---
 .../sdk/metrics/view/InstrumentSelector.java  |  2 +
 .../internal/view/ViewRegistryTest.java       | 74 ++++++++++++-------
 2 files changed, 50 insertions(+), 26 deletions(-)

diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java
index 31c0de107c..cbe7d47d82 100644
--- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java
+++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/view/InstrumentSelector.java
@@ -11,6 +11,7 @@ import io.opentelemetry.sdk.metrics.internal.view.StringPredicates;
 import java.util.Objects;
 import java.util.function.Predicate;
 import java.util.regex.Pattern;
+import javax.annotation.Nullable;
 import javax.annotation.concurrent.Immutable;
 
 /**
@@ -36,6 +37,7 @@ public abstract class InstrumentSelector {
    * Returns {@link InstrumentType} that should be selected. If null, then this specifier will not
    * be used.
    */
+  @Nullable
   public abstract InstrumentType getInstrumentType();
 
   /**
diff --git a/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/internal/view/ViewRegistryTest.java b/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/internal/view/ViewRegistryTest.java
index e9be9a2ba0..c2c379018c 100644
--- a/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/internal/view/ViewRegistryTest.java
+++ b/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/internal/view/ViewRegistryTest.java
@@ -28,10 +28,7 @@ class ViewRegistryTest {
     ViewRegistry viewRegistry =
         ViewRegistry.builder()
             .addView(
-                InstrumentSelector.builder()
-                    .setInstrumentType(InstrumentType.COUNTER)
-                    .setInstrumentNameRegex(".*")
-                    .build(),
+                InstrumentSelector.builder().setInstrumentType(InstrumentType.COUNTER).build(),
                 view)
             .build();
     assertThat(
@@ -59,12 +56,7 @@ class ViewRegistryTest {
 
     ViewRegistry viewRegistry =
         ViewRegistry.builder()
-            .addView(
-                InstrumentSelector.builder()
-                    .setInstrumentType(InstrumentType.COUNTER)
-                    .setInstrumentNameRegex("overridden")
-                    .build(),
-                view)
+            .addView(InstrumentSelector.builder().setInstrumentName("overridden").build(), view)
             .build();
     assertThat(
             viewRegistry.findViews(
@@ -93,17 +85,8 @@ class ViewRegistryTest {
     ViewRegistry viewRegistry =
         ViewRegistry.builder()
             .addView(
-                InstrumentSelector.builder()
-                    .setInstrumentType(InstrumentType.COUNTER)
-                    .setInstrumentNameRegex("overridden")
-                    .build(),
-                view2)
-            .addView(
-                InstrumentSelector.builder()
-                    .setInstrumentType(InstrumentType.COUNTER)
-                    .setInstrumentNameRegex(".*")
-                    .build(),
-                view1)
+                InstrumentSelector.builder().setInstrumentNameRegex("overridden").build(), view2)
+            .addView(InstrumentSelector.builder().setInstrumentNameRegex(".*").build(), view1)
             .build();
 
     assertThat(
@@ -131,10 +114,7 @@ class ViewRegistryTest {
     ViewRegistry viewRegistry =
         ViewRegistry.builder()
             .addView(
-                InstrumentSelector.builder()
-                    .setInstrumentNameRegex("overrid(es|den)")
-                    .setInstrumentType(InstrumentType.COUNTER)
-                    .build(),
+                InstrumentSelector.builder().setInstrumentNameRegex("overrid(es|den)").build(),
                 view)
             .build();
 
@@ -149,7 +129,7 @@ class ViewRegistryTest {
     assertThat(
             viewRegistry.findViews(
                 InstrumentDescriptor.create(
-                    "overrides", "", "", InstrumentType.COUNTER, InstrumentValueType.LONG),
+                    "overrides", "", "", InstrumentType.UP_DOWN_COUNTER, InstrumentValueType.LONG),
                 INSTRUMENTATION_LIBRARY_INFO))
         .hasSize(1)
         .element(0)
@@ -165,6 +145,48 @@ class ViewRegistryTest {
         .isSameAs(ViewRegistry.DEFAULT_VIEW);
   }
 
+  @Test
+  void selection_typeAndName() {
+    View view = View.builder().setAggregation(Aggregation.lastValue()).build();
+
+    ViewRegistry viewRegistry =
+        ViewRegistry.builder()
+            .addView(
+                InstrumentSelector.builder()
+                    .setInstrumentType(InstrumentType.COUNTER)
+                    .setInstrumentName("overrides")
+                    .build(),
+                view)
+            .build();
+
+    assertThat(
+            viewRegistry.findViews(
+                InstrumentDescriptor.create(
+                    "overrides", "", "", InstrumentType.COUNTER, InstrumentValueType.LONG),
+                INSTRUMENTATION_LIBRARY_INFO))
+        .hasSize(1)
+        .element(0)
+        .isEqualTo(view);
+    // this one hasn't been configured, so it gets the default still..
+    assertThat(
+            viewRegistry.findViews(
+                InstrumentDescriptor.create(
+                    "overrides", "", "", InstrumentType.UP_DOWN_COUNTER, InstrumentValueType.LONG),
+                INSTRUMENTATION_LIBRARY_INFO))
+        .hasSize(1)
+        .element(0)
+        .isEqualTo(ViewRegistry.DEFAULT_VIEW);
+    // this one hasn't been configured, so it gets the default still..
+    assertThat(
+            viewRegistry.findViews(
+                InstrumentDescriptor.create(
+                    "default", "", "", InstrumentType.COUNTER, InstrumentValueType.LONG),
+                INSTRUMENTATION_LIBRARY_INFO))
+        .hasSize(1)
+        .element(0)
+        .isEqualTo(ViewRegistry.DEFAULT_VIEW);
+  }
+
   @Test
   void defaults() {
     ViewRegistry viewRegistry = ViewRegistry.builder().build();