Allow views to select on instrument unit (#5255)

buildSrc/src/main/kotlin/otel.japicmp-conventions.gradle.kts

@@ -1,5 +1,6 @@
 import com.google.auto.value.AutoValue
 import japicmp.model.JApiChangeStatus
+import japicmp.model.JApiClass
 import japicmp.model.JApiCompatibility
 import japicmp.model.JApiCompatibilityChange
 import japicmp.model.JApiMethod
@@ -34,12 +35,18 @@ val latestReleasedVersion: String by lazy {
 
 class AllowNewAbstractMethodOnAutovalueClasses : AbstractRecordingSeenMembers() {
   override fun maybeAddViolation(member: JApiCompatibility): Violation? {
-    if (member.compatibilityChanges == listOf(JApiCompatibilityChange.METHOD_ABSTRACT_ADDED_TO_CLASS) &&
+    val allowableAutovalueChanges = setOf(JApiCompatibilityChange.METHOD_ABSTRACT_ADDED_TO_CLASS, JApiCompatibilityChange.METHOD_ADDED_TO_PUBLIC_CLASS)
+    if (member.compatibilityChanges.filter { !allowableAutovalueChanges.contains(it) }.isEmpty() &&
       member is JApiMethod &&
       member.getjApiClass().newClass.get().getAnnotation(AutoValue::class.java) != null
     ) {
       return Violation.accept(member, "Autovalue will automatically add implementation")
     }
+    if (member.compatibilityChanges.isEmpty() &&
+      member is JApiClass &&
+      member.newClass.get().getAnnotation(AutoValue::class.java) != null) {
+      return Violation.accept(member, "Autovalue class modification is allowed")
+    }
     return null
   }
 }

docs/apidiffs/current_vs_latest/opentelemetry-sdk-metrics.txt

@@ -1,2 +1,8 @@
 Comparing source compatibility of  against 
-No changes.
+**** MODIFIED CLASS: PUBLIC ABSTRACT io.opentelemetry.sdk.metrics.InstrumentSelector  (not serializable)
+	===  CLASS FILE FORMAT VERSION: 52.0 <- 52.0
+	+++* NEW METHOD: PUBLIC(+) ABSTRACT(+) java.lang.String getInstrumentUnit()
+		+++  NEW ANNOTATION: javax.annotation.Nullable
+***  MODIFIED CLASS: PUBLIC FINAL io.opentelemetry.sdk.metrics.InstrumentSelectorBuilder  (not serializable)
+	===  CLASS FILE FORMAT VERSION: 52.0 <- 52.0
+	+++  NEW METHOD: PUBLIC(+) io.opentelemetry.sdk.metrics.InstrumentSelectorBuilder setUnit(java.lang.String)

sdk-extensions/incubator/README.md

@@ -14,6 +14,7 @@ For example, suppose `/Users/user123/view.yaml` has the following content:
 - selector:
     instrument_name: my-instrument
     instrument_type: COUNTER
+    instrument_unit: ms
     meter_name: my-meter
     meter_version: 1.0.0
     meter_schema_url: http://example.com
@@ -36,6 +37,7 @@ SdkMeterProvider.builder()
        InstrumentSelector.builder()
            .setName("my-instrument")
            .setType(InstrumentType.COUNTER)
+           .setUnit("ms")
            .setMeterName("my-meter")
            .setMeterVersion("1.0.0")
            .setMeterSchemaUrl("http://example.com")

sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/metric/viewconfig/SelectorSpecification.java

@@ -22,6 +22,9 @@ abstract class SelectorSpecification {
   @Nullable
   abstract InstrumentType getInstrumentType();
 
+  @Nullable
+  abstract String getInstrumentUnit();
+
   @Nullable
   abstract String getMeterName();
 
@@ -37,6 +40,8 @@ abstract class SelectorSpecification {
 
     Builder instrumentType(@Nullable InstrumentType instrumentType);
 
+    Builder instrumentUnit(@Nullable String instrumentUnit);
+
     Builder meterName(@Nullable String meterName);
 
     Builder meterVersion(@Nullable String meterVersion);

sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/metric/viewconfig/ViewConfig.java

@@ -38,6 +38,7 @@ import org.snakeyaml.engine.v2.api.LoadSettings;
  *   - selector:
  *       instrument_name: my-instrument
  *       instrument_type: COUNTER
+ *       instrument_unit: ms
  *       meter_name: my-meter
  *       meter_version: 1.0.0
  *       meter_schema_url: http://example.com
@@ -58,6 +59,7 @@ import org.snakeyaml.engine.v2.api.LoadSettings;
  *         InstrumentSelector.builder()
  *             .setName("my-instrument")
  *             .setType(InstrumentType.COUNTER)
+ *             .setUnit("ms")
  *             .setMeterName("my-meter")
  *             .setMeterVersion("1.0.0")
  *             .setMeterSchemaUrl("http://example.com")
@@ -125,6 +127,7 @@ public final class ViewConfig {
                     SelectorSpecification.builder()
                         .instrumentName(getAsType(selectorSpecMap, "instrument_name", String.class))
                         .instrumentType(instrumentType)
+                        .instrumentUnit(getAsType(selectorSpecMap, "instrument_unit", String.class))
                         .meterName(getAsType(selectorSpecMap, "meter_name", String.class))
                         .meterVersion(getAsType(selectorSpecMap, "meter_version", String.class))
                         .meterSchemaUrl(
@@ -248,6 +251,10 @@ public final class ViewConfig {
     if (instrumentType != null) {
       builder.setType(instrumentType);
     }
+    String instrumentUnit = selectorSpec.getInstrumentUnit();
+    if (instrumentUnit != null) {
+      builder.setUnit(instrumentUnit);
+    }
 
     String meterName = selectorSpec.getMeterName();
     if (meterName != null) {

sdk-extensions/incubator/src/test/java/io/opentelemetry/sdk/extension/incubator/metric/viewconfig/ViewConfigCustomizerTest.java

@@ -30,15 +30,14 @@ class ViewConfigCustomizerTest {
     AutoConfiguredOpenTelemetrySdk.builder()
         .setResultAsGlobal(false)
         .addPropertiesSupplier(
-            () -> {
+            () ->
-              return ImmutableMap.of(
+                ImmutableMap.of(
-                  "otel.traces.exporter",
+                    "otel.traces.exporter",
-                  "none",
+                    "none",
-                  "otel.metrics.exporter",
+                    "otel.metrics.exporter",
-                  "none",
+                    "none",
-                  "otel.experimental.metrics.view.config",
+                    "otel.experimental.metrics.view.config",
-                  "classpath:/view-config-customizer-test.yaml");
+                    "classpath:/view-config-customizer-test.yaml"))
-            })
         .addMeterProviderCustomizer(
             (meterProviderBuilder, configProperties) ->
                 meterProviderBuilder.registerMetricReader(reader))

sdk-extensions/incubator/src/test/java/io/opentelemetry/sdk/extension/incubator/metric/viewconfig/ViewConfigTest.java

@@ -59,6 +59,7 @@ class ViewConfigTest {
               SelectorSpecification selectorSpec = viewConfigSpec.getSelectorSpecification();
               assertThat(selectorSpec.getInstrumentName()).isEqualTo("name1");
               assertThat(selectorSpec.getInstrumentType()).isEqualTo(InstrumentType.COUNTER);
+              assertThat(selectorSpec.getInstrumentUnit()).isEqualTo("ms");
               assertThat(selectorSpec.getMeterName()).isEqualTo("meterName1");
               assertThat(selectorSpec.getMeterVersion()).isEqualTo("1.0.0");
               assertThat(selectorSpec.getMeterSchemaUrl()).isEqualTo("http://example1.com");
@@ -72,6 +73,7 @@ class ViewConfigTest {
               SelectorSpecification selectorSpec = viewConfigSpec.getSelectorSpecification();
               assertThat(selectorSpec.getInstrumentName()).isEqualTo("name2");
               assertThat(selectorSpec.getInstrumentType()).isEqualTo(InstrumentType.COUNTER);
+              assertThat(selectorSpec.getInstrumentUnit()).isEqualTo("s");
               assertThat(selectorSpec.getMeterName()).isEqualTo("meterName2");
               assertThat(selectorSpec.getMeterVersion()).isEqualTo("2.0.0");
               assertThat(selectorSpec.getMeterSchemaUrl()).isEqualTo("http://example2.com");
@@ -153,22 +155,22 @@ class ViewConfigTest {
         .extracting(
             "attributesProcessor", as(InstanceOfAssertFactories.type(AttributesProcessor.class)))
         .satisfies(
-            attributesProcessor -> {
+            attributesProcessor ->
-              assertThat(
+                assertThat(
-                      attributesProcessor.process(
+                        attributesProcessor.process(
-                          Attributes.builder()
+                            Attributes.builder()
-                              .put("foo", "val")
+                                .put("foo", "val")
-                              .put("bar", "val")
+                                .put("bar", "val")
-                              .put("baz", "val")
+                                .put("baz", "val")
-                              .build(),
+                                .build(),
-                          Context.current()))
+                            Context.current()))
-                  .containsEntry("foo", "val")
+                    .containsEntry("foo", "val")
-                  .containsEntry("bar", "val")
+                    .containsEntry("bar", "val")
-                  .satisfies(
+                    .satisfies(
-                      (Consumer<Attributes>)
+                        (Consumer<Attributes>)
-                          attributes ->
+                            attributes ->
-                              assertThat(attributes.get(AttributeKey.stringKey("baz"))).isBlank());
+                                assertThat(attributes.get(AttributeKey.stringKey("baz")))
-            });
+                                    .isBlank()));
   }
 
   @Test
@@ -242,6 +244,7 @@ class ViewConfigTest {
             SelectorSpecification.builder()
                 .instrumentName("name")
                 .instrumentType(InstrumentType.COUNTER)
+                .instrumentUnit("ms")
                 .meterName("meterName")
                 .meterVersion("meterVersion")
                 .meterSchemaUrl("http://example.com")
@@ -249,6 +252,7 @@ class ViewConfigTest {
 
     assertThat(selector.getInstrumentName()).isEqualTo("name");
     assertThat(selector.getInstrumentType()).isEqualTo(InstrumentType.COUNTER);
+    assertThat(selector.getInstrumentUnit()).isEqualTo("ms");
     assertThat(selector.getMeterName()).isEqualTo("meterName");
     assertThat(selector.getMeterVersion()).isEqualTo("meterVersion");
     assertThat(selector.getMeterSchemaUrl()).isEqualTo("http://example.com");

sdk-extensions/incubator/src/test/resources/empty-view-config.yaml

@@ -1,6 +1,7 @@
 - selector:
     instrument_name: name1
     instrument_type: COUNTER
+    instrument_unit: ms
     meter_name: meterName1
     meter_version: 1.0.0
     meter_schema_url: http://example1.com

sdk-extensions/incubator/src/test/resources/full-config.yaml

@@ -1,6 +1,7 @@
 - selector:
     instrument_name: name1
     instrument_type: COUNTER
+    instrument_unit: ms
     meter_name: meterName1
     meter_version: 1.0.0
     meter_schema_url: http://example1.com
@@ -14,6 +15,7 @@
 - selector:
     instrument_name: name2
     instrument_type: COUNTER
+    instrument_unit: s
     meter_name: meterName2
     meter_version: 2.0.0
     meter_schema_url: http://example2.com

sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/InstrumentSelector.java

@@ -32,11 +32,12 @@ public abstract class InstrumentSelector {
   static InstrumentSelector create(
       @Nullable InstrumentType instrumentType,
       @Nullable String instrumentName,
+      @Nullable String instrumentUnit,
       @Nullable String meterName,
       @Nullable String meterVersion,
       @Nullable String meterSchemaUrl) {
     return new AutoValue_InstrumentSelector(
-        instrumentType, instrumentName, meterName, meterVersion, meterSchemaUrl);
+        instrumentType, instrumentName, instrumentUnit, meterName, meterVersion, meterSchemaUrl);
   }
 
   InstrumentSelector() {}
@@ -59,6 +60,10 @@ public abstract class InstrumentSelector {
   @Nullable
   public abstract String getInstrumentName();
 
+  /** Returns the selected instrument unit, or null if this selects all instrument units. */
+  @Nullable
+  public abstract String getInstrumentUnit();
+
   /** Returns the selected meter name, or null if this selects instruments from all meter names. */
   @Nullable
   public abstract String getMeterName();
@@ -86,6 +91,9 @@ public abstract class InstrumentSelector {
     if (getInstrumentName() != null) {
       joiner.add("instrumentName=" + getInstrumentName());
     }
+    if (getInstrumentUnit() != null) {
+      joiner.add("instrumentUnit=" + getInstrumentUnit());
+    }
     if (getMeterName() != null) {
       joiner.add("meterName=" + getMeterName());
     }

sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/InstrumentSelectorBuilder.java

@@ -19,6 +19,7 @@ public final class InstrumentSelectorBuilder {
 
   @Nullable private InstrumentType instrumentType;
   @Nullable private String instrumentName;
+  @Nullable private String instrumentUnit;
   @Nullable private String meterName;
   @Nullable private String meterVersion;
   @Nullable private String meterSchemaUrl;
@@ -49,6 +50,13 @@ public final class InstrumentSelectorBuilder {
     return this;
   }
 
+  /** Select instruments with the given {@code unit}. */
+  public InstrumentSelectorBuilder setUnit(String unit) {
+    requireNonNull(unit, "unit");
+    this.instrumentUnit = unit;
+    return this;
+  }
+
   /** Select instruments associated with the given {@code meterName}. */
   public InstrumentSelectorBuilder setMeterName(String meterName) {
     requireNonNull(meterName, "meterName");
@@ -75,11 +83,12 @@ public final class InstrumentSelectorBuilder {
     checkArgument(
         instrumentType != null
             || instrumentName != null
+            || instrumentUnit != null
             || meterName != null
             || meterVersion != null
             || meterSchemaUrl != null,
         "Instrument selector must contain selection criteria");
     return InstrumentSelector.create(
-        instrumentType, instrumentName, meterName, meterVersion, meterSchemaUrl);
+        instrumentType, instrumentName, instrumentUnit, meterName, meterVersion, meterSchemaUrl);
   }
 }

sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/view/ViewRegistry.java

@@ -147,6 +147,10 @@ public final class ViewRegistry {
         && selector.getInstrumentType() != descriptor.getType()) {
       return false;
     }
+    if (selector.getInstrumentUnit() != null
+        && !selector.getInstrumentUnit().equals(descriptor.getUnit())) {
+      return false;
+    }
     if (selector.getInstrumentName() != null
         && !toGlobPatternPredicate(selector.getInstrumentName()).test(descriptor.getName())) {
       return false;

sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/InstrumentSelectorTest.java

@@ -20,6 +20,9 @@ class InstrumentSelectorTest {
     assertThatThrownBy(() -> InstrumentSelector.builder().setName(null))
         .isInstanceOf(NullPointerException.class)
         .hasMessage("name");
+    assertThatThrownBy(() -> InstrumentSelector.builder().setUnit(null))
+        .isInstanceOf(NullPointerException.class)
+        .hasMessage("unit");
     assertThatThrownBy(() -> InstrumentSelector.builder().setMeterName(null))
         .isInstanceOf(NullPointerException.class)
         .hasMessage("meterName");
@@ -51,6 +54,12 @@ class InstrumentSelectorTest {
     assertThat(selector.getInstrumentName()).isEqualTo("bar");
   }
 
+  @Test
+  void instrumentUnit() {
+    InstrumentSelector selector = InstrumentSelector.builder().setName("ms").setName("s").build();
+    assertThat(selector.getInstrumentName()).isEqualTo("s");
+  }
+
   @Test
   void meterName() {
     InstrumentSelector selector =
@@ -83,6 +92,7 @@ class InstrumentSelectorTest {
             InstrumentSelector.builder()
                 .setType(InstrumentType.COUNTER)
                 .setName("name")
+                .setUnit("unit")
                 .setMeterName("meter")
                 .setMeterVersion("version")
                 .setMeterSchemaUrl("http://url.com")
@@ -92,6 +102,7 @@ class InstrumentSelectorTest {
             "InstrumentSelector{"
                 + "instrumentType=COUNTER, "
                 + "instrumentName=name, "
+                + "instrumentUnit=unit, "
                 + "meterName=meter, "
                 + "meterVersion=version, "
                 + "meterSchemaUrl=http://url.com"

sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/internal/view/ViewRegistryTest.java

@@ -56,7 +56,34 @@ class ViewRegistryTest {
                     "", "", "", InstrumentType.COUNTER, InstrumentValueType.LONG),
                 INSTRUMENTATION_SCOPE_INFO))
         .isEqualTo(Collections.singletonList(registeredView));
-    // this one hasn't been configured, so it gets the default still.
+    // this one doesn't match, so it gets the default still.
+    assertThat(
+            viewRegistry.findViews(
+                InstrumentDescriptor.create(
+                    "", "", "", InstrumentType.UP_DOWN_COUNTER, InstrumentValueType.LONG),
+                INSTRUMENTATION_SCOPE_INFO))
+        .isEqualTo(Collections.singletonList(DEFAULT_REGISTERED_VIEW));
+
+    assertThat(logs.getEvents()).hasSize(0);
+  }
+
+  @Test
+  void findViews_SelectionOnUnit() {
+    RegisteredView registeredView =
+        registeredView(
+            InstrumentSelector.builder().setUnit("ms").build(),
+            View.builder().setDescription("description").build());
+    ViewRegistry viewRegistry =
+        ViewRegistry.create(
+            DefaultAggregationSelector.getDefault(), Collections.singletonList(registeredView));
+
+    assertThat(
+            viewRegistry.findViews(
+                InstrumentDescriptor.create(
+                    "", "", "ms", InstrumentType.COUNTER, InstrumentValueType.LONG),
+                INSTRUMENTATION_SCOPE_INFO))
+        .isEqualTo(Collections.singletonList(registeredView));
+    // this one doesn't match, so it gets the default still.
     assertThat(
             viewRegistry.findViews(
                 InstrumentDescriptor.create(
@@ -83,7 +110,7 @@ class ViewRegistryTest {
                     "overridden", "", "", InstrumentType.COUNTER, InstrumentValueType.LONG),
                 INSTRUMENTATION_SCOPE_INFO))
         .isEqualTo(Collections.singletonList(registeredView));
-    // this one hasn't been configured, so it gets the default still.
+    // this one doesn't match, so it gets the default still.
     assertThat(
             viewRegistry.findViews(
                 InstrumentDescriptor.create(
@@ -145,14 +172,14 @@ class ViewRegistryTest {
                     "overrides", "", "", InstrumentType.COUNTER, InstrumentValueType.LONG),
                 INSTRUMENTATION_SCOPE_INFO))
         .isEqualTo(Collections.singletonList(registeredView));
-    // this one hasn't been configured, so it gets the default still..
+    // this one doesn't match, so it gets the default still.
     assertThat(
             viewRegistry.findViews(
                 InstrumentDescriptor.create(
                     "overrides", "", "", InstrumentType.UP_DOWN_COUNTER, InstrumentValueType.LONG),
                 INSTRUMENTATION_SCOPE_INFO))
         .isEqualTo(Collections.singletonList(DEFAULT_REGISTERED_VIEW));
-    // this one hasn't been configured, so it gets the default still..
+    // this one doesn't match, so it gets the default still.
     assertThat(
             viewRegistry.findViews(
                 InstrumentDescriptor.create(