Log warning when duplicate async instruments are registered

sdk/metrics/src/debugEnabledTest/java/io/opentelemetry/testing/TestSourceInfo.java

@@ -38,7 +38,7 @@ void testDuplicateExceptionMessage_pureInstruments() {
                 "name",
                 "description",
                 "unit",
-                InstrumentType.HISTOGRAM,
+                InstrumentType.OBSERVABLE_SUM,
                 InstrumentValueType.DOUBLE));
     MetricDescriptor simpleWithNewDescription =
         MetricDescriptor.create(
@@ -49,11 +49,11 @@ void testDuplicateExceptionMessage_pureInstruments() {
         .contains("Found duplicate metric definition: name")
         .contains("- Unit [unit2] does not match [unit]")
         .contains("- Description [description2] does not match [description]")
-        .contains("- InstrumentType [COUNTER] does not match [HISTOGRAM]")
+        .contains("- InstrumentType [COUNTER] does not match [OBSERVABLE_SUM]")
         .contains("- InstrumentValueType [LONG] does not match [DOUBLE]")
+        .contains("- InstrumentType [OBSERVABLE_SUM] is async and already registered")
         .contains(simple.getSourceInstrument().getSourceInfo().multiLineDebugString())
-        .contains(
-            "Original instrument registered with same name but different description, unit, instrument type, or instrument value type.")
+        .contains("Original instrument registered with same name but is incompatible.")
         .contains(
             simpleWithNewDescription.getSourceInstrument().getSourceInfo().multiLineDebugString());
   }
@@ -118,8 +118,7 @@ void testDuplicateExceptionMessage_viewBasedConflict2() {
         .contains("FROM instrument name2")
         .contains(simple.getSourceInstrument().getSourceInfo().multiLineDebugString())
         .contains("- Unit [unit] does not match [unit2]")
-        .contains(
-            "Original instrument registered with same name but different description, unit, instrument type, or instrument value type.")
+        .contains("Original instrument registered with same name but is incompatible.")
         .contains(
             simpleWithNewDescription.getSourceInstrument().getSourceInfo().multiLineDebugString());
   }

sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/descriptor/MetricDescriptor.java

@@ -86,4 +86,20 @@ public boolean isCompatibleWith(MetricDescriptor other) {
         && Objects.equals(
             getSourceInstrument().getValueType(), other.getSourceInstrument().getValueType());
   }
+
+  /** Returns whether the descriptor describes an async {@link InstrumentType}. */
+  public boolean isAsync() {
+    switch (getSourceInstrument().getType()) {
+      case OBSERVABLE_UP_DOWN_SUM:
+      case OBSERVABLE_GAUGE:
+      case OBSERVABLE_SUM:
+        return true;
+      case HISTOGRAM:
+      case COUNTER:
+      case UP_DOWN_COUNTER:
+        return false;
+    }
+    throw new IllegalStateException(
+        "Unrecognized instrument type " + getSourceInstrument().getType());
+  }
 }

sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/state/DebugUtils.java

@@ -96,13 +96,18 @@ public static String duplicateMetricErrorMessage(
           .append(existing.getSourceInstrument().getValueType())
           .append("]\n");
     }
+    if (existing.isAsync()) {
+      result
+          .append("- InstrumentType [")
+          .append(existing.getSourceInstrument().getType())
+          .append("] is async and already registered\n");
+    }
 
     // Next we write out where the existing metric descriptor came from, either a raw instrument
     // or a view on a raw instrument.
     if (existing.getName().equals(existing.getSourceInstrument().getName())) {
       result
-          .append(
-              "Original instrument registered with same name but different description, unit, instrument type, or instrument value type.\n")
+          .append("Original instrument registered with same name but is incompatible.\n")
           .append(existing.getSourceInstrument().getSourceInfo().multiLineDebugString())
           .append("\n");
     } else {

sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/state/MetricStorageRegistry.java

@@ -58,21 +58,25 @@ public <I extends MetricStorage> I register(I storage) {
     MetricDescriptor descriptor = storage.getMetricDescriptor();
     MetricStorage oldOrNewStorage =
         registry.computeIfAbsent(descriptor.getName().toLowerCase(), key -> storage);
+    // Metric didn't already exist in registry, return.
+    if (storage == oldOrNewStorage) {
+      return (I) oldOrNewStorage;
+    }
     // Make sure the storage is compatible.
     if (!oldOrNewStorage.getMetricDescriptor().isCompatibleWith(descriptor)) {
       throw new DuplicateMetricStorageException(
           oldOrNewStorage.getMetricDescriptor(),
           descriptor,
           "Metric with same name and different descriptor already created.");
     }
-    // Make sure we aren't mixing sync + async.
-    if (!storage.getClass().equals(oldOrNewStorage.getClass())) {
+    // Descriptors are compatible, but can't register async instruments multiple times.
+    if (descriptor.isAsync()) {
       throw new DuplicateMetricStorageException(
           oldOrNewStorage.getMetricDescriptor(),
           descriptor,
-          "Metric with same name and different instrument already created.");
+          "Async metric with same name has already been created.");
     }
-
+    // Metric already existed, and is compatible with new storage.
     return (I) oldOrNewStorage;
   }
 }

sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/internal/descriptor/MetricDescriptorTest.java

@@ -123,4 +123,21 @@ void metricDescriptor_isCompatible() {
                         InstrumentValueType.LONG))))
         .isFalse();
   }
+
+  @Test
+  void isAsync() {
+    assertThat(descriptorForInstrument(InstrumentType.OBSERVABLE_UP_DOWN_SUM).isAsync()).isTrue();
+    assertThat(descriptorForInstrument(InstrumentType.OBSERVABLE_GAUGE).isAsync()).isTrue();
+    assertThat(descriptorForInstrument(InstrumentType.OBSERVABLE_SUM).isAsync()).isTrue();
+    assertThat(descriptorForInstrument(InstrumentType.HISTOGRAM).isAsync()).isFalse();
+    assertThat(descriptorForInstrument(InstrumentType.COUNTER).isAsync()).isFalse();
+    assertThat(descriptorForInstrument(InstrumentType.UP_DOWN_COUNTER).isAsync()).isFalse();
+  }
+
+  private static MetricDescriptor descriptorForInstrument(InstrumentType instrumentType) {
+    InstrumentDescriptor instrument =
+        InstrumentDescriptor.create(
+            "name", "description", "unit", instrumentType, InstrumentValueType.DOUBLE);
+    return MetricDescriptor.create(View.builder().build(), instrument);
+  }
 }

sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/internal/state/MetricStorageRegistryTest.java

@@ -10,100 +10,84 @@
 
 import io.opentelemetry.api.common.Attributes;
 import io.opentelemetry.sdk.common.InstrumentationLibraryInfo;
+import io.opentelemetry.sdk.metrics.common.InstrumentType;
+import io.opentelemetry.sdk.metrics.common.InstrumentValueType;
 import io.opentelemetry.sdk.metrics.data.MetricData;
+import io.opentelemetry.sdk.metrics.internal.descriptor.InstrumentDescriptor;
 import io.opentelemetry.sdk.metrics.internal.descriptor.MetricDescriptor;
 import io.opentelemetry.sdk.metrics.internal.export.CollectionInfo;
