Stop validating instrument unit (#5279)

api/all/src/main/java/io/opentelemetry/api/internal/ApiUsageLogger.java

@@ -16,12 +16,10 @@ import java.util.logging.Logger;
  */
 public final class ApiUsageLogger {
 
-  public static final String LOGGER_NAME = "io.opentelemetry.ApiUsageLogger";
+  private static final Logger API_USAGE_LOGGER = Logger.getLogger(ApiUsageLogger.class.getName());
-
-  private static final Logger API_USAGE_LOGGER = Logger.getLogger(LOGGER_NAME);
 
   /**
-   * Log the {@code message} to the {@link #LOGGER_NAME API Usage Logger}.
+   * Log the {@code message} to the {@link #API_USAGE_LOGGER API Usage Logger}.
    *
    * <p>Log at {@link Level#FINEST} and include a stack trace.
    */
@@ -30,7 +28,7 @@ public final class ApiUsageLogger {
   }
 
   /**
-   * Log the {@code message} to the {@link #LOGGER_NAME API Usage Logger}.
+   * Log the {@code message} to the {@link #API_USAGE_LOGGER API Usage Logger}.
    *
    * <p>Log includes a stack trace.
    */

api/all/src/test/java/io/opentelemetry/api/internal/ApiUsageLoggerTest.java

@@ -8,13 +8,15 @@ package io.opentelemetry.api.internal;
 import static java.util.logging.Level.WARNING;
 
 import io.github.netmikey.logunit.api.LogCapturer;
+import io.opentelemetry.internal.testing.slf4j.SuppressLogger;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.RegisterExtension;
 
+@SuppressLogger(ApiUsageLogger.class)
 class ApiUsageLoggerTest {
 
   @RegisterExtension
-  LogCapturer apiUsageLogs = LogCapturer.create().captureForLogger(ApiUsageLogger.LOGGER_NAME);
+  LogCapturer apiUsageLogs = LogCapturer.create().captureForLogger(ApiUsageLogger.class.getName());
 
   @Test
   void log() {

api/all/src/test/java/io/opentelemetry/api/metrics/DefaultMeterTest.java

@@ -6,21 +6,16 @@
 package io.opentelemetry.api.metrics;
 
 import static io.opentelemetry.api.common.AttributeKey.stringKey;
-import static io.opentelemetry.api.internal.ApiUsageLogger.LOGGER_NAME;
 
-import io.github.netmikey.logunit.api.LogCapturer;
 import io.opentelemetry.api.common.Attributes;
 import io.opentelemetry.context.Context;
 import io.opentelemetry.internal.testing.slf4j.SuppressLogger;
 import org.junit.jupiter.api.Test;
-import org.junit.jupiter.api.extension.RegisterExtension;
 
-@SuppressLogger(loggerName = LOGGER_NAME)
+@SuppressLogger()
 public class DefaultMeterTest {
   private static final Meter METER = DefaultMeter.getInstance();
 
-  @RegisterExtension LogCapturer apiUsageLogs = LogCapturer.create().captureForLogger(LOGGER_NAME);
-
   @Test
   void noopLongCounter_doesNotThrow() {
     LongCounter counter =

exporters/common/src/test/java/io/opentelemetry/exporter/internal/TlsConfigHelperTest.java

@@ -16,6 +16,7 @@ import static org.mockito.Mockito.when;
 
 import com.linecorp.armeria.testing.junit5.server.SelfSignedCertificateExtension;
 import io.github.netmikey.logunit.api.LogCapturer;
+import io.opentelemetry.internal.testing.slf4j.SuppressLogger;
 import java.util.concurrent.atomic.AtomicReference;
 import javax.net.ssl.SSLException;
 import javax.net.ssl.SSLSocketFactory;
@@ -28,6 +29,7 @@ import org.mockito.Mock;
 import org.mockito.junit.jupiter.MockitoExtension;
 
 @ExtendWith(MockitoExtension.class)
+@SuppressLogger(TlsConfigHelper.class)
 class TlsConfigHelperTest {
   @RegisterExtension
   static final SelfSignedCertificateExtension serverTls = new SelfSignedCertificateExtension();

sdk/logs/src/test/java/io/opentelemetry/sdk/logs/SdkLoggerTest.java

@@ -9,7 +9,6 @@ import static io.opentelemetry.api.common.AttributeKey.booleanArrayKey;
 import static io.opentelemetry.api.common.AttributeKey.doubleArrayKey;
 import static io.opentelemetry.api.common.AttributeKey.longArrayKey;
 import static io.opentelemetry.api.common.AttributeKey.stringArrayKey;
-import static io.opentelemetry.api.internal.ApiUsageLogger.LOGGER_NAME;
 import static io.opentelemetry.sdk.testing.assertj.LogAssertions.assertThat;
 import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.assertThat;
 import static org.mockito.ArgumentMatchers.any;
@@ -18,7 +17,6 @@ import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
-import io.github.netmikey.logunit.api.LogCapturer;
 import io.opentelemetry.api.common.AttributeKey;
 import io.opentelemetry.api.common.Attributes;
 import io.opentelemetry.api.common.AttributesBuilder;
@@ -32,12 +30,9 @@ import java.util.Arrays;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicReference;
 import org.junit.jupiter.api.Test;
-import org.junit.jupiter.api.extension.RegisterExtension;
 
 class SdkLoggerTest {
 
-  @RegisterExtension LogCapturer apiUsageLogs = LogCapturer.create().captureForLogger(LOGGER_NAME);
-
   @Test
   void logRecordBuilder() {
     LoggerSharedState state = mock(LoggerSharedState.class);

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

@@ -5,8 +5,6 @@
 
 package io.opentelemetry.sdk.metrics;
 
-import static java.util.logging.Level.WARNING;
-
 import io.opentelemetry.api.metrics.ObservableDoubleMeasurement;
 import io.opentelemetry.api.metrics.ObservableLongMeasurement;
 import io.opentelemetry.sdk.metrics.internal.descriptor.InstrumentDescriptor;
@@ -15,18 +13,14 @@ import io.opentelemetry.sdk.metrics.internal.state.MeterProviderSharedState;
 import io.opentelemetry.sdk.metrics.internal.state.MeterSharedState;
 import io.opentelemetry.sdk.metrics.internal.state.SdkObservableMeasurement;
 import io.opentelemetry.sdk.metrics.internal.state.WriteableMetricStorage;
-import java.nio.charset.StandardCharsets;
 import java.util.Collections;
 import java.util.function.BiFunction;
 import java.util.function.Consumer;
-import java.util.logging.Logger;
 
 /** Helper to make implementing builders easier. */
 abstract class AbstractInstrumentBuilder<BuilderT extends AbstractInstrumentBuilder<?>> {
 
   static final String DEFAULT_UNIT = "";
-  public static final String LOGGER_NAME = "io.opentelemetry.sdk.metrics.AbstractInstrumentBuilder";
-  private static final Logger LOGGER = Logger.getLogger(LOGGER_NAME);
 
   private final MeterProviderSharedState meterProviderSharedState;
   private final InstrumentType type;
@@ -57,13 +51,7 @@ abstract class AbstractInstrumentBuilder<BuilderT extends AbstractInstrumentBuil
   protected abstract BuilderT getThis();
 
   public BuilderT setUnit(String unit) {
-    if (!checkValidInstrumentUnit(
+    this.unit = unit;
-        unit,
-        " Using \"" + DEFAULT_UNIT + "\" for instrument " + this.instrumentName + " instead.")) {
-      this.unit = DEFAULT_UNIT;
-    } else {
-      this.unit = unit;
-    }
     return getThis();
   }
 
@@ -120,34 +108,6 @@ abstract class AbstractInstrumentBuilder<BuilderT extends AbstractInstrumentBuil
         + "}";
   }
 
-  /** Check if the instrument unit is valid. If invalid, log a warning. */
-  static boolean checkValidInstrumentUnit(String unit) {
-    return checkValidInstrumentUnit(unit, "");
-  }
-
-  /**
-   * Check if the instrument unit is valid. If invalid, log a warning with the {@code logSuffix}
-   * appended. This method should be removed and unit validation should not happen.
-   */
-  static boolean checkValidInstrumentUnit(String unit, String logSuffix) {
-    if (unit != null
-        && !unit.equals("")
-        && unit.length() < 64
-        && StandardCharsets.US_ASCII.newEncoder().canEncode(unit)) {
-      return true;
-    }
-    if (LOGGER.isLoggable(WARNING)) {
-      LOGGER.log(
-          WARNING,
-          "Unit \""
-              + unit
-              + "\" is invalid. Instrument unit must be 63 or fewer ASCII characters."
-              + logSuffix,
-          new AssertionError());
-    }
-    return false;
-  }
-
   @FunctionalInterface
   protected interface SwapBuilder<T> {
     T newBuilder(

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

@@ -5,8 +5,6 @@
 
 package io.opentelemetry.sdk.metrics;
 
-import static io.opentelemetry.sdk.metrics.internal.InstrumentNameValidator.checkValidInstrumentName;
-
 import io.opentelemetry.api.metrics.BatchCallback;
 import io.opentelemetry.api.metrics.DoubleGaugeBuilder;
 import io.opentelemetry.api.metrics.DoubleHistogramBuilder;
@@ -30,6 +28,7 @@ import java.util.List;
 import java.util.Set;
 import java.util.logging.Level;
 import java.util.logging.Logger;
+import java.util.regex.Pattern;
 
 /** {@link SdkMeter} is SDK implementation of {@link Meter}. */
 final class SdkMeter implements Meter {
@@ -37,10 +36,18 @@ final class SdkMeter implements Meter {
   private static final Logger logger = Logger.getLogger(SdkMeter.class.getName());
 
   /**
-   * Message appended to warnings when {@link
+   * Instrument names MUST conform to the following syntax.
-   * InstrumentNameValidator#checkValidInstrumentName(String, String)} is {@code false}.
+   *
+   * <ul>
+   *   <li>They are not null or empty strings.
+   *   <li>They are case-insensitive, ASCII strings.
+   *   <li>The first character must be an alphabetic character.
+   *   <li>Subsequent characters must belong to the alphanumeric characters, '_', '.', and '-'.
+   *   <li>They can have a maximum length of 63 characters.
+   * </ul>
    */
-  private static final String NOOP_INSTRUMENT_WARNING = " Returning noop instrument.";
+  private static final Pattern VALID_INSTRUMENT_NAME_PATTERN =
+      Pattern.compile("([A-Za-z]){1}([A-Za-z0-9\\_\\-\\.]){0,62}");
 
   private static final Meter NOOP_METER = MeterProvider.noop().get("noop");
   private static final String NOOP_INSTRUMENT_NAME = "noop";
@@ -75,7 +82,7 @@ final class SdkMeter implements Meter {
 
   @Override
   public LongCounterBuilder counterBuilder(String name) {
-    return !checkValidInstrumentName(name, NOOP_INSTRUMENT_WARNING)
+    return !checkValidInstrumentName(name)
         ? NOOP_METER.counterBuilder(NOOP_INSTRUMENT_NAME)
         : new SdkLongCounter.SdkLongCounterBuilder(
             meterProviderSharedState, meterSharedState, name);
@@ -83,7 +90,7 @@ final class SdkMeter implements Meter {
 
   @Override
   public LongUpDownCounterBuilder upDownCounterBuilder(String name) {
-    return !checkValidInstrumentName(name, NOOP_INSTRUMENT_WARNING)
+    return !checkValidInstrumentName(name)
         ? NOOP_METER.upDownCounterBuilder(NOOP_INSTRUMENT_NAME)
         : new SdkLongUpDownCounter.SdkLongUpDownCounterBuilder(
             meterProviderSharedState, meterSharedState, name);
@@ -91,7 +98,7 @@ final class SdkMeter implements Meter {
 
   @Override
   public DoubleHistogramBuilder histogramBuilder(String name) {
-    return !checkValidInstrumentName(name, NOOP_INSTRUMENT_WARNING)
+    return !checkValidInstrumentName(name)
         ? NOOP_METER.histogramBuilder(NOOP_INSTRUMENT_NAME)
         : new SdkDoubleHistogram.SdkDoubleHistogramBuilder(
             meterProviderSharedState, meterSharedState, name);
@@ -99,7 +106,7 @@ final class SdkMeter implements Meter {
 
   @Override
   public DoubleGaugeBuilder gaugeBuilder(String name) {
-    return !checkValidInstrumentName(name, NOOP_INSTRUMENT_WARNING)
+    return !checkValidInstrumentName(name)
         ? NOOP_METER.gaugeBuilder(NOOP_INSTRUMENT_NAME)
         : new SdkDoubleGaugeBuilder(meterProviderSharedState, meterSharedState, name);
   }
@@ -143,4 +150,22 @@ final class SdkMeter implements Meter {
   public String toString() {
     return "SdkMeter{instrumentationScopeInfo=" + instrumentationScopeInfo + "}";
   }
+
+  /** Check if the instrument name is valid. If invalid, log a warning. */
+  // Visible for testing
+  static boolean checkValidInstrumentName(String name) {
+    if (name != null && VALID_INSTRUMENT_NAME_PATTERN.matcher(name).matches()) {
+      return true;
+    }
+    if (logger.isLoggable(Level.WARNING)) {
+      logger.log(
+          Level.WARNING,
+          "Instrument name \""
+              + name
+              + "\" is invalid, returning noop instrument. Instrument names must consist of 63 or fewer characters including alphanumeric, _, ., -, and start with a letter.",
+          new AssertionError());
+    }
+
+    return false;
+  }
 }

sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/InstrumentNameValidator.java

@@ -1,63 +0,0 @@
-/*
- * Copyright The OpenTelemetry Authors
- * SPDX-License-Identifier: Apache-2.0
- */
-
-package io.opentelemetry.sdk.metrics.internal;
-
-import java.util.logging.Level;
-import java.util.logging.Logger;
-import java.util.regex.Pattern;
-
-/**
- * Utility for validating instrument names. This class is internal to the SDK and is not intended
- * for public use.
- */
-public class InstrumentNameValidator {
-
-  public static final String LOGGER_NAME =
-      "io.opentelemetry.sdk.metrics.internal.InstrumentNameValidator";
-  private static final Logger LOGGER = Logger.getLogger(LOGGER_NAME);
-
-  /**
-   * Instrument names MUST conform to the following syntax.
-   *
-   * <ul>
-   *   <li>They are not null or empty strings.
-   *   <li>They are case-insensitive, ASCII strings.
-   *   <li>The first character must be an alphabetic character.
-   *   <li>Subsequent characters must belong to the alphanumeric characters, '_', '.', and '-'.
-   *   <li>They can have a maximum length of 63 characters.
-   * </ul>
-   */
-  private static final Pattern VALID_INSTRUMENT_NAME_PATTERN =
-      Pattern.compile("([A-Za-z]){1}([A-Za-z0-9\\_\\-\\.]){0,62}");
-
-  /** Check if the instrument name is valid. If invalid, log a warning. */
-  public static boolean checkValidInstrumentName(String name) {
-    return checkValidInstrumentName(name, "");
-  }
-
-  /**
-   * Check if the instrument name is valid. If invalid, log a warning with the {@code logSuffix}
-   * appended.
-   */
-  public static boolean checkValidInstrumentName(String name, String logSuffix) {
-    if (name != null && VALID_INSTRUMENT_NAME_PATTERN.matcher(name).matches()) {
-      return true;
-    }
-    if (LOGGER.isLoggable(Level.WARNING)) {
-      LOGGER.log(
-          Level.WARNING,
-          "Instrument name \""
-              + name
-              + "\" is invalid, returning noop instrument. Instrument names must consist of 63 or fewer characters including alphanumeric, _, ., -, and start with a letter."
-              + logSuffix,
-          new AssertionError());
-    }
-
-    return false;
-  }
-
-  private InstrumentNameValidator() {}
-}

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

@@ -7,8 +7,6 @@ package io.opentelemetry.sdk.metrics;
 
 import static org.assertj.core.api.Assertions.assertThat;
 
-import io.github.netmikey.logunit.api.LogCapturer;
-import io.opentelemetry.internal.testing.slf4j.SuppressLogger;
 import io.opentelemetry.sdk.common.InstrumentationScopeInfo;
 import io.opentelemetry.sdk.metrics.internal.exemplar.ExemplarFilter;
 import io.opentelemetry.sdk.metrics.internal.state.MeterProviderSharedState;
@@ -17,15 +15,9 @@ import io.opentelemetry.sdk.resources.Resource;
 import io.opentelemetry.sdk.testing.time.TestClock;
 import java.util.Collections;
 import org.junit.jupiter.api.Test;
-import org.junit.jupiter.api.extension.RegisterExtension;
 
-@SuppressLogger(loggerName = AbstractInstrumentBuilder.LOGGER_NAME)
 class AbstractInstrumentBuilderTest {
 
-  @RegisterExtension
-  LogCapturer apiUsageLogs =
-      LogCapturer.create().captureForLogger(AbstractInstrumentBuilder.LOGGER_NAME);
-
   @Test
   void stringRepresentation() {
     InstrumentationScopeInfo scope = InstrumentationScopeInfo.create("scope-name");
@@ -52,36 +44,6 @@ class AbstractInstrumentBuilderTest {
                 + "}}");
   }
 
-  @Test
-  void checkValidInstrumentUnit_InvalidUnitLogs() {
-    assertThat(AbstractInstrumentBuilder.checkValidInstrumentUnit("日", " suffix")).isFalse();
-    apiUsageLogs.assertContains(
-        "Unit \"日\" is invalid. Instrument unit must be 63 or fewer ASCII characters." + " suffix");
-  }
-
-  @Test
-  void checkValidInstrumentUnit() {
-    assertThat(AbstractInstrumentBuilder.checkValidInstrumentUnit("a")).isTrue();
-    assertThat(AbstractInstrumentBuilder.checkValidInstrumentUnit("A")).isTrue();
-    assertThat(AbstractInstrumentBuilder.checkValidInstrumentUnit("foo129")).isTrue();
-    assertThat(AbstractInstrumentBuilder.checkValidInstrumentUnit("!@#$%^&*()")).isTrue();
-    assertThat(
-            AbstractInstrumentBuilder.checkValidInstrumentUnit(
-                new String(new char[63]).replace('\0', 'a')))
-        .isTrue();
-
-    // Empty and null not allowed
-    assertThat(AbstractInstrumentBuilder.checkValidInstrumentUnit(null)).isFalse();
-    assertThat(AbstractInstrumentBuilder.checkValidInstrumentUnit("")).isFalse();
-    // Non-ascii characters
-    assertThat(AbstractInstrumentBuilder.checkValidInstrumentUnit("日")).isFalse();
-    // Must be 63 characters or fewer
-    assertThat(
-            AbstractInstrumentBuilder.checkValidInstrumentUnit(
-                new String(new char[64]).replace('\0', 'a')))
-        .isFalse();
-  }
-
   private static class TestInstrumentBuilder
       extends AbstractInstrumentBuilder<TestInstrumentBuilder> {
 

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

@@ -5,7 +5,7 @@
 
 package io.opentelemetry.sdk.metrics;
 
-import static io.opentelemetry.api.internal.ApiUsageLogger.LOGGER_NAME;
+import static io.opentelemetry.sdk.metrics.SdkMeter.checkValidInstrumentName;
 import static org.assertj.core.api.Assertions.assertThat;
 
 import io.github.netmikey.logunit.api.LogCapturer;
@@ -23,8 +23,8 @@ import io.opentelemetry.sdk.testing.exporter.InMemoryMetricReader;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.RegisterExtension;
 
-@SuppressLogger(loggerName = LOGGER_NAME)
 @SuppressLogger(MetricStorageRegistry.class)
+@SuppressLogger(SdkMeter.class)
 class SdkMeterTest {
 
   private static final Meter NOOP_METER = MeterProvider.noop().get("noop");
@@ -35,7 +35,9 @@ class SdkMeterTest {
       SdkMeterProvider.builder().registerMetricReader(InMemoryMetricReader.create()).build();
 
   @RegisterExtension
-  LogCapturer logs = LogCapturer.create().captureForType(MetricStorageRegistry.class);
+  LogCapturer metricStorageLogs = LogCapturer.create().captureForType(MetricStorageRegistry.class);
+
+  @RegisterExtension LogCapturer sdkMeterLogs = LogCapturer.create().captureForType(SdkMeter.class);
 
   private final Meter sdkMeter = testMeterProvider.get(getClass().getName());
 
@@ -88,35 +90,57 @@ class SdkMeterTest {
   }
 
   @Test
-  void builder_InvalidUnit() {
+  void checkValidInstrumentName_InvalidNameLogs() {
-    String unit = "日";
+    assertThat(checkValidInstrumentName("1")).isFalse();
-    // Counter
+    sdkMeterLogs.assertContains(
-    sdkMeter.counterBuilder("my-instrument").setUnit(unit).build();
+        "Instrument name \"1\" is invalid, returning noop instrument. Instrument names must consist of 63 or fewer characters including alphanumeric, _, ., -, and start with a letter.");
-    sdkMeter.counterBuilder("my-instrument").setUnit(unit).buildWithCallback(unused -> {});
+  }
-    sdkMeter.counterBuilder("my-instrument").setUnit(unit).ofDoubles().build();
-    sdkMeter
-        .counterBuilder("my-instrument")
-        .setUnit(unit)
-        .ofDoubles()
-        .buildWithCallback(unused -> {});
 
-    // UpDownCounter
+  @Test
-    sdkMeter.upDownCounterBuilder("my-instrument").setUnit(unit).build();
+  void checkValidInstrumentNameTest() {
-    sdkMeter.upDownCounterBuilder("my-instrument").setUnit(unit).buildWithCallback(unused -> {});
+    // Valid names
-    sdkMeter.upDownCounterBuilder("my-instrument").setUnit(unit).ofDoubles().build();
+    assertThat(checkValidInstrumentName("f")).isTrue();
-    sdkMeter
+    assertThat(checkValidInstrumentName("F")).isTrue();
-        .upDownCounterBuilder("my-instrument")
+    assertThat(checkValidInstrumentName("foo")).isTrue();
-        .setUnit(unit)
+    assertThat(checkValidInstrumentName("a1")).isTrue();
-        .ofDoubles()
+    assertThat(checkValidInstrumentName("a.")).isTrue();
-        .buildWithCallback(unused -> {});
+    assertThat(checkValidInstrumentName("abcdefghijklmnopqrstuvwxyz")).isTrue();
+    assertThat(checkValidInstrumentName("ABCDEFGHIJKLMNOPQRSTUVWXYZ")).isTrue();
+    assertThat(checkValidInstrumentName("a1234567890")).isTrue();
+    assertThat(checkValidInstrumentName("a_-.")).isTrue();
+    assertThat(checkValidInstrumentName(new String(new char[63]).replace('\0', 'a'))).isTrue();
 
-    // Histogram
+    // Empty and null not allowed
-    sdkMeter.histogramBuilder("my-instrument").setUnit(unit).build();
+    assertThat(checkValidInstrumentName(null)).isFalse();
-    sdkMeter.histogramBuilder("my-instrument").setUnit(unit).ofLongs().build();
+    assertThat(checkValidInstrumentName("")).isFalse();
-
+    // Must start with a letter
-    // Gauge
+    assertThat(checkValidInstrumentName("1")).isFalse();
-    sdkMeter.gaugeBuilder("my-instrument").setUnit(unit).buildWithCallback(unused -> {});
+    assertThat(checkValidInstrumentName(".")).isFalse();
-    sdkMeter.gaugeBuilder("my-instrument").setUnit(unit).ofLongs().buildWithCallback(unused -> {});
+    // Illegal characters
+    assertThat(checkValidInstrumentName("a~")).isFalse();
+    assertThat(checkValidInstrumentName("a!")).isFalse();
+    assertThat(checkValidInstrumentName("a@")).isFalse();
+    assertThat(checkValidInstrumentName("a#")).isFalse();
+    assertThat(checkValidInstrumentName("a$")).isFalse();
+    assertThat(checkValidInstrumentName("a%")).isFalse();
+    assertThat(checkValidInstrumentName("a^")).isFalse();
+    assertThat(checkValidInstrumentName("a&")).isFalse();
+    assertThat(checkValidInstrumentName("a*")).isFalse();
+    assertThat(checkValidInstrumentName("a(")).isFalse();
+    assertThat(checkValidInstrumentName("a)")).isFalse();
+    assertThat(checkValidInstrumentName("a=")).isFalse();
+    assertThat(checkValidInstrumentName("a+")).isFalse();
+    assertThat(checkValidInstrumentName("a{")).isFalse();
+    assertThat(checkValidInstrumentName("a}")).isFalse();
+    assertThat(checkValidInstrumentName("a[")).isFalse();
+    assertThat(checkValidInstrumentName("a]")).isFalse();
+    assertThat(checkValidInstrumentName("a\\")).isFalse();
+    assertThat(checkValidInstrumentName("a|")).isFalse();
+    assertThat(checkValidInstrumentName("a<")).isFalse();
+    assertThat(checkValidInstrumentName("a>")).isFalse();
+    assertThat(checkValidInstrumentName("a?")).isFalse();
+    // Must be 63 characters or fewer
+    assertThat(checkValidInstrumentName(new String(new char[64]).replace('\0', 'a'))).isFalse();
   }
 
   @Test
@@ -136,10 +160,10 @@ class SdkMeterTest {
         .setDescription("My very own counter")
         .setUnit("metric tonnes")
         .build();
-    assertThat(logs.getEvents()).isEmpty();
+    assertThat(metricStorageLogs.getEvents()).isEmpty();
 
     sdkMeter.counterBuilder("testLongCounter").build();
-    logs.assertContains("Found duplicate metric definition");
+    metricStorageLogs.assertContains("Found duplicate metric definition");
   }
 
   @Test
@@ -152,7 +176,7 @@ class SdkMeterTest {
             .build();
     assertThat(longCounter).isNotNull();
     sdkMeter.counterBuilder("testLongCounter".toUpperCase()).build();
-    logs.assertContains("Found duplicate metric definition");
+    metricStorageLogs.assertContains("Found duplicate metric definition");
   }
 
   @Test
@@ -173,10 +197,10 @@ class SdkMeterTest {
         .setDescription("My very own counter")
         .setUnit("metric tonnes")
         .build();
-    assertThat(logs.getEvents()).isEmpty();
+    assertThat(metricStorageLogs.getEvents()).isEmpty();
 
     sdkMeter.upDownCounterBuilder("testLongUpDownCounter").build();
-    logs.assertContains("Found duplicate metric definition");
+    metricStorageLogs.assertContains("Found duplicate metric definition");
   }
 
   @Test
@@ -188,9 +212,9 @@ class SdkMeterTest {
             .setUnit("metric tonnes")
             .build();
     assertThat(longUpDownCounter).isNotNull();
-    assertThat(logs.getEvents()).isEmpty();
+    assertThat(metricStorageLogs.getEvents()).isEmpty();
     sdkMeter.upDownCounterBuilder("testLongUpDownCounter".toUpperCase()).build();
-    logs.assertContains("Found duplicate metric definition");
+    metricStorageLogs.assertContains("Found duplicate metric definition");
   }
 
   @Test
@@ -203,7 +227,7 @@ class SdkMeterTest {
             .setUnit("metric tonnes")
             .build();
     assertThat(longHistogram).isNotNull();
-    assertThat(logs.getEvents()).isEmpty();
+    assertThat(metricStorageLogs.getEvents()).isEmpty();
 
     // Note: We no longer get the same instrument instance as these instances are lightweight
     // objects backed by storage now.  Here we just make sure it doesn't log an error.
@@ -213,10 +237,10 @@ class SdkMeterTest {
         .setDescription("My very own counter")
         .setUnit("metric tonnes")
         .build();
-    assertThat(logs.getEvents()).isEmpty();
+    assertThat(metricStorageLogs.getEvents()).isEmpty();
 
     sdkMeter.histogramBuilder("testLongValueRecorder").ofLongs().build();
-    logs.assertContains("Found duplicate metric definition");
+    metricStorageLogs.assertContains("Found duplicate metric definition");
   }
 
   @Test
@@ -229,10 +253,10 @@ class SdkMeterTest {
             .setUnit("metric tonnes")
             .build();
     assertThat(longHistogram).isNotNull();
-    assertThat(logs.getEvents()).isEmpty();
+    assertThat(metricStorageLogs.getEvents()).isEmpty();
 
     sdkMeter.histogramBuilder("testLongValueRecorder".toUpperCase()).ofLongs().build();
-    logs.assertContains("Found duplicate metric definition");
+    metricStorageLogs.assertContains("Found duplicate metric definition");
   }
 
   @Test
@@ -243,10 +267,10 @@ class SdkMeterTest {
         .setDescription("My very own counter")
         .setUnit("metric tonnes")
         .buildWithCallback(obs -> {});
-    assertThat(logs.getEvents()).isEmpty();
+    assertThat(metricStorageLogs.getEvents()).isEmpty();
 
     sdkMeter.gaugeBuilder("longValueObserver").ofLongs().buildWithCallback(x -> {});
-    logs.assertContains("Found duplicate metric definition");
+    metricStorageLogs.assertContains("Found duplicate metric definition");
   }
 
   @Test
@@ -257,10 +281,10 @@ class SdkMeterTest {
         .setDescription("My very own counter")
         .setUnit("metric tonnes")
         .buildWithCallback(obs -> {});
-    assertThat(logs.getEvents()).isEmpty();
+    assertThat(metricStorageLogs.getEvents()).isEmpty();
 
     sdkMeter.gaugeBuilder("longValueObserver".toUpperCase()).ofLongs().buildWithCallback(x -> {});
-    logs.assertContains("Found duplicate metric definition");
+    metricStorageLogs.assertContains("Found duplicate metric definition");
   }
 
   @Test
@@ -270,10 +294,10 @@ class SdkMeterTest {
         .setDescription("My very own counter")
         .setUnit("metric tonnes")
         .buildWithCallback(x -> {});
-    assertThat(logs.getEvents()).isEmpty();
+    assertThat(metricStorageLogs.getEvents()).isEmpty();
 
     sdkMeter.counterBuilder("testLongSumObserver").buildWithCallback(x -> {});
-    logs.assertContains("Found duplicate metric definition");
+    metricStorageLogs.assertContains("Found duplicate metric definition");
   }
 
   @Test
@@ -283,10 +307,10 @@ class SdkMeterTest {
         .setDescription("My very own counter")
         .setUnit("metric tonnes")
         .buildWithCallback(x -> {});
-    assertThat(logs.getEvents()).isEmpty();
+    assertThat(metricStorageLogs.getEvents()).isEmpty();
 
     sdkMeter.counterBuilder("testLongSumObserver".toUpperCase()).buildWithCallback(x -> {});
-    logs.assertContains("Found duplicate metric definition");
+    metricStorageLogs.assertContains("Found duplicate metric definition");
   }
 
   @Test
@@ -296,10 +320,10 @@ class SdkMeterTest {
         .setDescription("My very own counter")
         .setUnit("metric tonnes")
         .buildWithCallback(x -> {});
-    assertThat(logs.getEvents()).isEmpty();
+    assertThat(metricStorageLogs.getEvents()).isEmpty();
 
     sdkMeter.upDownCounterBuilder("testLongUpDownSumObserver").buildWithCallback(x -> {});
-    logs.assertContains("Found duplicate metric definition");
+    metricStorageLogs.assertContains("Found duplicate metric definition");
   }
 
   @Test
@@ -309,12 +333,12 @@ class SdkMeterTest {
         .setDescription("My very own counter")
         .setUnit("metric tonnes")
         .buildWithCallback(x -> {});
-    assertThat(logs.getEvents()).isEmpty();
+    assertThat(metricStorageLogs.getEvents()).isEmpty();
 
     sdkMeter
         .upDownCounterBuilder("testLongUpDownSumObserver".toUpperCase())
         .buildWithCallback(x -> {});
-    logs.assertContains("Found duplicate metric definition");
+    metricStorageLogs.assertContains("Found duplicate metric definition");
   }
 
   @Test
@@ -336,10 +360,10 @@ class SdkMeterTest {
         .setDescription("My very own counter")
         .setUnit("metric tonnes")
         .build();
-    assertThat(logs.getEvents()).isEmpty();
+    assertThat(metricStorageLogs.getEvents()).isEmpty();
 
     sdkMeter.counterBuilder("testDoubleCounter").ofDoubles().build();
-    logs.assertContains("Found duplicate metric definition");
+    metricStorageLogs.assertContains("Found duplicate metric definition");
   }
 
   @Test
@@ -361,10 +385,10 @@ class SdkMeterTest {
         .setDescription("My very own counter")
         .setUnit("metric tonnes")
         .build();
-    assertThat(logs.getEvents()).isEmpty();
+    assertThat(metricStorageLogs.getEvents()).isEmpty();
 
     sdkMeter.upDownCounterBuilder("testDoubleUpDownCounter").ofDoubles().build();
-    logs.assertContains("Found duplicate metric definition");
+    metricStorageLogs.assertContains("Found duplicate metric definition");
   }
 
   @Test
@@ -384,10 +408,10 @@ class SdkMeterTest {
         .setDescription("My very own ValueRecorder")
         .setUnit("metric tonnes")
         .build();
-    assertThat(logs.getEvents()).isEmpty();
+    assertThat(metricStorageLogs.getEvents()).isEmpty();
 
     sdkMeter.histogramBuilder("testDoubleValueRecorder").build();
-    logs.assertContains("Found duplicate metric definition");
+    metricStorageLogs.assertContains("Found duplicate metric definition");
   }
 
   @Test
@@ -398,10 +422,10 @@ class SdkMeterTest {
         .setDescription("My very own counter")
         .setUnit("metric tonnes")
         .buildWithCallback(x -> {});
-    assertThat(logs.getEvents()).isEmpty();
+    assertThat(metricStorageLogs.getEvents()).isEmpty();
     sdkMeter.counterBuilder("testDoubleSumObserver").ofDoubles().buildWithCallback(x -> {});
     sdkMeter.histogramBuilder("testDoubleValueRecorder").build();
-    logs.assertContains("Found duplicate metric definition");
+    metricStorageLogs.assertContains("Found duplicate metric definition");
   }
 
   @Test
@@ -412,14 +436,14 @@ class SdkMeterTest {
         .setDescription("My very own counter")
         .setUnit("metric tonnes")
         .buildWithCallback(x -> {});
-    assertThat(logs.getEvents()).isEmpty();
+    assertThat(metricStorageLogs.getEvents()).isEmpty();
 
     sdkMeter
         .upDownCounterBuilder("testDoubleUpDownSumObserver")
         .ofDoubles()
         .buildWithCallback(x -> {});
     sdkMeter.histogramBuilder("testDoubleValueRecorder").build();
-    logs.assertContains("Found duplicate metric definition");
+    metricStorageLogs.assertContains("Found duplicate metric definition");
   }
 
   @Test
@@ -429,10 +453,10 @@ class SdkMeterTest {
         .setDescription("My very own counter")
         .setUnit("metric tonnes")
         .buildWithCallback(x -> {});
-    assertThat(logs.getEvents()).isEmpty();
+    assertThat(metricStorageLogs.getEvents()).isEmpty();
 
     sdkMeter.gaugeBuilder("doubleValueObserver").buildWithCallback(x -> {});
-    logs.assertContains("Found duplicate metric definition");
+    metricStorageLogs.assertContains("Found duplicate metric definition");
   }
 
   @Test

sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/internal/InstrumentNameValidatorTest.java

@@ -1,74 +0,0 @@
-/*
- * Copyright The OpenTelemetry Authors
- * SPDX-License-Identifier: Apache-2.0
- */
-
-package io.opentelemetry.sdk.metrics.internal;
-
-import static io.opentelemetry.sdk.metrics.internal.InstrumentNameValidator.checkValidInstrumentName;
-import static org.assertj.core.api.Assertions.assertThat;
-
-import io.github.netmikey.logunit.api.LogCapturer;
-import org.junit.jupiter.api.Test;
-import org.junit.jupiter.api.extension.RegisterExtension;
-
-class InstrumentNameValidatorTest {
-
-  @RegisterExtension
-  LogCapturer apiUsageLogs =
-      LogCapturer.create().captureForLogger(InstrumentNameValidator.LOGGER_NAME);
-
-  @Test
-  void checkValidInstrumentName_InvalidNameLogs() {
-    assertThat(checkValidInstrumentName("1", " suffix")).isFalse();
-    apiUsageLogs.assertContains(
-        "Instrument name \"1\" is invalid, returning noop instrument. Instrument names must consist of 63 or fewer characters including alphanumeric, _, ., -, and start with a letter. suffix");
-  }
-
-  @Test
-  void checkValidInstrumentNameTest() {
-    // Valid names
-    assertThat(checkValidInstrumentName("f")).isTrue();
-    assertThat(checkValidInstrumentName("F")).isTrue();
-    assertThat(checkValidInstrumentName("foo")).isTrue();
-    assertThat(checkValidInstrumentName("a1")).isTrue();
-    assertThat(checkValidInstrumentName("a.")).isTrue();
-    assertThat(checkValidInstrumentName("abcdefghijklmnopqrstuvwxyz")).isTrue();
-    assertThat(checkValidInstrumentName("ABCDEFGHIJKLMNOPQRSTUVWXYZ")).isTrue();
-    assertThat(checkValidInstrumentName("a1234567890")).isTrue();
-    assertThat(checkValidInstrumentName("a_-.")).isTrue();
-    assertThat(checkValidInstrumentName(new String(new char[63]).replace('\0', 'a'))).isTrue();
-
-    // Empty and null not allowed
-    assertThat(checkValidInstrumentName(null)).isFalse();
-    assertThat(checkValidInstrumentName("")).isFalse();
-    // Must start with a letter
-    assertThat(checkValidInstrumentName("1")).isFalse();
-    assertThat(checkValidInstrumentName(".")).isFalse();
-    // Illegal characters
-    assertThat(checkValidInstrumentName("a~")).isFalse();
-    assertThat(checkValidInstrumentName("a!")).isFalse();
-    assertThat(checkValidInstrumentName("a@")).isFalse();
-    assertThat(checkValidInstrumentName("a#")).isFalse();
-    assertThat(checkValidInstrumentName("a$")).isFalse();
-    assertThat(checkValidInstrumentName("a%")).isFalse();
-    assertThat(checkValidInstrumentName("a^")).isFalse();
-    assertThat(checkValidInstrumentName("a&")).isFalse();
-    assertThat(checkValidInstrumentName("a*")).isFalse();
-    assertThat(checkValidInstrumentName("a(")).isFalse();
-    assertThat(checkValidInstrumentName("a)")).isFalse();
-    assertThat(checkValidInstrumentName("a=")).isFalse();
-    assertThat(checkValidInstrumentName("a+")).isFalse();
-    assertThat(checkValidInstrumentName("a{")).isFalse();
-    assertThat(checkValidInstrumentName("a}")).isFalse();
-    assertThat(checkValidInstrumentName("a[")).isFalse();
-    assertThat(checkValidInstrumentName("a]")).isFalse();
-    assertThat(checkValidInstrumentName("a\\")).isFalse();
-    assertThat(checkValidInstrumentName("a|")).isFalse();
-    assertThat(checkValidInstrumentName("a<")).isFalse();
-    assertThat(checkValidInstrumentName("a>")).isFalse();
-    assertThat(checkValidInstrumentName("a?")).isFalse();
-    // Must be 63 characters or fewer
-    assertThat(checkValidInstrumentName(new String(new char[64]).replace('\0', 'a'))).isFalse();
-  }
-}