diff --git a/api/all/src/main/java/io/opentelemetry/api/internal/ApiUsageLogger.java b/api/all/src/main/java/io/opentelemetry/api/internal/ApiUsageLogger.java index f7bdf236cee..9253afa10a0 100644 --- a/api/all/src/main/java/io/opentelemetry/api/internal/ApiUsageLogger.java +++ b/api/all/src/main/java/io/opentelemetry/api/internal/ApiUsageLogger.java @@ -16,12 +16,10 @@ */ public final class ApiUsageLogger { - public static final String LOGGER_NAME = "io.opentelemetry.ApiUsageLogger"; - - private static final Logger API_USAGE_LOGGER = Logger.getLogger(LOGGER_NAME); + private static final Logger API_USAGE_LOGGER = Logger.getLogger(ApiUsageLogger.class.getName()); /** - * 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}. * *

Log at {@link Level#FINEST} and include a stack trace. */ @@ -30,7 +28,7 @@ public static void log(String message) { } /** - * 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}. * *

Log includes a stack trace. */ diff --git a/api/all/src/test/java/io/opentelemetry/api/internal/ApiUsageLoggerTest.java b/api/all/src/test/java/io/opentelemetry/api/internal/ApiUsageLoggerTest.java index 5897145ea99..ee284f13406 100644 --- a/api/all/src/test/java/io/opentelemetry/api/internal/ApiUsageLoggerTest.java +++ b/api/all/src/test/java/io/opentelemetry/api/internal/ApiUsageLoggerTest.java @@ -8,13 +8,15 @@ 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() { diff --git a/api/all/src/test/java/io/opentelemetry/api/metrics/DefaultMeterTest.java b/api/all/src/test/java/io/opentelemetry/api/metrics/DefaultMeterTest.java index 4de729116da..a42b194f81a 100644 --- a/api/all/src/test/java/io/opentelemetry/api/metrics/DefaultMeterTest.java +++ b/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 = diff --git a/exporters/common/src/test/java/io/opentelemetry/exporter/internal/TlsConfigHelperTest.java b/exporters/common/src/test/java/io/opentelemetry/exporter/internal/TlsConfigHelperTest.java index ccdac920265..d6e871881e5 100644 --- a/exporters/common/src/test/java/io/opentelemetry/exporter/internal/TlsConfigHelperTest.java +++ b/exporters/common/src/test/java/io/opentelemetry/exporter/internal/TlsConfigHelperTest.java @@ -16,6 +16,7 @@ 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.junit.jupiter.MockitoExtension; @ExtendWith(MockitoExtension.class) +@SuppressLogger(TlsConfigHelper.class) class TlsConfigHelperTest { @RegisterExtension static final SelfSignedCertificateExtension serverTls = new SelfSignedCertificateExtension(); diff --git a/sdk/logs/src/test/java/io/opentelemetry/sdk/logs/SdkLoggerTest.java b/sdk/logs/src/test/java/io/opentelemetry/sdk/logs/SdkLoggerTest.java index f67020b6adc..29f447bb69d 100644 --- a/sdk/logs/src/test/java/io/opentelemetry/sdk/logs/SdkLoggerTest.java +++ b/sdk/logs/src/test/java/io/opentelemetry/sdk/logs/SdkLoggerTest.java @@ -9,7 +9,6 @@ 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.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.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); diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/AbstractInstrumentBuilder.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/AbstractInstrumentBuilder.java index 3c41cb1c38b..fd38837b6f3 100644 --- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/AbstractInstrumentBuilder.java +++ b/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.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> { 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 { T newBuilder( diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/SdkMeter.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/SdkMeter.java index 634df444321..906c5a41a02 100644 --- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/SdkMeter.java +++ b/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.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 - * InstrumentNameValidator#checkValidInstrumentName(String, String)} is {@code false}. + * Instrument names MUST conform to the following syntax. + * + *

*/ - 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 @@ void resetForTest() { @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 @@ public LongCounterBuilder counterBuilder(String name) { @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 @@ public LongUpDownCounterBuilder upDownCounterBuilder(String name) { @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 @@ public DoubleHistogramBuilder histogramBuilder(String name) { @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 @@ public BatchCallback batchCallback( 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; + } } diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/InstrumentNameValidator.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/InstrumentNameValidator.java deleted file mode 100644 index 4e12c53ac6c..00000000000 --- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/InstrumentNameValidator.java +++ /dev/null @@ -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. - * - * - */ - 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() {} -} diff --git a/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/AbstractInstrumentBuilderTest.java b/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/AbstractInstrumentBuilderTest.java index 1fdaab3d121..ec79e45960c 100644 --- a/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/AbstractInstrumentBuilderTest.java +++ b/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/AbstractInstrumentBuilderTest.java @@ -7,8 +7,6 @@ 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.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 @@ void stringRepresentation() { + "}}"); } - @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 { diff --git a/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/SdkMeterTest.java b/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/SdkMeterTest.java index a1987c4ed32..4892fb7c0f9 100644 --- a/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/SdkMeterTest.java +++ b/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 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 @@ void builder_InvalidName() { } @Test - void builder_InvalidUnit() { - String unit = "日"; - // Counter - sdkMeter.counterBuilder("my-instrument").setUnit(unit).build(); - 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 - sdkMeter.upDownCounterBuilder("my-instrument").setUnit(unit).build(); - sdkMeter.upDownCounterBuilder("my-instrument").setUnit(unit).buildWithCallback(unused -> {}); - sdkMeter.upDownCounterBuilder("my-instrument").setUnit(unit).ofDoubles().build(); - sdkMeter - .upDownCounterBuilder("my-instrument") - .setUnit(unit) - .ofDoubles() - .buildWithCallback(unused -> {}); - - // Histogram - sdkMeter.histogramBuilder("my-instrument").setUnit(unit).build(); - sdkMeter.histogramBuilder("my-instrument").setUnit(unit).ofLongs().build(); + void checkValidInstrumentName_InvalidNameLogs() { + assertThat(checkValidInstrumentName("1")).isFalse(); + sdkMeterLogs.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."); + } - // Gauge - sdkMeter.gaugeBuilder("my-instrument").setUnit(unit).buildWithCallback(unused -> {}); - sdkMeter.gaugeBuilder("my-instrument").setUnit(unit).ofLongs().buildWithCallback(unused -> {}); + @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(); } @Test @@ -136,10 +160,10 @@ void testLongCounter() { .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 @@ void testLongCounter_upperCaseConflict() { .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 @@ void testLongUpDownCounter() { .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 @@ void testLongUpDownCounter_upperCaseConflict() { .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 @@ void testLongHistogram() { .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 @@ void testLongHistogram() { .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 @@ void testLongHistogram_upperCaseConflict() { .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 @@ void testLongGauge_conflicts() { .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 @@ void testLongGauge_upperCaseConflicts() { .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 @@ void testLongSumObserver_conflicts() { .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 @@ void testLongSumObserver_upperCaseConflicts() { .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 @@ void testLongUpDownSumObserver_conflicts() { .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 @@ void testLongUpDownSumObserver_upperCaseConflicts() { .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 @@ void testDoubleCounter() { .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 @@ void testDoubleUpDownCounter() { .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 @@ void testDoubleHistogram() { .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 @@ void testDoubleSumObserver() { .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 @@ void testDoubleUpDownSumObserver() { .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 @@ void testDoubleGauge() { .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 diff --git a/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/internal/InstrumentNameValidatorTest.java b/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/internal/InstrumentNameValidatorTest.java deleted file mode 100644 index 5025deb60c8..00000000000 --- a/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/internal/InstrumentNameValidatorTest.java +++ /dev/null @@ -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(); - } -}