Cleanup and additional tests for ConfigBuilder (#1296)

* Cleanup and tests for ConfigBuilder

* Add more tests

* Document mockito extension

* Reformat after rebase
This commit is contained in:
Giovanni Liva 2020-06-02 20:33:21 +02:00 committed by GitHub
parent a1bae308db
commit 4808387afb
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 269 additions and 170 deletions

View File

@ -24,7 +24,6 @@ import io.opentelemetry.sdk.metrics.data.MetricData;
import io.opentelemetry.sdk.metrics.export.MetricExporter;
import java.util.Collection;
import java.util.Map;
import java.util.Properties;
import java.util.concurrent.TimeUnit;
import java.util.logging.Level;
import java.util.logging.Logger;
@ -200,36 +199,5 @@ public final class OtlpGrpcMetricExporter implements MetricExporter {
}
return this;
}
/**
* Sets the configuration values from the given properties object for only the available keys.
*
* @param properties {@link Properties} holding the configuration values.
* @return this.
*/
@Override
public Builder readProperties(Properties properties) {
return super.readProperties(properties);
}
/**
* Sets the configuration values from environment variables for only the available keys.
*
* @return this.
*/
@Override
public Builder readEnvironmentVariables() {
return super.readEnvironmentVariables();
}
/**
* Sets the configuration values from system properties for only the available keys.
*
* @return this.
*/
@Override
public Builder readSystemProperties() {
return super.readSystemProperties();
}
}
}

View File

@ -24,7 +24,6 @@ import io.opentelemetry.sdk.trace.data.SpanData;
import io.opentelemetry.sdk.trace.export.SpanExporter;
import java.util.Collection;
import java.util.Map;
import java.util.Properties;
import java.util.concurrent.TimeUnit;
import java.util.logging.Level;
import java.util.logging.Logger;
@ -199,36 +198,5 @@ public final class OtlpGrpcSpanExporter implements SpanExporter {
}
return this;
}
/**
* Sets the configuration values from the given properties object for only the available keys.
*
* @param properties {@link Properties} holding the configuration values.
* @return this.
*/
@Override
public Builder readProperties(Properties properties) {
return super.readProperties(properties);
}
/**
* Sets the configuration values from environment variables for only the available keys.
*
* @return this.
*/
@Override
public Builder readEnvironmentVariables() {
return super.readEnvironmentVariables();
}
/**
* Sets the configuration values from system properties for only the available keys.
*
* @return this.
*/
@Override
public Builder readSystemProperties() {
return super.readSystemProperties();
}
}
}

View File

