Skip to content

Commit

Permalink
Avoid mockito magic for span processors test, test with empty configm…
Browse files Browse the repository at this point in the history 
…ap (#1309)

Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
  • Loading branch information
@bogdandrutu
bogdandrutu authored Jun 4, 2020
1 parent 37b64df commit 643d25d
Show file tree
Hide file tree
Showing 4 changed files with 83 additions and 41 deletions.
42 changes: 33 additions & 9 deletions sdk/src/main/java/io/opentelemetry/sdk/trace/export/BatchSpanProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -321,11 +321,11 @@ public static final class Builder extends ConfigBuilder<Builder> {
private static final String KEY_EXPORT_TIMEOUT_MILLIS = "otel.bsp.export.timeout";
private static final String KEY_SAMPLED = "otel.bsp.export.sampled";

private static final long DEFAULT_SCHEDULE_DELAY_MILLIS = 5000;
private static final int DEFAULT_MAX_QUEUE_SIZE = 2048;
private static final int DEFAULT_MAX_EXPORT_BATCH_SIZE = 512;
private static final int DEFAULT_EXPORT_TIMEOUT_MILLIS = 30_000;
private static final boolean DEFAULT_EXPORT_ONLY_SAMPLED = true;
@VisibleForTesting static final long DEFAULT_SCHEDULE_DELAY_MILLIS = 5000;
@VisibleForTesting static final int DEFAULT_MAX_QUEUE_SIZE = 2048;
@VisibleForTesting static final int DEFAULT_MAX_EXPORT_BATCH_SIZE = 512;
@VisibleForTesting static final int DEFAULT_EXPORT_TIMEOUT_MILLIS = 30_000;
@VisibleForTesting static final boolean DEFAULT_EXPORT_ONLY_SAMPLED = true;

private final SpanExporter spanExporter;
private long scheduleDelayMillis = DEFAULT_SCHEDULE_DELAY_MILLIS;
Expand All @@ -344,7 +344,6 @@ private Builder(SpanExporter spanExporter) {
* @param configMap {@link Map} holding the configuration values.
* @return this.
*/
@VisibleForTesting
@Override
protected Builder fromConfigMap(
Map<String, String> configMap, Builder.NamingConvention namingConvention) {
Expand Down Expand Up @@ -379,15 +378,20 @@ protected Builder fromConfigMap(
*
* <p>Default value is {@code true}.
*
* @param sampled report only sampled spans.
* @param exportOnlySampled if {@code true} report only sampled spans.
* @return this.
* @see BatchSpanProcessor.Builder#DEFAULT_EXPORT_ONLY_SAMPLED
*/
public Builder setExportOnlySampled(boolean sampled) {
this.exportOnlySampled = sampled;
public Builder setExportOnlySampled(boolean exportOnlySampled) {
this.exportOnlySampled = exportOnlySampled;
return this;
}

@VisibleForTesting
boolean getExportOnlySampled() {
return exportOnlySampled;
}

/**
* Sets the delay interval between two consecutive exports. The actual interval may be shorter
* if the batch size is getting larger than {@code maxQueuedSpans / 2}.
Expand All @@ -403,6 +407,11 @@ public Builder setScheduleDelayMillis(long scheduleDelayMillis) {
return this;
}

@VisibleForTesting
long getScheduleDelayMillis() {
return scheduleDelayMillis;
}

/**
* Sets the maximum time an exporter will be allowed to run before being cancelled.
*
Expand All @@ -417,6 +426,11 @@ public Builder setExporterTimeoutMillis(int exporterTimeoutMillis) {
return this;
}

@VisibleForTesting
int getExporterTimeoutMillis() {
return exporterTimeoutMillis;
}

/**
* Sets the maximum number of Spans that are kept in the queue before start dropping.
*
Expand All @@ -435,6 +449,11 @@ public Builder setMaxQueueSize(int maxQueueSize) {
return this;
}

@VisibleForTesting
int getMaxQueueSize() {
return maxQueueSize;
}

/**
* Sets the maximum batch size for every export. This must be smaller or equal to {@code
* maxQueuedSpans}.
Expand All @@ -451,6 +470,11 @@ public Builder setMaxExportBatchSize(int maxExportBatchSize) {
return this;
}

@VisibleForTesting
int getMaxExportBatchSize() {
return maxExportBatchSize;
}

/**
* Returns a new {@link BatchSpanProcessor} that batches, then converts spans to proto and
* forwards them to the given {@code spanExporter}.
Expand Down
19 changes: 11 additions & 8 deletions sdk/src/main/java/io/opentelemetry/sdk/trace/export/SimpleSpanProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,9 +114,9 @@ public static final class Builder extends ConfigBuilder<Builder> {

private static final String KEY_SAMPLED = "otel.ssp.export.sampled";

private static final boolean DEFAULT_EXPORT_ONLY_SAMPLED = true;
@VisibleForTesting static final boolean DEFAULT_EXPORT_ONLY_SAMPLED = true;
private final SpanExporter spanExporter;
private boolean sampled = DEFAULT_EXPORT_ONLY_SAMPLED;
private boolean exportOnlySampled = DEFAULT_EXPORT_ONLY_SAMPLED;

private Builder(SpanExporter spanExporter) {
this.spanExporter = Objects.requireNonNull(spanExporter, "spanExporter");
Expand All @@ -133,7 +133,6 @@ private Builder(SpanExporter spanExporter) {
* @param configMap {@link Map} holding the configuration values.
* @return this.
*/
@VisibleForTesting
@Override
protected Builder fromConfigMap(
Map<String, String> configMap, NamingConvention namingConvention) {
Expand All @@ -150,15 +149,19 @@ protected Builder fromConfigMap(
*
* <p>Default value is {@code true}.
*
* @see SimpleSpanProcessor.Builder#DEFAULT_EXPORT_ONLY_SAMPLED
* @param sampled report only sampled spans.
* @param exportOnlySampled if {@code true} report only sampled spans.
* @return this.
*/
public Builder setExportOnlySampled(boolean sampled) {
this.sampled = sampled;
public Builder setExportOnlySampled(boolean exportOnlySampled) {
this.exportOnlySampled = exportOnlySampled;
return this;
}

@VisibleForTesting
boolean getExportOnlySampled() {
return exportOnlySampled;
}

// TODO: Add metrics for total exported spans.
// TODO: Consider to add support for constant Attributes and/or Resource.

Expand All @@ -170,7 +173,7 @@ public Builder setExportOnlySampled(boolean sampled) {
* @throws NullPointerException if the {@code spanExporter} is {@code null}.
*/
public SimpleSpanProcessor build() {
return new SimpleSpanProcessor(spanExporter, sampled);
return new SimpleSpanProcessor(spanExporter, exportOnlySampled);
}
}
}
40 changes: 26 additions & 14 deletions sdk/src/test/java/io/opentelemetry/sdk/trace/export/BatchSpanProcessorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
Expand All @@ -45,7 +46,6 @@
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}. */
Expand Down Expand Up @@ -113,19 +113,31 @@ public void configTest() {
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);
BatchSpanProcessor.Builder config =
BatchSpanProcessor.newBuilder(new WaitingSpanExporter(0))
.fromConfigMap(options, ConfigTester.getNamingDot());
assertThat(config.getScheduleDelayMillis()).isEqualTo(12);
assertThat(config.getMaxQueueSize()).isEqualTo(34);
assertThat(config.getMaxExportBatchSize()).isEqualTo(56);
assertThat(config.getExporterTimeoutMillis()).isEqualTo(78);
assertThat(config.getExportOnlySampled()).isEqualTo(false);
}

@Test
public void configTest_EmptyOptions() {
BatchSpanProcessor.Builder config =
BatchSpanProcessor.newBuilder(new WaitingSpanExporter(0))
.fromConfigMap(Collections.<String, String>emptyMap(), ConfigTester.getNamingDot());
assertThat(config.getScheduleDelayMillis())
.isEqualTo(BatchSpanProcessor.Builder.DEFAULT_SCHEDULE_DELAY_MILLIS);
assertThat(config.getMaxQueueSize())
.isEqualTo(BatchSpanProcessor.Builder.DEFAULT_MAX_QUEUE_SIZE);
assertThat(config.getMaxExportBatchSize())
.isEqualTo(BatchSpanProcessor.Builder.DEFAULT_MAX_EXPORT_BATCH_SIZE);
assertThat(config.getExporterTimeoutMillis())
.isEqualTo(BatchSpanProcessor.Builder.DEFAULT_EXPORT_TIMEOUT_MILLIS);
assertThat(config.getExportOnlySampled())
.isEqualTo(BatchSpanProcessor.Builder.DEFAULT_EXPORT_ONLY_SAMPLED);
}

@Test
Expand Down
23 changes: 13 additions & 10 deletions sdk/src/test/java/io/opentelemetry/sdk/trace/export/SimpleSpanProcessorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@
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}. */
Expand Down Expand Up @@ -77,15 +76,19 @@ public void setUp() {
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);
SimpleSpanProcessor.Builder config =
SimpleSpanProcessor.newBuilder(spanExporter)
.fromConfigMap(options, ConfigTester.getNamingDot());
assertThat(config.getExportOnlySampled()).isEqualTo(false);
}

@Test
public void configTest_EmptyOptions() {
SimpleSpanProcessor.Builder config =
SimpleSpanProcessor.newBuilder(spanExporter)
.fromConfigMap(Collections.<String, String>emptyMap(), ConfigTester.getNamingDot());
assertThat(config.getExportOnlySampled())
.isEqualTo(SimpleSpanProcessor.Builder.DEFAULT_EXPORT_ONLY_SAMPLED);
}

@Test
Expand Down

0 comments on commit 643d25d

Please sign in to comment.