+import io.opentelemetry.sdk.metrics.view.View;
 import io.opentelemetry.sdk.resources.Resource;
 import org.junit.jupiter.api.Test;
 
 /** Unit tests for {@link MetricStorageRegistry}. */
 class MetricStorageRegistryTest {
-  private static final MetricDescriptor METRIC_DESCRIPTOR =
-      MetricDescriptor.create("name", "description", "1");
-  private static final MetricDescriptor OTHER_METRIC_DESCRIPTOR =
-      MetricDescriptor.create("name", "other_description", "1");
+  private static final MetricDescriptor SYNC_DESCRIPTOR =
+      descriptor("sync", "description", InstrumentType.COUNTER);
+  private static final MetricDescriptor OTHER_SYNC_DESCRIPTOR =
+      descriptor("sync", "other_description", InstrumentType.COUNTER);
+  private static final MetricDescriptor ASYNC_DESCRIPTOR =
+      descriptor("async", "description", InstrumentType.OBSERVABLE_GAUGE);
+
+  private final MeterSharedState meterSharedState =
+      MeterSharedState.create(InstrumentationLibraryInfo.empty());
 
   @Test
-  void register() {
-    MeterSharedState meterSharedState = MeterSharedState.create(InstrumentationLibraryInfo.empty());
-    TestMetricStorage testInstrument = new TestMetricStorage(METRIC_DESCRIPTOR);
+  void register_Sync() {
+    TestMetricStorage testInstrument = new TestMetricStorage(SYNC_DESCRIPTOR);
     assertThat(meterSharedState.getMetricStorageRegistry().register(testInstrument))
         .isSameAs(testInstrument);
     assertThat(meterSharedState.getMetricStorageRegistry().register(testInstrument))
         .isSameAs(testInstrument);
     assertThat(
             meterSharedState
                 .getMetricStorageRegistry()
-                .register(new TestMetricStorage(METRIC_DESCRIPTOR)))
+                .register(new TestMetricStorage(SYNC_DESCRIPTOR)))
         .isSameAs(testInstrument);
   }
 
   @Test
-  void register_OtherDescriptor() {
-    MeterSharedState meterSharedState = MeterSharedState.create(InstrumentationLibraryInfo.empty());
-    TestMetricStorage testInstrument = new TestMetricStorage(METRIC_DESCRIPTOR);
+  void register_SyncIncompatibleDescriptor() {
+    TestMetricStorage testInstrument = new TestMetricStorage(SYNC_DESCRIPTOR);
     assertThat(meterSharedState.getMetricStorageRegistry().register(testInstrument))
         .isSameAs(testInstrument);
 
     assertThatThrownBy(
             () ->
                 meterSharedState
                     .getMetricStorageRegistry()
-                    .register(new TestMetricStorage(OTHER_METRIC_DESCRIPTOR)))
+                    .register(new TestMetricStorage(OTHER_SYNC_DESCRIPTOR)))
         .isInstanceOf(IllegalArgumentException.class)
         .hasMessageContaining("Metric with same name and different descriptor already created.");
   }
 
   @Test
-  void register_OtherInstance() {
-    MeterSharedState meterSharedState = MeterSharedState.create(InstrumentationLibraryInfo.empty());
-    TestMetricStorage testInstrument = new TestMetricStorage(METRIC_DESCRIPTOR);
+  void register_Async() {
+    TestMetricStorage testInstrument = new TestMetricStorage(ASYNC_DESCRIPTOR);
     assertThat(meterSharedState.getMetricStorageRegistry().register(testInstrument))
         .isSameAs(testInstrument);
 
     assertThatThrownBy(
             () ->
                 meterSharedState
                     .getMetricStorageRegistry()
-                    .register(new OtherTestMetricStorage(METRIC_DESCRIPTOR)))
+                    .register(new TestMetricStorage(ASYNC_DESCRIPTOR)))
         .isInstanceOf(IllegalArgumentException.class)
-        .hasMessageContaining("Metric with same name and different instrument already created.");
+        .hasMessageContaining("Async metric with same name has already been created.");
   }
 
-  private static final class TestMetricStorage implements MetricStorage, WriteableMetricStorage {
-    private final MetricDescriptor descriptor;
-
-    TestMetricStorage(MetricDescriptor descriptor) {
-      this.descriptor = descriptor;
-    }
-
-    @Override
-    public MetricDescriptor getMetricDescriptor() {
-      return descriptor;
-    }
-
-    @Override
-    public MetricData collectAndReset(
-        CollectionInfo collectionInfo,
-        Resource resource,
-        InstrumentationLibraryInfo instrumentationLibraryInfo,
-        long startEpochNanos,
-        long epochNanos,
-        boolean suppressSynchronousCollection) {
-      return null;
-    }
-
-    @Override
-    public BoundStorageHandle bind(Attributes attributes) {
-      return null;
-    }
+  private static MetricDescriptor descriptor(
+      String name, String description, InstrumentType instrumentType) {
+    return MetricDescriptor.create(
+        View.builder().build(),
+        InstrumentDescriptor.create(
+            name, description, "1", instrumentType, InstrumentValueType.DOUBLE));
   }
 
-  private static final class OtherTestMetricStorage
-      implements MetricStorage, WriteableMetricStorage {
+  private static final class TestMetricStorage implements MetricStorage, WriteableMetricStorage {
     private final MetricDescriptor descriptor;
 
-    OtherTestMetricStorage(MetricDescriptor descriptor) {
+    TestMetricStorage(MetricDescriptor descriptor) {
       this.descriptor = descriptor;
     }