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

Set unit to 1 if invalid (#4384) 

* Set unit to 1 if invalid

* Improve error message, validate unit in DefaultMeter calls

* Fix typos

* Include stack trace
This commit is contained in:
jack-berg 2022-04-19 10:45:55 -05:00 committed by GitHub
parent a547e2a1ab
commit c2d8f6abd9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 221 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -5,6 +5,7 @@
package io.opentelemetry.api.internal;
import java.nio.charset.StandardCharsets;
import java.util.logging.Level;
import java.util.logging.Logger;
import javax.annotation.concurrent.Immutable;
@ -18,12 +19,52 @@ import javax.annotation.concurrent.Immutable;
@Immutable
public final class ValidationUtil {
private ValidationUtil() {}
public static final String API_USAGE_LOGGER_NAME = "io.opentelemetry.ApiUsageLogging";
private static final Logger API_USAGE_LOGGER =
Logger.getLogger("io.opentelemetry.ApiUsageLogging");
private static final Logger API_USAGE_LOGGER = Logger.getLogger(API_USAGE_LOGGER_NAME);
public static void log(String msg) {
API_USAGE_LOGGER.log(Level.FINEST, msg, new AssertionError());
/**
* 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) {
API_USAGE_LOGGER.log(level, message, new AssertionError());
}
/** Determine if the instrument unit is valid. If invalid, log a warning. */
public static boolean isValidInstrumentUnit(String unit) {
return isValidInstrumentUnit(unit, "");
}
/**
* Determine if the instrument unit is valid. If invalid, log a warning with the {@code logSuffix}
* appended.
*/
public static boolean isValidInstrumentUnit(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 less ASCII characters."
+ logSuffix,
Level.WARNING);
return false;
}
private ValidationUtil() {}
}

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

@ -6,6 +6,7 @@
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;
@ -106,6 +107,7 @@ class DefaultMeter implements Meter {
@Override
public LongCounterBuilder setUnit(String unit) {
ValidationUtil.isValidInstrumentUnit(unit);
return this;
}
@ -137,6 +139,7 @@ class DefaultMeter implements Meter {
@Override
public DoubleCounterBuilder setUnit(String unit) {
ValidationUtil.isValidInstrumentUnit(unit);
return this;
}
@ -188,6 +191,7 @@ class DefaultMeter implements Meter {
@Override
public LongUpDownCounterBuilder setUnit(String unit) {
ValidationUtil.isValidInstrumentUnit(unit);
return this;
}
@ -221,6 +225,7 @@ class DefaultMeter implements Meter {
@Override
public DoubleUpDownCounterBuilder setUnit(String unit) {
ValidationUtil.isValidInstrumentUnit(unit);
return this;
}
@ -270,6 +275,7 @@ class DefaultMeter implements Meter {
@Override
public DoubleHistogramBuilder setUnit(String unit) {
ValidationUtil.isValidInstrumentUnit(unit);
return this;
}
@ -294,6 +300,7 @@ class DefaultMeter implements Meter {
@Override
public LongHistogramBuilder setUnit(String unit) {
ValidationUtil.isValidInstrumentUnit(unit);
return this;
}
@ -314,6 +321,7 @@ class DefaultMeter implements Meter {
@Override
public DoubleGaugeBuilder setUnit(String unit) {
ValidationUtil.isValidInstrumentUnit(unit);
return this;
}
@ -338,6 +346,7 @@ class DefaultMeter implements Meter {
@Override
public LongGaugeBuilder setUnit(String unit) {
ValidationUtil.isValidInstrumentUnit(unit);
return this;
}

47
api/all/src/test/java/io/opentelemetry/api/internal/ValidationUtilTest.java Normal file
View File

@ -0,0 +1,47 @@
/*
* 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 isValidInstrumentName_InvalidNameLogs() {
assertThat(ValidationUtil.isValidInstrumentUnit("日", " suffix")).isFalse();
apiUsageLogs.assertContains(
"Unit \"日\" is invalid. Instrument unit must be 63 or less ASCII characters." + " suffix");
}
@Test
void isValidInstrumentUnit() {
assertThat(ValidationUtil.isValidInstrumentUnit("a")).isTrue();
assertThat(ValidationUtil.isValidInstrumentUnit("A")).isTrue();
assertThat(ValidationUtil.isValidInstrumentUnit("foo129")).isTrue();
assertThat(ValidationUtil.isValidInstrumentUnit("!@#$%^&*()")).isTrue();
assertThat(ValidationUtil.isValidInstrumentUnit(new String(new char[63]).replace('\0', 'a')))
.isTrue();
// Empty and null not allowed
assertThat(ValidationUtil.isValidInstrumentUnit(null)).isFalse();
assertThat(ValidationUtil.isValidInstrumentUnit("")).isFalse();
// Non-ascii characters
assertThat(ValidationUtil.isValidInstrumentUnit("日")).isFalse();
// Must be 63 characters or less
assertThat(ValidationUtil.isValidInstrumentUnit(new String(new char[64]).replace('\0', 'a')))
.isFalse();
}
}

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

@ -6,18 +6,64 @@
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 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;
import org.slf4j.event.LoggingEvent;
@SuppressLogger(loggerName = API_USAGE_LOGGER_NAME)
public class DefaultMeterTest {
private static final Meter meter = DefaultMeter.getInstance();
private static final Meter METER = DefaultMeter.getInstance();
@RegisterExtension
LogCapturer apiUsageLogs = LogCapturer.create().captureForLogger(API_USAGE_LOGGER_NAME);
@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 less ASCII characters."));
}
@Test
void noopLongCounter_doesNotThrow() {
LongCounter counter =
meter.counterBuilder("size").setDescription("The size I'm measuring").setUnit("1").build();
METER.counterBuilder("size").setDescription("The size I'm measuring").setUnit("1").build();
counter.add(1);
counter.add(1, Attributes.of(stringKey("thing"), "car"));
counter.add(1, Attributes.of(stringKey("thing"), "car"), Context.current());
@ -26,7 +72,7 @@ public class DefaultMeterTest {
@Test
void noopDoubleCounter_doesNotThrow() {
DoubleCounter counter =
meter
METER
.counterBuilder("size")
.ofDoubles()
.setDescription("The size I'm measuring")
@ -40,7 +86,7 @@ public class DefaultMeterTest {
@Test
void noopLongUpDownCounter_doesNotThrow() {
LongUpDownCounter counter =
meter
METER
.upDownCounterBuilder("size")
.setDescription("The size I'm measuring")
.setUnit("1")
@ -53,7 +99,7 @@ public class DefaultMeterTest {
@Test
void noopDoubleUpDownCounter_doesNotThrow() {
DoubleUpDownCounter counter =
meter
METER
.upDownCounterBuilder("size")
.ofDoubles()
.setDescription("The size I'm measuring")
@ -67,7 +113,7 @@ public class DefaultMeterTest {
@Test
void noopLongHistogram_doesNotThrow() {
LongHistogram histogram =
meter
METER
.histogramBuilder("size")
.ofLongs()
.setDescription("The size I'm measuring")
@ -81,7 +127,7 @@ public class DefaultMeterTest {
@Test
void noopDoubleHistogram_doesNotThrow() {
DoubleHistogram histogram =
meter
METER
.histogramBuilder("size")
.setDescription("The size I'm measuring")
.setUnit("1")
@ -93,7 +139,7 @@ public class DefaultMeterTest {
@Test
void noopObservableLongGauage_doesNotThrow() {
meter
METER
.gaugeBuilder("temperature")
.ofLongs()
.setDescription("The current temperature")
@ -107,7 +153,7 @@ public class DefaultMeterTest {
@Test
void noopObservableDoubleGauage_doesNotThrow() {
meter
METER
.gaugeBuilder("temperature")
.setDescription("The current temperature")
.setUnit("C")
@ -120,7 +166,7 @@ public class DefaultMeterTest {
@Test
void noopObservableLongCounter_doesNotThrow() {
meter
METER
.counterBuilder("temperature")
.setDescription("The current temperature")
.setUnit("C")
@ -133,7 +179,7 @@ public class DefaultMeterTest {
@Test
void noopObservableDoubleCounter_doesNotThrow() {
meter
METER
.counterBuilder("temperature")
.ofDoubles()
.setDescription("The current temperature")
@ -147,7 +193,7 @@ public class DefaultMeterTest {
@Test
void noopObservableLongUpDownCounter_doesNotThrow() {
meter
METER
.upDownCounterBuilder("temperature")
.setDescription("The current temperature")
.setUnit("C")
@ -160,7 +206,7 @@ public class DefaultMeterTest {
@Test
void noopObservableDoubleUpDownCounter_doesNotThrow() {
meter
METER
.upDownCounterBuilder("temperature")
.ofDoubles()
.setDescription("The current temperature")

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

@ -5,6 +5,7 @@
package io.opentelemetry.sdk.metrics;
import io.opentelemetry.api.internal.ValidationUtil;
import io.opentelemetry.api.metrics.ObservableDoubleMeasurement;
import io.opentelemetry.api.metrics.ObservableLongMeasurement;
import io.opentelemetry.sdk.metrics.internal.descriptor.InstrumentDescriptor;
@ -18,6 +19,8 @@ import java.util.function.Consumer;
/** Helper to make implementing builders easier. */
abstract class AbstractInstrumentBuilder<BuilderT extends AbstractInstrumentBuilder<?>> {
static final String DEFAULT_UNIT = "1";
private final MeterProviderSharedState meterProviderSharedState;
private String description;
private String unit;
@ -41,7 +44,12 @@ abstract class AbstractInstrumentBuilder<BuilderT extends AbstractInstrumentBuil
protected abstract BuilderT getThis();
public BuilderT setUnit(String unit) {
this.unit = unit;
if (!ValidationUtil.isValidInstrumentUnit(
unit, " Using " + DEFAULT_UNIT + " for instrument " + this.instrumentName + " instead.")) {
this.unit = DEFAULT_UNIT;
} else {
this.unit = unit;
}
return getThis();
}

2
sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/SdkDoubleGaugeBuilder.java
View File

@ -20,7 +20,7 @@ final class SdkDoubleGaugeBuilder extends AbstractInstrumentBuilder<SdkDoubleGau
MeterProviderSharedState meterProviderSharedState,
MeterSharedState meterSharedState,
String name) {
this(meterProviderSharedState, meterSharedState, name, "", "1");
this(meterProviderSharedState, meterSharedState, name, "", DEFAULT_UNIT);
}
SdkDoubleGaugeBuilder(

2
sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/SdkDoubleHistogram.java
View File

@ -102,7 +102,7 @@ final class SdkDoubleHistogram extends AbstractInstrument implements DoubleHisto
MeterProviderSharedState meterProviderSharedState,
MeterSharedState meterSharedState,
String name) {
this(meterProviderSharedState, meterSharedState, name, "", "1");
this(meterProviderSharedState, meterSharedState, name, "", DEFAULT_UNIT);
}
Builder(

2
sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/SdkLongCounter.java
View File

@ -106,7 +106,7 @@ final class SdkLongCounter extends AbstractInstrument implements LongCounter {
MeterProviderSharedState meterProviderSharedState,
MeterSharedState meterSharedState,
String name) {
this(meterProviderSharedState, meterSharedState, name, "", "1");
this(meterProviderSharedState, meterSharedState, name, "", DEFAULT_UNIT);
}
Builder(

2
sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/SdkLongUpDownCounter.java
View File

@ -80,7 +80,7 @@ final class SdkLongUpDownCounter extends AbstractInstrument implements LongUpDow
MeterProviderSharedState meterProviderSharedState,
MeterSharedState meterSharedState,
String name) {
this(meterProviderSharedState, meterSharedState, name, "", "1");
this(meterProviderSharedState, meterSharedState, name, "", DEFAULT_UNIT);
}
Builder(

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

@ -5,6 +5,7 @@
package io.opentelemetry.sdk.metrics;
import static io.opentelemetry.api.internal.ValidationUtil.API_USAGE_LOGGER_NAME;
import static org.assertj.core.api.Assertions.assertThat;
import io.github.netmikey.logunit.api.LogCapturer;
@ -20,9 +21,12 @@ 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(MetricStorageRegistry.class)
class SdkMeterTest {
// Meter must have an exporter configured to actual run.
private final SdkMeterProvider testMeterProvider =
SdkMeterProvider.builder().registerMetricReader(InMemoryMetricReader.create()).build();
@ -31,6 +35,49 @@ class SdkMeterTest {
@RegisterExtension
LogCapturer logs = LogCapturer.create().captureForType(MetricStorageRegistry.class);
@RegisterExtension
LogCapturer apiUsageLogs = LogCapturer.create().captureForLogger(API_USAGE_LOGGER_NAME);
@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();
// 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 less ASCII characters. Using 1 for instrument my-instrument instead."));
}
@Test
void testLongCounter() {
LongCounter longCounter =