open-telemetry
/
opentelemetry-java
mirror of https://github.com/open-telemetry/opentelemetry-java.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Remove validations for noop instrument names and units (#5146) 

* remove validations for noop implementation

* remove unwanted tests

* remove unused

* remove instrument unit checks from ValidationUtil and move temporarily to AbstractInstrumentBuilder.

* move tests

* rename ValidationUtil to ApiUsageLogger

* fix tests by removing logs check (not important here)
This commit is contained in:
jason plumb 2023-03-08 12:07:07 -08:00 committed by GitHub
parent 018a76670c
commit dd40fbeab7
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
18 changed files with 306 additions and 361 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
api/all/src/main/java/io/opentelemetry/api/baggage/ImmutableEntryMetadata.java
View File

@ -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);

44
api/all/src/main/java/io/opentelemetry/api/internal/ApiUsageLogger.java Normal file
View File

@ -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.
*
* <p>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}.
*
* <p>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}.
*
* <p>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() {}
}

107
api/all/src/main/java/io/opentelemetry/api/internal/ValidationUtil.java
View File

@ -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.
*
* <p>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.
*
* <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}");
/**
* Log the {@code message} to the {@link #API_USAGE_LOGGER_NAME API Usage Logger}.
*
* <p>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}.
*
* <p>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() {}
}

13
api/all/src/main/java/io/opentelemetry/api/metrics/DefaultMeter.java
View File

@ -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 @@ class DefaultMeter implements Meter {
@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 @@ class DefaultMeter implements Meter {
@Override
public LongCounterBuilder setUnit(String unit) {
ValidationUtil.checkValidInstrumentUnit(unit);
return this;
}
@ -144,7 +138,6 @@ class DefaultMeter implements Meter {
@Override
public DoubleCounterBuilder setUnit(String unit) {
ValidationUtil.checkValidInstrumentUnit(unit);
return this;
}
@ -201,7 +194,6 @@ class DefaultMeter implements Meter {
@Override
public LongUpDownCounterBuilder setUnit(String unit) {
ValidationUtil.checkValidInstrumentUnit(unit);
return this;
}
@ -240,7 +232,6 @@ class DefaultMeter implements Meter {
@Override
public DoubleUpDownCounterBuilder setUnit(String unit) {
ValidationUtil.checkValidInstrumentUnit(unit);
return this;
}
@ -295,7 +286,6 @@ class DefaultMeter implements Meter {
@Override
public DoubleHistogramBuilder setUnit(String unit) {
ValidationUtil.checkValidInstrumentUnit(unit);
return this;
}
@ -320,7 +310,6 @@ class DefaultMeter implements Meter {
@Override
public LongHistogramBuilder setUnit(String unit) {
ValidationUtil.checkValidInstrumentUnit(unit);
return this;
}
@ -341,7 +330,6 @@ class DefaultMeter implements Meter {
@Override
public DoubleGaugeBuilder setUnit(String unit) {
ValidationUtil.checkValidInstrumentUnit(unit);
return this;
}
@ -371,7 +359,6 @@ class DefaultMeter implements Meter {
@Override
public LongGaugeBuilder setUnit(String unit) {
ValidationUtil.checkValidInstrumentUnit(unit);
return this;
}

4
api/all/src/main/java/io/opentelemetry/api/trace/DefaultTracer.java
View File

@ -7,7 +7,7 @@ package io.opentelemetry.api.trace;
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 @@ final class DefaultTracer implements Tracer {
@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();

8
api/all/src/main/java/io/opentelemetry/api/trace/Span.java
View File

@ -10,7 +10,7 @@ import static java.util.concurrent.TimeUnit.SECONDS;
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 @@ public interface Span extends ImplicitContextKeyed {
*/
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 @@ public interface Span extends ImplicitContextKeyed {
@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 @@ public interface Span extends ImplicitContextKeyed {
*/
static Span wrap(SpanContext spanContext) {
if (spanContext == null) {
ValidationUtil.log("context is null");
ApiUsageLogger.log("context is null");
return getInvalid();
}
if (!spanContext.isValid()) {

4
api/all/src/main/java/io/opentelemetry/api/trace/SpanId.java
View File

@ -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 final class 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);

4
api/all/src/main/java/io/opentelemetry/api/trace/TraceId.java
View File

@ -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 final class 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);

24
api/all/src/test/java/io/opentelemetry/api/internal/ApiUsageLoggerTest.java Normal file
View File

@ -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");
}
}

103
api/all/src/test/java/io/opentelemetry/api/internal/ValidationUtilTest.java
View File

@ -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();
}
}

95
api/all/src/test/java/io/opentelemetry/api/metrics/DefaultMeterTest.java
View File

@ -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.context.Context;
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() {

5
sdk/logs/src/test/java/io/opentelemetry/sdk/logs/SdkLoggerTest.java
View File

@ -9,7 +9,7 @@ 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.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 @@ import org.junit.jupiter.api.extension.RegisterExtension;
class SdkLoggerTest {
@RegisterExtension
LogCapturer apiUsageLogs = LogCapturer.create().captureForLogger(API_USAGE_LOGGER_NAME);
@RegisterExtension LogCapturer apiUsageLogs = LogCapturer.create().captureForLogger(LOGGER_NAME);
@Test
void logRecordBuilder() {

37
sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/AbstractInstrumentBuilder.java
View File

@ -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.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;
@ -52,7 +57,7 @@ abstract class AbstractInstrumentBuilder<BuilderT extends AbstractInstrumentBuil
protected abstract BuilderT getThis();
public BuilderT setUnit(String unit) {
if (!ValidationUtil.checkValidInstrumentUnit(
if (!checkValidInstrumentUnit(
unit,
" Using \"" + DEFAULT_UNIT + "\" for instrument " + this.instrumentName + " instead.")) {
this.unit = DEFAULT_UNIT;
@ -115,6 +120,34 @@ 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(

15
sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/SdkMeter.java
View File

@ -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 @@ final class SdkMeter implements Meter {
@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 @@ final class SdkMeter implements Meter {
@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 @@ final class SdkMeter implements Meter {
@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 @@ final class SdkMeter implements Meter {
@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);
}

63
sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/InstrumentNameValidator.java Normal file
View File

@ -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.
*
* <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() {}
}

38
sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/AbstractInstrumentBuilderTest.java
View File

@ -7,6 +7,8 @@ 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;
@ -15,9 +17,15 @@ 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");
@ -44,6 +52,36 @@ 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> {

25
sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/SdkMeterTest.java
View File

@ -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.metrics.internal.state.MetricStorageRegistry;
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 @@ class SdkMeterTest {
.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 @@ class SdkMeterTest {
// 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

74
sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/internal/InstrumentNameValidatorTest.java Normal file
View File

@ -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();
}
}