From cc4720de131433b58c1f10dab85a81e3522082fb Mon Sep 17 00:00:00 2001 From: John Watson Date: Wed, 5 May 2021 17:12:06 -0700 Subject: [PATCH] Add the option to configure the zipkin exporter timeout. (#3203) --- .../zipkin/ZipkinSpanExporterBuilder.java | 35 ++++++++++++++++++- .../zipkin/ZipkinSpanExporterTest.java | 30 ++++++++++++++++ .../SpanExporterConfiguration.java | 5 +++ .../SpanExporterConfigurationTest.java | 15 ++++++++ .../sdk/autoconfigure/ZipkinConfigTest.java | 1 + 5 files changed, 85 insertions(+), 1 deletion(-) diff --git a/exporters/zipkin/src/main/java/io/opentelemetry/exporter/zipkin/ZipkinSpanExporterBuilder.java b/exporters/zipkin/src/main/java/io/opentelemetry/exporter/zipkin/ZipkinSpanExporterBuilder.java index 8774bc8b79..051b6baef4 100644 --- a/exporters/zipkin/src/main/java/io/opentelemetry/exporter/zipkin/ZipkinSpanExporterBuilder.java +++ b/exporters/zipkin/src/main/java/io/opentelemetry/exporter/zipkin/ZipkinSpanExporterBuilder.java @@ -5,6 +5,11 @@ package io.opentelemetry.exporter.zipkin; +import static io.opentelemetry.api.internal.Utils.checkArgument; +import static java.util.Objects.requireNonNull; + +import java.time.Duration; +import java.util.concurrent.TimeUnit; import zipkin2.Span; import zipkin2.codec.BytesEncoder; import zipkin2.codec.SpanBytesEncoder; @@ -16,6 +21,7 @@ public final class ZipkinSpanExporterBuilder { private BytesEncoder encoder = SpanBytesEncoder.JSON_V2; private Sender sender; private String endpoint = ZipkinSpanExporter.DEFAULT_ENDPOINT; + private long readTimeoutMillis = TimeUnit.SECONDS.toMillis(10); /** * Sets the Zipkin sender. Implements the client side of the span transport. A {@link @@ -27,6 +33,7 @@ public final class ZipkinSpanExporterBuilder { * @return this. */ public ZipkinSpanExporterBuilder setSender(Sender sender) { + requireNonNull(sender, "sender"); this.sender = sender; return this; } @@ -40,6 +47,7 @@ public final class ZipkinSpanExporterBuilder { * @see SpanBytesEncoder */ public ZipkinSpanExporterBuilder setEncoder(BytesEncoder encoder) { + requireNonNull(encoder, "encoder"); this.encoder = encoder; return this; } @@ -53,10 +61,34 @@ public final class ZipkinSpanExporterBuilder { * @see OkHttpSender */ public ZipkinSpanExporterBuilder setEndpoint(String endpoint) { + requireNonNull(endpoint, "endpoint"); this.endpoint = endpoint; return this; } + /** + * Sets the maximum time to wait for the export of a batch of spans. If unset, defaults to 10s. + * + * @return this. + */ + public ZipkinSpanExporterBuilder setReadTimeout(long timeout, TimeUnit unit) { + requireNonNull(unit, "unit"); + checkArgument(timeout >= 0, "timeout must be non-negative"); + this.readTimeoutMillis = unit.toMillis(timeout); + return this; + } + + /** + * Sets the maximum time to wait for the export of a batch of spans. If unset, defaults to 10s. + * + * @return this. + */ + public ZipkinSpanExporterBuilder setReadTimeout(Duration timeout) { + requireNonNull(timeout, "timeout"); + setReadTimeout(timeout.toMillis(), TimeUnit.MILLISECONDS); + return this; + } + /** * Builds a {@link ZipkinSpanExporter}. * @@ -64,7 +96,8 @@ public final class ZipkinSpanExporterBuilder { */ public ZipkinSpanExporter build() { if (sender == null) { - sender = OkHttpSender.create(endpoint); + sender = + OkHttpSender.newBuilder().endpoint(endpoint).readTimeout((int) readTimeoutMillis).build(); } return new ZipkinSpanExporter(this.encoder, this.sender); } diff --git a/exporters/zipkin/src/test/java/io/opentelemetry/exporter/zipkin/ZipkinSpanExporterTest.java b/exporters/zipkin/src/test/java/io/opentelemetry/exporter/zipkin/ZipkinSpanExporterTest.java index 225eafd361..97c4d73cc3 100644 --- a/exporters/zipkin/src/test/java/io/opentelemetry/exporter/zipkin/ZipkinSpanExporterTest.java +++ b/exporters/zipkin/src/test/java/io/opentelemetry/exporter/zipkin/ZipkinSpanExporterTest.java @@ -14,6 +14,7 @@ import static io.opentelemetry.api.common.AttributeKey.longKey; import static io.opentelemetry.api.common.AttributeKey.stringArrayKey; import static io.opentelemetry.api.common.AttributeKey.stringKey; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.verify; @@ -38,6 +39,7 @@ import java.io.IOException; import java.util.Arrays; import java.util.Collections; import java.util.List; +import java.util.concurrent.TimeUnit; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mock; @@ -423,4 +425,32 @@ class ZipkinSpanExporterTest { .addAnnotation(1505855799000000L + 433901068L / 1000, "RECEIVED") .addAnnotation(1505855799000000L + 459486280L / 1000, "SENT"); } + + @Test + @SuppressWarnings("PreferJavaTimeOverload") + void invalidConfig() { + assertThatThrownBy(() -> ZipkinSpanExporter.builder().setReadTimeout(-1, TimeUnit.MILLISECONDS)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("timeout must be non-negative"); + + assertThatThrownBy(() -> ZipkinSpanExporter.builder().setReadTimeout(1, null)) + .isInstanceOf(NullPointerException.class) + .hasMessage("unit"); + + assertThatThrownBy(() -> ZipkinSpanExporter.builder().setReadTimeout(null)) + .isInstanceOf(NullPointerException.class) + .hasMessage("timeout"); + + assertThatThrownBy(() -> ZipkinSpanExporter.builder().setEndpoint(null)) + .isInstanceOf(NullPointerException.class) + .hasMessage("endpoint"); + + assertThatThrownBy(() -> ZipkinSpanExporter.builder().setSender(null)) + .isInstanceOf(NullPointerException.class) + .hasMessage("sender"); + + assertThatThrownBy(() -> ZipkinSpanExporter.builder().setEncoder(null)) + .isInstanceOf(NullPointerException.class) + .hasMessage("encoder"); + } } diff --git a/sdk-extensions/autoconfigure/src/main/java/io/opentelemetry/sdk/autoconfigure/SpanExporterConfiguration.java b/sdk-extensions/autoconfigure/src/main/java/io/opentelemetry/sdk/autoconfigure/SpanExporterConfiguration.java index 92c2bbe04a..50432cbc60 100644 --- a/sdk-extensions/autoconfigure/src/main/java/io/opentelemetry/sdk/autoconfigure/SpanExporterConfiguration.java +++ b/sdk-extensions/autoconfigure/src/main/java/io/opentelemetry/sdk/autoconfigure/SpanExporterConfiguration.java @@ -135,6 +135,11 @@ final class SpanExporterConfiguration { builder.setEndpoint(endpoint); } + Duration timeout = config.getDuration("otel.exporter.zipkin.timeout"); + if (timeout != null) { + builder.setReadTimeout(timeout); + } + return builder.build(); } diff --git a/sdk-extensions/autoconfigure/src/testFullConfig/java/io/opentelemetry/sdk/autoconfigure/SpanExporterConfigurationTest.java b/sdk-extensions/autoconfigure/src/testFullConfig/java/io/opentelemetry/sdk/autoconfigure/SpanExporterConfigurationTest.java index efda9456fc..3c05675eda 100644 --- a/sdk-extensions/autoconfigure/src/testFullConfig/java/io/opentelemetry/sdk/autoconfigure/SpanExporterConfigurationTest.java +++ b/sdk-extensions/autoconfigure/src/testFullConfig/java/io/opentelemetry/sdk/autoconfigure/SpanExporterConfigurationTest.java @@ -57,4 +57,19 @@ class SpanExporterConfigurationTest { exporter.shutdown(); } } + + // Timeout difficult to test using real exports so just check that things don't blow up. + @Test + void configureZipkinTimeout() { + SpanExporter exporter = + SpanExporterConfiguration.configureExporter( + "zipkin", + ConfigProperties.createForTest( + Collections.singletonMap("otel.exporter.zipkin.timeout", "5s"))); + try { + assertThat(exporter).isNotNull(); + } finally { + exporter.shutdown(); + } + } } diff --git a/sdk-extensions/autoconfigure/src/testZipkin/java/io/opentelemetry/sdk/autoconfigure/ZipkinConfigTest.java b/sdk-extensions/autoconfigure/src/testZipkin/java/io/opentelemetry/sdk/autoconfigure/ZipkinConfigTest.java index 592eb58999..24f6ba8a04 100644 --- a/sdk-extensions/autoconfigure/src/testZipkin/java/io/opentelemetry/sdk/autoconfigure/ZipkinConfigTest.java +++ b/sdk-extensions/autoconfigure/src/testZipkin/java/io/opentelemetry/sdk/autoconfigure/ZipkinConfigTest.java @@ -52,6 +52,7 @@ class ZipkinConfigTest { String endpoint = "localhost:" + server.httpPort(); System.setProperty("otel.exporter.zipkin.endpoint", "http://" + endpoint + "/api/v2/spans"); + System.setProperty("otel.exporter.zipkin.timeout", "5s"); OpenTelemetrySdkAutoConfiguration.initialize();