diff --git a/api/all/src/main/java/io/opentelemetry/api/baggage/ImmutableEntryMetadata.java b/api/all/src/main/java/io/opentelemetry/api/baggage/ImmutableEntryMetadata.java index a367e288a1c..2a2e1a3cc6c 100644 --- a/api/all/src/main/java/io/opentelemetry/api/baggage/ImmutableEntryMetadata.java +++ b/api/all/src/main/java/io/opentelemetry/api/baggage/ImmutableEntryMetadata.java @@ -6,7 +6,7 @@ package io.opentelemetry.api.baggage; import com.google.auto.value.AutoValue; -import io.opentelemetry.api.internal.ValidationUtil; +import io.opentelemetry.api.internal.ApiUsageLogger; import javax.annotation.concurrent.Immutable; @Immutable @@ -25,7 +25,7 @@ abstract class ImmutableEntryMetadata implements BaggageEntryMetadata { */ static ImmutableEntryMetadata create(String metadata) { if (metadata == null) { - ValidationUtil.log("metadata is null"); + ApiUsageLogger.log("metadata is null"); return EMPTY; } return new AutoValue_ImmutableEntryMetadata(metadata); 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 new file mode 100644 index 00000000000..f7bdf236cee --- /dev/null +++ b/api/all/src/main/java/io/opentelemetry/api/internal/ApiUsageLogger.java @@ -0,0 +1,44 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.api.internal; + +import java.util.logging.Level; +import java.util.logging.Logger; + +/** + * Helper for API misuse logging. + * + *

This class is internal and is hence not for public use. Its APIs are unstable and can change + * at any time. + */ +public final class ApiUsageLogger { + + public static final String LOGGER_NAME = "io.opentelemetry.ApiUsageLogger"; + + private static final Logger API_USAGE_LOGGER = Logger.getLogger(LOGGER_NAME); + + /** + * Log the {@code message} to the {@link #LOGGER_NAME API Usage Logger}. + * + *

Log at {@link Level#FINEST} and include a stack trace. + */ + public static void log(String message) { + log(message, Level.FINEST); + } + + /** + * Log the {@code message} to the {@link #LOGGER_NAME API Usage Logger}. + * + *

Log includes a stack trace. + */ + public static void log(String message, Level level) { + if (API_USAGE_LOGGER.isLoggable(level)) { + API_USAGE_LOGGER.log(level, message, new AssertionError()); + } + } + + private ApiUsageLogger() {} +} diff --git a/api/all/src/main/java/io/opentelemetry/api/internal/ValidationUtil.java b/api/all/src/main/java/io/opentelemetry/api/internal/ValidationUtil.java deleted file mode 100644 index 5f743ca25e8..00000000000 --- a/api/all/src/main/java/io/opentelemetry/api/internal/ValidationUtil.java +++ /dev/null @@ -1,107 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.api.internal; - -import java.nio.charset.StandardCharsets; -import java.util.logging.Level; -import java.util.logging.Logger; -import java.util.regex.Pattern; - -/** - * General internal validation utility methods. - * - *

This class is internal and is hence not for public use. Its APIs are unstable and can change - * at any time. - */ -public final class ValidationUtil { - - public static final String API_USAGE_LOGGER_NAME = "io.opentelemetry.ApiUsageLogging"; - - private static final Logger API_USAGE_LOGGER = Logger.getLogger(API_USAGE_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}"); - - /** - * Log the {@code message} to the {@link #API_USAGE_LOGGER_NAME API Usage Logger}. - * - *

Log at {@link Level#FINEST} and include a stack trace. - */ - public static void log(String message) { - log(message, Level.FINEST); - } - - /** - * Log the {@code message} to the {@link #API_USAGE_LOGGER_NAME API Usage Logger}. - * - *

Log includes a stack trace. - */ - public static void log(String message, Level level) { - if (API_USAGE_LOGGER.isLoggable(level)) { - API_USAGE_LOGGER.log(level, message, new AssertionError()); - } - } - - /** 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; - } - log( - "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, - Level.WARNING); - return false; - } - - /** Check if the instrument unit is valid. If invalid, log a warning. */ - public 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. - */ - public static boolean checkValidInstrumentUnit(String unit, String logSuffix) { - if (unit != null - && !unit.equals("") - && unit.length() < 64 - && StandardCharsets.US_ASCII.newEncoder().canEncode(unit)) { - return true; - } - log( - "Unit \"" - + unit - + "\" is invalid. Instrument unit must be 63 or fewer ASCII characters." - + logSuffix, - Level.WARNING); - return false; - } - - private ValidationUtil() {} -} diff --git a/api/all/src/main/java/io/opentelemetry/api/metrics/DefaultMeter.java b/api/all/src/main/java/io/opentelemetry/api/metrics/DefaultMeter.java index 6a0a7bb7c7e..9cab67f239d 100644 --- a/api/all/src/main/java/io/opentelemetry/api/metrics/DefaultMeter.java +++ b/api/all/src/main/java/io/opentelemetry/api/metrics/DefaultMeter.java @@ -6,7 +6,6 @@ package io.opentelemetry.api.metrics; import io.opentelemetry.api.common.Attributes; -import io.opentelemetry.api.internal.ValidationUtil; import io.opentelemetry.context.Context; import java.util.function.Consumer; import javax.annotation.concurrent.ThreadSafe; @@ -39,25 +38,21 @@ static Meter getInstance() { @Override public LongCounterBuilder counterBuilder(String name) { - ValidationUtil.checkValidInstrumentName(name); return NOOP_LONG_COUNTER_BUILDER; } @Override public LongUpDownCounterBuilder upDownCounterBuilder(String name) { - ValidationUtil.checkValidInstrumentName(name); return NOOP_LONG_UP_DOWN_COUNTER_BUILDER; } @Override public DoubleHistogramBuilder histogramBuilder(String name) { - ValidationUtil.checkValidInstrumentName(name); return NOOP_DOUBLE_HISTOGRAM_BUILDER; } @Override public DoubleGaugeBuilder gaugeBuilder(String name) { - ValidationUtil.checkValidInstrumentName(name); return NOOP_DOUBLE_GAUGE_BUILDER; } @@ -107,7 +102,6 @@ public LongCounterBuilder setDescription(String description) { @Override public LongCounterBuilder setUnit(String unit) { - ValidationUtil.checkValidInstrumentUnit(unit); return this; } @@ -144,7 +138,6 @@ public DoubleCounterBuilder setDescription(String description) { @Override public DoubleCounterBuilder setUnit(String unit) { - ValidationUtil.checkValidInstrumentUnit(unit); return this; } @@ -201,7 +194,6 @@ public LongUpDownCounterBuilder setDescription(String description) { @Override public LongUpDownCounterBuilder setUnit(String unit) { - ValidationUtil.checkValidInstrumentUnit(unit); return this; } @@ -240,7 +232,6 @@ public DoubleUpDownCounterBuilder setDescription(String description) { @Override public DoubleUpDownCounterBuilder setUnit(String unit) { - ValidationUtil.checkValidInstrumentUnit(unit); return this; } @@ -295,7 +286,6 @@ public DoubleHistogramBuilder setDescription(String description) { @Override public DoubleHistogramBuilder setUnit(String unit) { - ValidationUtil.checkValidInstrumentUnit(unit); return this; } @@ -320,7 +310,6 @@ public LongHistogramBuilder setDescription(String description) { @Override public LongHistogramBuilder setUnit(String unit) { - ValidationUtil.checkValidInstrumentUnit(unit); return this; } @@ -341,7 +330,6 @@ public DoubleGaugeBuilder setDescription(String description) { @Override public DoubleGaugeBuilder setUnit(String unit) { - ValidationUtil.checkValidInstrumentUnit(unit); return this; } @@ -371,7 +359,6 @@ public LongGaugeBuilder setDescription(String description) { @Override public LongGaugeBuilder setUnit(String unit) { - ValidationUtil.checkValidInstrumentUnit(unit); return this; } diff --git a/api/all/src/main/java/io/opentelemetry/api/trace/DefaultTracer.java b/api/all/src/main/java/io/opentelemetry/api/trace/DefaultTracer.java index 49d32688a7f..a7b02e9363f 100644 --- a/api/all/src/main/java/io/opentelemetry/api/trace/DefaultTracer.java +++ b/api/all/src/main/java/io/opentelemetry/api/trace/DefaultTracer.java @@ -7,7 +7,7 @@ import io.opentelemetry.api.common.AttributeKey; import io.opentelemetry.api.common.Attributes; -import io.opentelemetry.api.internal.ValidationUtil; +import io.opentelemetry.api.internal.ApiUsageLogger; import io.opentelemetry.context.Context; import java.util.concurrent.TimeUnit; import javax.annotation.Nullable; @@ -50,7 +50,7 @@ public Span startSpan() { @Override public NoopSpanBuilder setParent(Context context) { if (context == null) { - ValidationUtil.log("context is null"); + ApiUsageLogger.log("context is null"); return this; } spanContext = Span.fromContext(context).getSpanContext(); diff --git a/api/all/src/main/java/io/opentelemetry/api/trace/Span.java b/api/all/src/main/java/io/opentelemetry/api/trace/Span.java index 878ab6693f0..8f1f1381a71 100644 --- a/api/all/src/main/java/io/opentelemetry/api/trace/Span.java +++ b/api/all/src/main/java/io/opentelemetry/api/trace/Span.java @@ -10,7 +10,7 @@ import io.opentelemetry.api.common.AttributeKey; import io.opentelemetry.api.common.Attributes; -import io.opentelemetry.api.internal.ValidationUtil; +import io.opentelemetry.api.internal.ApiUsageLogger; import io.opentelemetry.context.Context; import io.opentelemetry.context.ImplicitContextKeyed; import java.time.Instant; @@ -43,7 +43,7 @@ static Span current() { */ static Span fromContext(Context context) { if (context == null) { - ValidationUtil.log("context is null"); + ApiUsageLogger.log("context is null"); return Span.getInvalid(); } Span span = context.get(SpanContextKey.KEY); @@ -57,7 +57,7 @@ static Span fromContext(Context context) { @Nullable static Span fromContextOrNull(Context context) { if (context == null) { - ValidationUtil.log("context is null"); + ApiUsageLogger.log("context is null"); return null; } return context.get(SpanContextKey.KEY); @@ -78,7 +78,7 @@ static Span getInvalid() { */ static Span wrap(SpanContext spanContext) { if (spanContext == null) { - ValidationUtil.log("context is null"); + ApiUsageLogger.log("context is null"); return getInvalid(); } if (!spanContext.isValid()) { diff --git a/api/all/src/main/java/io/opentelemetry/api/trace/SpanId.java b/api/all/src/main/java/io/opentelemetry/api/trace/SpanId.java index f153b6c9f22..0076974bb1c 100644 --- a/api/all/src/main/java/io/opentelemetry/api/trace/SpanId.java +++ b/api/all/src/main/java/io/opentelemetry/api/trace/SpanId.java @@ -5,9 +5,9 @@ package io.opentelemetry.api.trace; +import io.opentelemetry.api.internal.ApiUsageLogger; import io.opentelemetry.api.internal.OtelEncodingUtils; import io.opentelemetry.api.internal.TemporaryBuffers; -import io.opentelemetry.api.internal.ValidationUtil; import javax.annotation.concurrent.Immutable; /** @@ -73,7 +73,7 @@ public static boolean isValid(CharSequence spanId) { */ public static String fromBytes(byte[] spanIdBytes) { if (spanIdBytes == null || spanIdBytes.length < BYTES_LENGTH) { - ValidationUtil.log("spanIdBytes is null or too short"); + ApiUsageLogger.log("spanIdBytes is null or too short"); return INVALID; } char[] result = TemporaryBuffers.chars(HEX_LENGTH); diff --git a/api/all/src/main/java/io/opentelemetry/api/trace/TraceId.java b/api/all/src/main/java/io/opentelemetry/api/trace/TraceId.java index d68d81d9f3c..5be08017f7b 100644 --- a/api/all/src/main/java/io/opentelemetry/api/trace/TraceId.java +++ b/api/all/src/main/java/io/opentelemetry/api/trace/TraceId.java @@ -5,9 +5,9 @@ package io.opentelemetry.api.trace; +import io.opentelemetry.api.internal.ApiUsageLogger; import io.opentelemetry.api.internal.OtelEncodingUtils; import io.opentelemetry.api.internal.TemporaryBuffers; -import io.opentelemetry.api.internal.ValidationUtil; import javax.annotation.concurrent.Immutable; /** @@ -77,7 +77,7 @@ public static boolean isValid(CharSequence traceId) { */ public static String fromBytes(byte[] traceIdBytes) { if (traceIdBytes == null || traceIdBytes.length < BYTES_LENGTH) { - ValidationUtil.log("traceIdBytes is null or too short"); + ApiUsageLogger.log("traceIdBytes is null or too short"); return INVALID; } char[] result = TemporaryBuffers.chars(HEX_LENGTH); 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 new file mode 100644 index 00000000000..5897145ea99 --- /dev/null +++ b/api/all/src/test/java/io/opentelemetry/api/internal/ApiUsageLoggerTest.java @@ -0,0 +1,24 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.api.internal; + +import static java.util.logging.Level.WARNING; + +import io.github.netmikey.logunit.api.LogCapturer; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +class ApiUsageLoggerTest { + + @RegisterExtension + LogCapturer apiUsageLogs = LogCapturer.create().captureForLogger(ApiUsageLogger.LOGGER_NAME); + + @Test + void log() { + ApiUsageLogger.log("thing", WARNING); + apiUsageLogs.assertContains("thing"); + } +} diff --git a/api/all/src/test/java/io/opentelemetry/api/internal/ValidationUtilTest.java b/api/all/src/test/java/io/opentelemetry/api/internal/ValidationUtilTest.java deleted file mode 100644 index b9ecad858b9..00000000000 --- a/api/all/src/test/java/io/opentelemetry/api/internal/ValidationUtilTest.java +++ /dev/null @@ -1,103 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.api.internal; - -import static org.assertj.core.api.Assertions.assertThat; - -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(loggerName = ValidationUtil.API_USAGE_LOGGER_NAME) -public class ValidationUtilTest { - - @RegisterExtension - LogCapturer apiUsageLogs = - LogCapturer.create().captureForLogger(ValidationUtil.API_USAGE_LOGGER_NAME); - - @Test - void checkValidInstrumentName_InvalidNameLogs() { - assertThat(ValidationUtil.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 checkValidInstrumentName() { - // Valid names - assertThat(ValidationUtil.checkValidInstrumentName("f")).isTrue(); - assertThat(ValidationUtil.checkValidInstrumentName("F")).isTrue(); - assertThat(ValidationUtil.checkValidInstrumentName("foo")).isTrue(); - assertThat(ValidationUtil.checkValidInstrumentName("a1")).isTrue(); - assertThat(ValidationUtil.checkValidInstrumentName("a.")).isTrue(); - assertThat(ValidationUtil.checkValidInstrumentName("abcdefghijklmnopqrstuvwxyz")).isTrue(); - assertThat(ValidationUtil.checkValidInstrumentName("ABCDEFGHIJKLMNOPQRSTUVWXYZ")).isTrue(); - assertThat(ValidationUtil.checkValidInstrumentName("a1234567890")).isTrue(); - assertThat(ValidationUtil.checkValidInstrumentName("a_-.")).isTrue(); - assertThat(ValidationUtil.checkValidInstrumentName(new String(new char[63]).replace('\0', 'a'))) - .isTrue(); - - // Empty and null not allowed - assertThat(ValidationUtil.checkValidInstrumentName(null)).isFalse(); - assertThat(ValidationUtil.checkValidInstrumentName("")).isFalse(); - // Must start with a letter - assertThat(ValidationUtil.checkValidInstrumentName("1")).isFalse(); - assertThat(ValidationUtil.checkValidInstrumentName(".")).isFalse(); - // Illegal characters - assertThat(ValidationUtil.checkValidInstrumentName("a~")).isFalse(); - assertThat(ValidationUtil.checkValidInstrumentName("a!")).isFalse(); - assertThat(ValidationUtil.checkValidInstrumentName("a@")).isFalse(); - assertThat(ValidationUtil.checkValidInstrumentName("a#")).isFalse(); - assertThat(ValidationUtil.checkValidInstrumentName("a$")).isFalse(); - assertThat(ValidationUtil.checkValidInstrumentName("a%")).isFalse(); - assertThat(ValidationUtil.checkValidInstrumentName("a^")).isFalse(); - assertThat(ValidationUtil.checkValidInstrumentName("a&")).isFalse(); - assertThat(ValidationUtil.checkValidInstrumentName("a*")).isFalse(); - assertThat(ValidationUtil.checkValidInstrumentName("a(")).isFalse(); - assertThat(ValidationUtil.checkValidInstrumentName("a)")).isFalse(); - assertThat(ValidationUtil.checkValidInstrumentName("a=")).isFalse(); - assertThat(ValidationUtil.checkValidInstrumentName("a+")).isFalse(); - assertThat(ValidationUtil.checkValidInstrumentName("a{")).isFalse(); - assertThat(ValidationUtil.checkValidInstrumentName("a}")).isFalse(); - assertThat(ValidationUtil.checkValidInstrumentName("a[")).isFalse(); - assertThat(ValidationUtil.checkValidInstrumentName("a]")).isFalse(); - assertThat(ValidationUtil.checkValidInstrumentName("a\\")).isFalse(); - assertThat(ValidationUtil.checkValidInstrumentName("a|")).isFalse(); - assertThat(ValidationUtil.checkValidInstrumentName("a<")).isFalse(); - assertThat(ValidationUtil.checkValidInstrumentName("a>")).isFalse(); - assertThat(ValidationUtil.checkValidInstrumentName("a?")).isFalse(); - // Must be 63 characters or less - assertThat(ValidationUtil.checkValidInstrumentName(new String(new char[64]).replace('\0', 'a'))) - .isFalse(); - } - - @Test - void checkValidInstrumentUnit_InvalidUnitLogs() { - assertThat(ValidationUtil.checkValidInstrumentUnit("日", " suffix")).isFalse(); - apiUsageLogs.assertContains( - "Unit \"日\" is invalid. Instrument unit must be 63 or fewer ASCII characters." + " suffix"); - } - - @Test - void checkValidInstrumentUnit() { - assertThat(ValidationUtil.checkValidInstrumentUnit("a")).isTrue(); - assertThat(ValidationUtil.checkValidInstrumentUnit("A")).isTrue(); - assertThat(ValidationUtil.checkValidInstrumentUnit("foo129")).isTrue(); - assertThat(ValidationUtil.checkValidInstrumentUnit("!@#$%^&*()")).isTrue(); - assertThat(ValidationUtil.checkValidInstrumentUnit(new String(new char[63]).replace('\0', 'a'))) - .isTrue(); - - // Empty and null not allowed - assertThat(ValidationUtil.checkValidInstrumentUnit(null)).isFalse(); - assertThat(ValidationUtil.checkValidInstrumentUnit("")).isFalse(); - // Non-ascii characters - assertThat(ValidationUtil.checkValidInstrumentUnit("日")).isFalse(); - // Must be 63 characters or less - assertThat(ValidationUtil.checkValidInstrumentUnit(new String(new char[64]).replace('\0', 'a'))) - .isFalse(); - } -} 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 66efaddc6c4..4de729116da 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,8 +6,7 @@ package io.opentelemetry.api.metrics; import static io.opentelemetry.api.common.AttributeKey.stringKey; -import static io.opentelemetry.api.internal.ValidationUtil.API_USAGE_LOGGER_NAME; -import static org.assertj.core.api.Assertions.assertThat; +import static io.opentelemetry.api.internal.ApiUsageLogger.LOGGER_NAME; import io.github.netmikey.logunit.api.LogCapturer; import io.opentelemetry.api.common.Attributes; @@ -15,100 +14,12 @@ import io.opentelemetry.internal.testing.slf4j.SuppressLogger; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; -import org.slf4j.event.LoggingEvent; -@SuppressLogger(loggerName = API_USAGE_LOGGER_NAME) +@SuppressLogger(loggerName = LOGGER_NAME) public class DefaultMeterTest { private static final Meter METER = DefaultMeter.getInstance(); - private static final String NOOP_INSTRUMENT_NAME = "noop"; - @RegisterExtension - LogCapturer apiUsageLogs = LogCapturer.create().captureForLogger(API_USAGE_LOGGER_NAME); - - @Test - void builder_InvalidName() { - // Counter - assertThat(METER.counterBuilder("1").build()) - .isSameAs(METER.counterBuilder(NOOP_INSTRUMENT_NAME).build()); - assertThat(METER.counterBuilder("1").ofDoubles().build()) - .isSameAs(METER.counterBuilder(NOOP_INSTRUMENT_NAME).ofDoubles().build()); - assertThat(METER.counterBuilder("1").buildWithCallback(unused -> {})) - .isSameAs(METER.counterBuilder(NOOP_INSTRUMENT_NAME).buildWithCallback(unused -> {})); - assertThat(METER.counterBuilder("1").ofDoubles().buildWithCallback(unused -> {})) - .isSameAs( - METER.counterBuilder(NOOP_INSTRUMENT_NAME).ofDoubles().buildWithCallback(unused -> {})); - - // UpDownCounter - assertThat(METER.upDownCounterBuilder("1").build()) - .isSameAs(METER.upDownCounterBuilder(NOOP_INSTRUMENT_NAME).build()); - assertThat(METER.upDownCounterBuilder("1").ofDoubles().build()) - .isSameAs(METER.upDownCounterBuilder(NOOP_INSTRUMENT_NAME).ofDoubles().build()); - assertThat(METER.upDownCounterBuilder("1").buildWithCallback(unused -> {})) - .isSameAs(METER.upDownCounterBuilder(NOOP_INSTRUMENT_NAME).buildWithCallback(unused -> {})); - assertThat(METER.upDownCounterBuilder("1").ofDoubles().buildWithCallback(unused -> {})) - .isSameAs( - METER - .upDownCounterBuilder(NOOP_INSTRUMENT_NAME) - .ofDoubles() - .buildWithCallback(unused -> {})); - - // Histogram - assertThat(METER.histogramBuilder("1").build()) - .isSameAs(METER.histogramBuilder(NOOP_INSTRUMENT_NAME).build()); - assertThat(METER.histogramBuilder("1").ofLongs().build()) - .isSameAs(METER.histogramBuilder(NOOP_INSTRUMENT_NAME).ofLongs().build()); - - // Gauage - assertThat(METER.gaugeBuilder("1").buildWithCallback(unused -> {})) - .isSameAs(METER.gaugeBuilder(NOOP_INSTRUMENT_NAME).buildWithCallback(unused -> {})); - assertThat(METER.gaugeBuilder("1").ofLongs().buildWithCallback(unused -> {})) - .isSameAs( - METER.gaugeBuilder(NOOP_INSTRUMENT_NAME).ofLongs().buildWithCallback(unused -> {})); - - assertThat(apiUsageLogs.getEvents()) - .extracting(LoggingEvent::getMessage) - .hasSize(12) - .allMatch( - log -> - log.equals( - "Instrument name \"1\" is invalid, returning noop instrument. Instrument names must consist of 63 or fewer characters including alphanumeric, _, ., -, and start with a letter.")); - } - - @Test - void builder_InvalidUnit() { - String unit = "日"; - // Counter - METER.counterBuilder("my-instrument").setUnit(unit).build(); - METER.counterBuilder("my-instrument").setUnit(unit).buildWithCallback(unused -> {}); - METER.counterBuilder("my-instrument").setUnit(unit).ofDoubles().build(); - METER.counterBuilder("my-instrument").setUnit(unit).ofDoubles().buildWithCallback(unused -> {}); - - // UpDownCounter - METER.upDownCounterBuilder("my-instrument").setUnit(unit).build(); - METER.upDownCounterBuilder("my-instrument").setUnit(unit).buildWithCallback(unused -> {}); - METER.upDownCounterBuilder("my-instrument").setUnit(unit).ofDoubles().build(); - METER - .upDownCounterBuilder("my-instrument") - .setUnit(unit) - .ofDoubles() - .buildWithCallback(unused -> {}); - - // Histogram - METER.histogramBuilder("my-instrument").setUnit(unit).build(); - METER.histogramBuilder("my-instrument").setUnit(unit).ofLongs().build(); - - // Gauge - METER.gaugeBuilder("my-instrument").setUnit(unit).buildWithCallback(unused -> {}); - METER.gaugeBuilder("my-instrument").setUnit(unit).ofLongs().buildWithCallback(unused -> {}); - - assertThat(apiUsageLogs.getEvents()) - .hasSize(12) - .extracting(LoggingEvent::getMessage) - .allMatch( - log -> - log.equals( - "Unit \"日\" is invalid. Instrument unit must be 63 or fewer ASCII characters.")); - } + @RegisterExtension LogCapturer apiUsageLogs = LogCapturer.create().captureForLogger(LOGGER_NAME); @Test void noopLongCounter_doesNotThrow() { 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 d58611712c9..f67020b6adc 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,7 @@ 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.ValidationUtil.API_USAGE_LOGGER_NAME; +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; @@ -36,8 +36,7 @@ class SdkLoggerTest { - @RegisterExtension - LogCapturer apiUsageLogs = LogCapturer.create().captureForLogger(API_USAGE_LOGGER_NAME); + @RegisterExtension LogCapturer apiUsageLogs = LogCapturer.create().captureForLogger(LOGGER_NAME); @Test void logRecordBuilder() { 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 911e0c4bc55..3c41cb1c38b 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,7 +5,8 @@ package io.opentelemetry.sdk.metrics; -import io.opentelemetry.api.internal.ValidationUtil; +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; @@ -14,14 +15,18 @@ 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; @@ -52,7 +57,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 630f3313f1c..634df444321 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,7 +5,8 @@ package io.opentelemetry.sdk.metrics; -import io.opentelemetry.api.internal.ValidationUtil; +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; @@ -36,8 +37,8 @@ final class SdkMeter implements Meter { private static final Logger logger = Logger.getLogger(SdkMeter.class.getName()); /** - * Message appended to warnings when {@link ValidationUtil#checkValidInstrumentName(String, - * String)} is {@code false}. + * Message appended to warnings when {@link + * InstrumentNameValidator#checkValidInstrumentName(String, String)} is {@code false}. */ private static final String NOOP_INSTRUMENT_WARNING = " Returning noop instrument."; @@ -74,7 +75,7 @@ void resetForTest() { @Override public LongCounterBuilder counterBuilder(String name) { - return !ValidationUtil.checkValidInstrumentName(name, NOOP_INSTRUMENT_WARNING) + return !checkValidInstrumentName(name, NOOP_INSTRUMENT_WARNING) ? NOOP_METER.counterBuilder(NOOP_INSTRUMENT_NAME) : new SdkLongCounter.SdkLongCounterBuilder( meterProviderSharedState, meterSharedState, name); @@ -82,7 +83,7 @@ public LongCounterBuilder counterBuilder(String name) { @Override public LongUpDownCounterBuilder upDownCounterBuilder(String name) { - return !ValidationUtil.checkValidInstrumentName(name, NOOP_INSTRUMENT_WARNING) + return !checkValidInstrumentName(name, NOOP_INSTRUMENT_WARNING) ? NOOP_METER.upDownCounterBuilder(NOOP_INSTRUMENT_NAME) : new SdkLongUpDownCounter.SdkLongUpDownCounterBuilder( meterProviderSharedState, meterSharedState, name); @@ -90,7 +91,7 @@ public LongUpDownCounterBuilder upDownCounterBuilder(String name) { @Override public DoubleHistogramBuilder histogramBuilder(String name) { - return !ValidationUtil.checkValidInstrumentName(name, NOOP_INSTRUMENT_WARNING) + return !checkValidInstrumentName(name, NOOP_INSTRUMENT_WARNING) ? NOOP_METER.histogramBuilder(NOOP_INSTRUMENT_NAME) : new SdkDoubleHistogram.SdkDoubleHistogramBuilder( meterProviderSharedState, meterSharedState, name); @@ -98,7 +99,7 @@ public DoubleHistogramBuilder histogramBuilder(String name) { @Override public DoubleGaugeBuilder gaugeBuilder(String name) { - return !ValidationUtil.checkValidInstrumentName(name, NOOP_INSTRUMENT_WARNING) + return !checkValidInstrumentName(name, NOOP_INSTRUMENT_WARNING) ? NOOP_METER.gaugeBuilder(NOOP_INSTRUMENT_NAME) : new SdkDoubleGaugeBuilder(meterProviderSharedState, meterSharedState, name); } 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 new file mode 100644 index 00000000000..4e12c53ac6c --- /dev/null +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/InstrumentNameValidator.java @@ -0,0 +1,63 @@ +/* + * 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 ec79e45960c..1fdaab3d121 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,6 +7,8 @@ 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; @@ -15,9 +17,15 @@ 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"); @@ -44,6 +52,36 @@ 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 4471f2f4418..a1987c4ed32 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.ValidationUtil.API_USAGE_LOGGER_NAME; +import static io.opentelemetry.api.internal.ApiUsageLogger.LOGGER_NAME; import static org.assertj.core.api.Assertions.assertThat; import io.github.netmikey.logunit.api.LogCapturer; @@ -22,9 +22,8 @@ import io.opentelemetry.sdk.testing.exporter.InMemoryMetricReader; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; -import org.slf4j.event.LoggingEvent; -@SuppressLogger(loggerName = API_USAGE_LOGGER_NAME) +@SuppressLogger(loggerName = LOGGER_NAME) @SuppressLogger(MetricStorageRegistry.class) class SdkMeterTest { @@ -34,13 +33,11 @@ class SdkMeterTest { // Meter must have an exporter configured to actual run. private final SdkMeterProvider testMeterProvider = SdkMeterProvider.builder().registerMetricReader(InMemoryMetricReader.create()).build(); - private final Meter sdkMeter = testMeterProvider.get(getClass().getName()); @RegisterExtension LogCapturer logs = LogCapturer.create().captureForType(MetricStorageRegistry.class); - @RegisterExtension - LogCapturer apiUsageLogs = LogCapturer.create().captureForLogger(API_USAGE_LOGGER_NAME); + private final Meter sdkMeter = testMeterProvider.get(getClass().getName()); @Test void builder_InvalidName() { @@ -88,14 +85,6 @@ void builder_InvalidName() { .gaugeBuilder(NOOP_INSTRUMENT_NAME) .ofLongs() .buildWithCallback(unused -> {})); - - assertThat(apiUsageLogs.getEvents()) - .extracting(LoggingEvent::getMessage) - .hasSize(12) - .allMatch( - log -> - log.equals( - "Instrument name \"1\" is invalid, returning noop instrument. Instrument names must consist of 63 or fewer characters including alphanumeric, _, ., -, and start with a letter. Returning noop instrument.")); } @Test @@ -128,14 +117,6 @@ void builder_InvalidUnit() { // Gauge sdkMeter.gaugeBuilder("my-instrument").setUnit(unit).buildWithCallback(unused -> {}); sdkMeter.gaugeBuilder("my-instrument").setUnit(unit).ofLongs().buildWithCallback(unused -> {}); - - assertThat(apiUsageLogs.getEvents()) - .hasSize(12) - .extracting(LoggingEvent::getMessage) - .allMatch( - log -> - log.equals( - "Unit \"日\" is invalid. Instrument unit must be 63 or fewer ASCII characters. Using \"\" for instrument my-instrument instead.")); } @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 new file mode 100644 index 00000000000..5025deb60c8 --- /dev/null +++ b/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/internal/InstrumentNameValidatorTest.java @@ -0,0 +1,74 @@ +/* + * 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(); + } +}