@ -28,6 +28,7 @@ import io.opentelemetry.proto.collector.metrics.v1.ExportMetricsServiceResponse;
import io.opentelemetry.proto.collector.metrics.v1.MetricsServiceGrpc;
import io.opentelemetry.proto.metrics.v1.ResourceMetrics;
import io.opentelemetry.sdk.common.InstrumentationLibraryInfo;
import io.opentelemetry.sdk.common.export.ConfigBuilder;
import io.opentelemetry.sdk.metrics.data.MetricData;
import io.opentelemetry.sdk.metrics.data.MetricData.Descriptor;
import io.opentelemetry.sdk.metrics.data.MetricData.Descriptor.Type;
@ -38,13 +39,16 @@ import io.opentelemetry.sdk.resources.Resource;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.concurrent.TimeUnit;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
import org.mockito.Mockito;
/** Unit tests for {@link OtlpGrpcMetricExporter}. */
@RunWith(JUnit4.class)
@ -56,6 +60,12 @@ public class OtlpGrpcMetricExporterTest {
private final ManagedChannel inProcessChannel =
InProcessChannelBuilder.forName(serverName).directExecutor().build();
abstract static class ConfigBuilderTest extends ConfigBuilder<ConfigBuilderTest> {
public static NamingConvention getNaming() {
return NamingConvention.DOT;
}
}
@Before
public void setup() throws IOException {
grpcCleanup.register(
@ -67,6 +77,16 @@ public class OtlpGrpcMetricExporterTest {
grpcCleanup.register(inProcessChannel);
}
@Test
public void configTest() {
Map<String, String> options = new HashMap<>();
options.put("otel.otlp.metric.timeout", "12");
OtlpGrpcMetricExporter.Builder config = OtlpGrpcMetricExporter.newBuilder();
OtlpGrpcMetricExporter.Builder spy = Mockito.spy(config);
spy.fromConfigMap(options, ConfigBuilderTest.getNaming());
Mockito.verify(spy).setDeadlineMs(12);
}
@Test
public void testExport() {
MetricData span = generateFakeMetric();

View File

@ -23,6 +23,7 @@ import io.grpc.Status.Code;
import io.grpc.inprocess.InProcessChannelBuilder;
import io.grpc.inprocess.InProcessServerBuilder;
import io.grpc.testing.GrpcCleanupRule;
import io.opentelemetry.exporters.otlp.OtlpGrpcMetricExporterTest.ConfigBuilderTest;
import io.opentelemetry.proto.collector.trace.v1.ExportTraceServiceRequest;
import io.opentelemetry.proto.collector.trace.v1.ExportTraceServiceResponse;
import io.opentelemetry.proto.collector.trace.v1.TraceServiceGrpc;
@ -38,13 +39,16 @@ import io.opentelemetry.trace.TraceId;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.concurrent.TimeUnit;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
import org.mockito.Mockito;
/** Unit tests for {@link OtlpGrpcSpanExporter}. */
@RunWith(JUnit4.class)
@ -59,6 +63,16 @@ public class OtlpGrpcSpanExporterTest {
private final ManagedChannel inProcessChannel =
InProcessChannelBuilder.forName(serverName).directExecutor().build();
@Test
public void configTest() {
Map<String, String> options = new HashMap<>();
options.put("otel.otlp.span.timeout", "12");
OtlpGrpcSpanExporter.Builder config = OtlpGrpcSpanExporter.newBuilder();
OtlpGrpcSpanExporter.Builder spy = Mockito.spy(config);
spy.fromConfigMap(options, ConfigBuilderTest.getNaming());
Mockito.verify(spy).setDeadlineMs(12);
}
@Before
public void setup() throws IOException {
grpcCleanup.register(

View File

@ -26,7 +26,6 @@ import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Properties;
import java.util.concurrent.Executors;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.TimeUnit;
@ -179,37 +178,6 @@ public final class IntervalMetricReader {
}
return this;
}
/**
* Sets the configuration values from the given properties object for only the available keys.
*
* @param properties {@link Properties} holding the configuration values.
* @return this.
*/
@Override
public Builder readProperties(Properties properties) {
return super.readProperties(properties);
}
/**
* Sets the configuration values from environment variables for only the available keys.
*
* @return this.
*/
@Override
public Builder readEnvironmentVariables() {
return super.readEnvironmentVariables();
}
/**
* Sets the configuration values from system properties for only the available keys.
*
* @return this.
*/
@Override
public Builder readSystemProperties() {
return super.readSystemProperties();
}
}
@SuppressWarnings("FutureReturnValueIgnored")

View File

@ -31,7 +31,6 @@ import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Properties;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
@ -373,47 +372,6 @@ public final class BatchSpanProcessor implements SpanProcessor {
return this;
}
/**
* Sets the configuration values from the given properties object for only the available keys.
*
* @param properties {@link Properties} holding the configuration values.
* @return this.
*/
@Override
public Builder readProperties(Properties properties) {
return super.readProperties(properties);
}
/**
* Sets the configuration values from environment variables for only the available keys. This
* method looks for the following keys:
*
* <ul>
* <li>{@code OTEL_BSP_SCHEDULE_DELAY}: to set the delay interval between two consecutive
* exports.
* <li>{@code OTEL_BSP_MAX_QUEUE}: to set the maximum queue size.
* <li>{@code OTEL_BSP_MAX_EXPORT_BATCH}: to set the maximum batch size.
* <li>{@code OTEL_BSP_EXPORT_TIMEOUT}: to set the maximum allowed time to export data.
* <li>{@code OTEL_BSP_EXPORT_SAMPLED}: to set whether only sampled spans should be exported.
* </ul>
*
* @return this.
*/
@Override
public Builder readEnvironmentVariables() {
return super.readEnvironmentVariables();
}
/**
* Sets the configuration values from system properties for only the available keys.
*
* @return this.
*/
@Override
public Builder readSystemProperties() {
return super.readSystemProperties();
}
// TODO: Consider to add support for constant Attributes and/or Resource.
/**

View File

@ -25,7 +25,6 @@ import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Properties;
import java.util.logging.Level;
import java.util.logging.Logger;
@ -146,37 +145,6 @@ public final class SimpleSpanProcessor implements SpanProcessor {
return this;
}
/**
* Sets the configuration values from the given properties object for only the available keys.
*
* @param properties {@link Properties} holding the configuration values.
* @return this.
*/
@Override
public Builder readProperties(Properties properties) {
return super.readProperties(properties);
}
/**
* Sets the configuration values from environment variables for only the available keys.
*
* @return this.
*/
@Override
public Builder readEnvironmentVariables() {
return super.readEnvironmentVariables();
}
/**
* Sets the configuration values from system properties for only the available keys.
*
* @return this.
*/
@Override
public Builder readSystemProperties() {
return super.readSystemProperties();
}
/**
* Set whether only sampled spans should be exported.
*

View File

@ -20,8 +20,12 @@ import static com.google.common.truth.Truth.assertThat;
import io.opentelemetry.sdk.common.export.ConfigBuilder.NamingConvention;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import org.junit.Rule;
import org.junit.Test;
import org.junit.contrib.java.lang.system.EnvironmentVariables;
import org.junit.contrib.java.lang.system.ProvideSystemProperty;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
@ -123,4 +127,174 @@ public class ConfigBuilderTest {
ConfigBuilder.getDoubleProperty("double", Collections.<String, String>emptyMap());
assertThat(doubleProperty).isNull();
}
@Test
public void testNormalize_dot() {
assertThat(NamingConvention.DOT.normalize("lower.case")).isEqualTo("lower.case");
assertThat(NamingConvention.DOT.normalize("lower_case")).isEqualTo("lower_case");
assertThat(NamingConvention.DOT.normalize("loWer.cAsE")).isEqualTo("lower.case");
assertThat(NamingConvention.DOT.normalize("loWer_cAsE")).isEqualTo("lower_case");
}
@Test
public void testNormalize_env() {
assertThat(NamingConvention.ENV_VAR.normalize("lower.case")).isEqualTo("lower.case");
assertThat(NamingConvention.ENV_VAR.normalize("lower_case")).isEqualTo("lower.case");
assertThat(NamingConvention.ENV_VAR.normalize("loWer.cAsE")).isEqualTo("lower.case");
assertThat(NamingConvention.ENV_VAR.normalize("loWer_cAsE")).isEqualTo("lower.case");
}
@Test
public void testNormalize_dotMap() {
Map<String, String> map = new HashMap<>();
map.put("lower.case", "1");
map.put("lower_case", "2");
map.put("loWer.cAsE", "3");
map.put("loWer_cAsE", "4");
Map<String, String> normalized = NamingConvention.DOT.normalize(map);
assertThat(normalized.size()).isEqualTo(2);
assertThat(normalized).containsExactly("lower.case", "3", "lower_case", "4");
}
@Test
public void testNormalize_envMap() {
Map<String, String> map = new HashMap<>();
map.put("lower.case", "1");
map.put("lower_case", "2");
map.put("loWer.cAsE", "3");
map.put("loWer_cAsE", "4");
Map<String, String> normalized = NamingConvention.ENV_VAR.normalize(map);
assertThat(normalized.size()).isEqualTo(1);
assertThat(normalized).containsExactly("lower.case", "3");
}
@Test
public void testBoolProperty() {
Map<String, String> map = new HashMap<>();
map.put("int", "1");
map.put("long", "2L");
map.put("boolt", "true");
map.put("boolf", "false");
map.put("string", "random");
assertThat(ConfigBuilder.getBooleanProperty("int", map)).isFalse();
assertThat(ConfigBuilder.getBooleanProperty("long", map)).isFalse();
assertThat(ConfigBuilder.getBooleanProperty("boolt", map)).isTrue();
assertThat(ConfigBuilder.getBooleanProperty("boolf", map)).isFalse();
assertThat(ConfigBuilder.getBooleanProperty("string", map)).isFalse();
assertThat(ConfigBuilder.getBooleanProperty("no-key", map)).isNull();
}
@Test
public void testIntProperty() {
Map<String, String> map = new HashMap<>();
map.put("int", "1");
map.put("long", "2L");
map.put("boolt", "true");
map.put("boolf", "false");
map.put("string", "random");
assertThat(ConfigBuilder.getIntProperty("int", map)).isNotNull();
assertThat(ConfigBuilder.getIntProperty("int", map)).isEqualTo(1);
assertThat(ConfigBuilder.getIntProperty("long", map)).isNull();
assertThat(ConfigBuilder.getIntProperty("boolt", map)).isNull();
assertThat(ConfigBuilder.getIntProperty("boolf", map)).isNull();
assertThat(ConfigBuilder.getIntProperty("string", map)).isNull();
assertThat(ConfigBuilder.getIntProperty("no-key", map)).isNull();
}
@Test
public void testLongProperty() {
Map<String, String> map = new HashMap<>();
map.put("int", "1");
map.put("long", "2L");
map.put("boolt", "true");
map.put("boolf", "false");
map.put("string", "random");
assertThat(ConfigBuilder.getLongProperty("int", map)).isNotNull();
assertThat(ConfigBuilder.getLongProperty("int", map)).isEqualTo(1);
assertThat(ConfigBuilder.getLongProperty("long", map)).isNull();
assertThat(ConfigBuilder.getLongProperty("boolt", map)).isNull();
assertThat(ConfigBuilder.getLongProperty("boolf", map)).isNull();
assertThat(ConfigBuilder.getLongProperty("string", map)).isNull();
assertThat(ConfigBuilder.getLongProperty("no-key", map)).isNull();
}
@Test
public void testStringProperty() {
Map<String, String> map = new HashMap<>();
map.put("int", "1");
map.put("long", "2L");
map.put("boolt", "true");
map.put("boolf", "false");
map.put("string", "random");
assertThat(ConfigBuilder.getStringProperty("int", map)).isNotNull();
assertThat(ConfigBuilder.getStringProperty("long", map)).isNotNull();
assertThat(ConfigBuilder.getStringProperty("boolt", map)).isNotNull();
assertThat(ConfigBuilder.getStringProperty("boolf", map)).isNotNull();
assertThat(ConfigBuilder.getStringProperty("string", map)).isNotNull();
assertThat(ConfigBuilder.getStringProperty("no-key", map)).isNull();
}
public static final class ConfigTester extends ConfigBuilder<Map<String, String>> {
public static NamingConvention getNamingDot() {
return NamingConvention.DOT;
}
public static NamingConvention getNamingEnv() {
return NamingConvention.ENV_VAR;
}
@Override
protected Map<String, String> fromConfigMap(
Map<String, String> configMap, NamingConvention namingConvention) {
return configMap;
}
}
@RunWith(JUnit4.class)
public static class ConfigurationSystemPropertiesTest {
@Rule
public final ProvideSystemProperty systemProperties =
new ProvideSystemProperty("int", "1")
.and("long", "2L")
.and("boolt", "true")
.and("boolf", "false")
.and("string", "random");
@Test
public void testSystemProperties() {
ConfigTester config = new ConfigTester();
Map<String, String> map = config.readSystemProperties();
assertThat(ConfigBuilder.getStringProperty("int", map)).isEqualTo("1");
assertThat(ConfigBuilder.getStringProperty("long", map)).isEqualTo("2L");
assertThat(ConfigBuilder.getStringProperty("boolt", map)).isEqualTo("true");
assertThat(ConfigBuilder.getStringProperty("boolf", map)).isEqualTo("false");
assertThat(ConfigBuilder.getStringProperty("string", map)).isEqualTo("random");
assertThat(ConfigBuilder.getStringProperty("no-key", map)).isNull();
}
}
@RunWith(JUnit4.class)
public static class ConfigurationEnvVarsTest {
@Rule
public final EnvironmentVariables environmentVariables =
new EnvironmentVariables()
.set("int", "1")
.set("long", "2L")
.set("boolt", "true")
.set("boolf", "false")
.set("string", "random");
@Test
public void testEnvironmentVariables() {
ConfigTester config = new ConfigTester();
Map<String, String> map = config.readEnvironmentVariables();
assertThat(ConfigBuilder.getStringProperty("int", map)).isEqualTo("1");
assertThat(ConfigBuilder.getStringProperty("long", map)).isEqualTo("2L");
assertThat(ConfigBuilder.getStringProperty("boolt", map)).isEqualTo("true");
assertThat(ConfigBuilder.getStringProperty("boolf", map)).isEqualTo("false");
assertThat(ConfigBuilder.getStringProperty("string", map)).isEqualTo("random");
assertThat(ConfigBuilder.getStringProperty("no-key", map)).isNull();
}
}
}

View File

@ -20,6 +20,7 @@ import static com.google.common.truth.Truth.assertThat;
import static org.mockito.Mockito.when;
import io.opentelemetry.sdk.common.InstrumentationLibraryInfo;
import io.opentelemetry.sdk.common.export.ConfigBuilderTest.ConfigTester;
import io.opentelemetry.sdk.metrics.data.MetricData;
import io.opentelemetry.sdk.metrics.data.MetricData.Descriptor.Type;
import io.opentelemetry.sdk.metrics.data.MetricData.LongPoint;
@ -28,13 +29,16 @@ import io.opentelemetry.sdk.resources.Resource;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.concurrent.atomic.AtomicBoolean;
import javax.annotation.Nullable;
import javax.annotation.concurrent.GuardedBy;
import org.junit.Before;
import org.junit.Test;
import org.mockito.Mock;
import org.mockito.Mockito;
import org.mockito.MockitoAnnotations;
/** Unit tests for {@link IntervalMetricReader}. */
@ -66,6 +70,16 @@ public class IntervalMetricReaderTest {
when(metricProducer.getAllMetrics()).thenReturn(Collections.singletonList(METRIC_DATA));
}
@Test
public void configTest() {
Map<String, String> options = new HashMap<>();
options.put("otel.imr.export.interval", "12");
IntervalMetricReader.Builder config = IntervalMetricReader.builder();
IntervalMetricReader.Builder spy = Mockito.spy(config);
spy.fromConfigMap(options, ConfigTester.getNamingDot());
Mockito.verify(spy).setExportIntervalMillis(12);
}
@Test
public void intervalExport() {
WaitingMetricExporter waitingMetricExporter = new WaitingMetricExporter();

View File

@ -20,6 +20,7 @@ import static com.google.common.truth.Truth.assertThat;
import static org.mockito.Mockito.doThrow;
import io.opentelemetry.sdk.common.export.ConfigBuilder;
import io.opentelemetry.sdk.common.export.ConfigBuilderTest.ConfigTester;
import io.opentelemetry.sdk.trace.ReadableSpan;
import io.opentelemetry.sdk.trace.Samplers;
import io.opentelemetry.sdk.trace.TestUtils;
@ -29,7 +30,9 @@ import io.opentelemetry.trace.Tracer;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;
@ -42,6 +45,7 @@ import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
import org.mockito.ArgumentMatchers;
import org.mockito.Mock;
import org.mockito.Mockito;
import org.mockito.MockitoAnnotations;
/** Unit tests for {@link BatchSpanProcessor}. */
@ -101,6 +105,29 @@ public class BatchSpanProcessorTest {
.end();
}
@Test
public void configTest() {
Map<String, String> options = new HashMap<>();
options.put("otel.bsp.schedule.delay", "12");
options.put("otel.bsp.max.queue", "34");
options.put("otel.bsp.max.export.batch", "56");
options.put("otel.bsp.export.timeout", "78");
options.put("otel.bsp.export.sampled", "false");
BatchSpanProcessor.Builder config = BatchSpanProcessor.newBuilder(new WaitingSpanExporter(0));
/*
We are trying to spy a final class. To allow this, we need to create a resource file
./mockito-extensions/org.mockito.plugins.MockMaker with content "mock-maker-inline".
https://www.javadoc.io/doc/org.mockito/mockito-core/latest/org/mockito/plugins/MockMaker.html
*/
BatchSpanProcessor.Builder spy = Mockito.spy(config);
spy.fromConfigMap(options, ConfigTester.getNamingDot()).build();
Mockito.verify(spy).setScheduleDelayMillis(12);
Mockito.verify(spy).setMaxQueueSize(34);
Mockito.verify(spy).setMaxExportBatchSize(56);
Mockito.verify(spy).setExporterTimeoutMillis(78);
Mockito.verify(spy).setExportOnlySampled(false);
}
@Test
public void startEndRequirements() {
BatchSpanProcessor spansProcessor =

View File

@ -22,6 +22,7 @@ import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoInteractions;
import static org.mockito.Mockito.when;
import io.opentelemetry.sdk.common.export.ConfigBuilderTest.ConfigTester;
import io.opentelemetry.sdk.trace.ReadableSpan;
import io.opentelemetry.sdk.trace.Samplers;
import io.opentelemetry.sdk.trace.TestUtils;
@ -35,13 +36,16 @@ import io.opentelemetry.trace.TraceId;
import io.opentelemetry.trace.TraceState;
import io.opentelemetry.trace.Tracer;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Properties;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
import org.mockito.Mock;
import org.mockito.Mockito;
import org.mockito.MockitoAnnotations;
/** Unit tests for {@link SimpleSpanProcessor}. */
@ -69,6 +73,21 @@ public class SimpleSpanProcessorTest {
simpleSampledSpansProcessor = SimpleSpanProcessor.newBuilder(spanExporter).build();
}
@Test
public void configTest() {
Map<String, String> options = new HashMap<>();
options.put("otel.ssp.export.sampled", "false");
SimpleSpanProcessor.Builder config = SimpleSpanProcessor.newBuilder(spanExporter);
/*
We are trying to spy a final class. To allow this, we need to create a resource file
./mockito-extensions/org.mockito.plugins.MockMaker with content "mock-maker-inline".
https://www.javadoc.io/doc/org.mockito/mockito-core/latest/org/mockito/plugins/MockMaker.html
*/
SimpleSpanProcessor.Builder spy = Mockito.spy(config);
spy.fromConfigMap(options, ConfigTester.getNamingDot());
Mockito.verify(spy).setExportOnlySampled(false);
}
@Test
public void onStartSync() {
simpleSampledSpansProcessor.onStart(readableSpan);

View File

@ -0,0 +1 @@
mock-maker-inline