From 183931bfeddd081e86841689cc2c30f0946fe97b Mon Sep 17 00:00:00 2001 From: Jack Berg Date: Thu, 26 Oct 2023 13:57:35 -0500 Subject: [PATCH 1/2] Add connectTimeout configuration option OtlpHttp{Signal}Exporters --- .../opentelemetry-exporter-otlp.txt | 13 ++++- .../internal/http/HttpExporterBuilder.java | 11 +++-- .../internal/http/HttpSenderProvider.java | 2 + .../OtlpHttpLogRecordExporterBuilder.java | 20 ++++++++ .../OtlpHttpMetricExporterBuilder.java | 20 ++++++++ .../trace/OtlpHttpSpanExporterBuilder.java | 20 ++++++++ .../AbstractHttpTelemetryExporterTest.java | 49 +++++++++++++++++++ .../GrpcLogRecordExporterBuilderWrapper.java | 10 ++++ .../GrpcMetricExporterBuilderWrapper.java | 10 ++++ .../GrpcSpanExporterBuilderWrapper.java | 10 ++++ .../HttpLogRecordExporterBuilderWrapper.java | 12 +++++ .../HttpMetricExporterBuilderWrapper.java | 12 +++++ .../HttpSpanExporterBuilderWrapper.java | 12 +++++ ...anagedChannelTelemetryExporterBuilder.java | 12 +++++ .../internal/TelemetryExporterBuilder.java | 4 ++ .../sender/jdk/internal/JdkHttpSender.java | 11 ++--- .../jdk/internal/JdkHttpSenderProvider.java | 2 + .../jdk/internal/JdkHttpSenderTest.java | 12 ++++- .../okhttp/internal/OkHttpHttpSender.java | 3 ++ .../internal/OkHttpHttpSenderProvider.java | 2 + 20 files changed, 235 insertions(+), 12 deletions(-) diff --git a/docs/apidiffs/current_vs_latest/opentelemetry-exporter-otlp.txt b/docs/apidiffs/current_vs_latest/opentelemetry-exporter-otlp.txt index df26146497b..b889d450445 100644 --- a/docs/apidiffs/current_vs_latest/opentelemetry-exporter-otlp.txt +++ b/docs/apidiffs/current_vs_latest/opentelemetry-exporter-otlp.txt @@ -1,2 +1,13 @@ Comparing source compatibility of against -No changes. \ No newline at end of file +*** MODIFIED CLASS: PUBLIC FINAL io.opentelemetry.exporter.otlp.http.logs.OtlpHttpLogRecordExporterBuilder (not serializable) + === CLASS FILE FORMAT VERSION: 52.0 <- 52.0 + +++ NEW METHOD: PUBLIC(+) io.opentelemetry.exporter.otlp.http.logs.OtlpHttpLogRecordExporterBuilder setConnectTimeout(long, java.util.concurrent.TimeUnit) + +++ NEW METHOD: PUBLIC(+) io.opentelemetry.exporter.otlp.http.logs.OtlpHttpLogRecordExporterBuilder setConnectTimeout(java.time.Duration) +*** MODIFIED CLASS: PUBLIC FINAL io.opentelemetry.exporter.otlp.http.metrics.OtlpHttpMetricExporterBuilder (not serializable) + === CLASS FILE FORMAT VERSION: 52.0 <- 52.0 + +++ NEW METHOD: PUBLIC(+) io.opentelemetry.exporter.otlp.http.metrics.OtlpHttpMetricExporterBuilder setConnectTimeout(long, java.util.concurrent.TimeUnit) + +++ NEW METHOD: PUBLIC(+) io.opentelemetry.exporter.otlp.http.metrics.OtlpHttpMetricExporterBuilder setConnectTimeout(java.time.Duration) +*** MODIFIED CLASS: PUBLIC FINAL io.opentelemetry.exporter.otlp.http.trace.OtlpHttpSpanExporterBuilder (not serializable) + === CLASS FILE FORMAT VERSION: 52.0 <- 52.0 + +++ NEW METHOD: PUBLIC(+) io.opentelemetry.exporter.otlp.http.trace.OtlpHttpSpanExporterBuilder setConnectTimeout(long, java.util.concurrent.TimeUnit) + +++ NEW METHOD: PUBLIC(+) io.opentelemetry.exporter.otlp.http.trace.OtlpHttpSpanExporterBuilder setConnectTimeout(java.time.Duration) diff --git a/exporters/common/src/main/java/io/opentelemetry/exporter/internal/http/HttpExporterBuilder.java b/exporters/common/src/main/java/io/opentelemetry/exporter/internal/http/HttpExporterBuilder.java index 5227a760eb5..2a9b9128e34 100644 --- a/exporters/common/src/main/java/io/opentelemetry/exporter/internal/http/HttpExporterBuilder.java +++ b/exporters/common/src/main/java/io/opentelemetry/exporter/internal/http/HttpExporterBuilder.java @@ -14,7 +14,6 @@ import io.opentelemetry.exporter.internal.marshal.Marshaler; import io.opentelemetry.sdk.common.export.RetryPolicy; import java.net.URI; -import java.time.Duration; import java.util.Collections; import java.util.HashMap; import java.util.Map; @@ -37,6 +36,7 @@ @SuppressWarnings("checkstyle:JavadocMethod") public final class HttpExporterBuilder { public static final long DEFAULT_TIMEOUT_SECS = 10; + public static final long DEFAULT_CONNECT_TIMEOUT_SECS = 10; private static final Logger LOGGER = Logger.getLogger(HttpExporterBuilder.class.getName()); @@ -46,6 +46,7 @@ public final class HttpExporterBuilder { private String endpoint; private long timeoutNanos = TimeUnit.SECONDS.toNanos(DEFAULT_TIMEOUT_SECS); + private long connectTimeoutNanos = TimeUnit.SECONDS.toNanos(DEFAULT_CONNECT_TIMEOUT_SECS); private boolean compressionEnabled = false; private boolean exportAsJson = false; @Nullable private Map headers; @@ -67,8 +68,9 @@ public HttpExporterBuilder setTimeout(long timeout, TimeUnit unit) { return this; } - public HttpExporterBuilder setTimeout(Duration timeout) { - return setTimeout(timeout.toNanos(), TimeUnit.NANOSECONDS); + public HttpExporterBuilder setConnectTimeout(long timeout, TimeUnit unit) { + connectTimeoutNanos = unit.toNanos(timeout); + return this; } public HttpExporterBuilder setEndpoint(String endpoint) { @@ -132,6 +134,7 @@ public HttpExporterBuilder copy() { HttpExporterBuilder copy = new HttpExporterBuilder<>(exporterName, type, endpoint); copy.endpoint = endpoint; copy.timeoutNanos = timeoutNanos; + copy.connectTimeoutNanos = connectTimeoutNanos; copy.exportAsJson = exportAsJson; copy.compressionEnabled = compressionEnabled; if (headers != null) { @@ -157,6 +160,7 @@ public HttpExporter build() { compressionEnabled, exportAsJson ? "application/json" : "application/x-protobuf", timeoutNanos, + connectTimeoutNanos, headerSupplier, authenticator, retryPolicy, @@ -176,6 +180,7 @@ public String toString(boolean includePrefixAndSuffix) { joiner.add("type=" + type); joiner.add("endpoint=" + endpoint); joiner.add("timeoutNanos=" + timeoutNanos); + joiner.add("connectTimeoutNanos=" + connectTimeoutNanos); joiner.add("compressionEnabled=" + compressionEnabled); joiner.add("exportAsJson=" + exportAsJson); if (headers != null) { diff --git a/exporters/common/src/main/java/io/opentelemetry/exporter/internal/http/HttpSenderProvider.java b/exporters/common/src/main/java/io/opentelemetry/exporter/internal/http/HttpSenderProvider.java index 22050e8e624..fba8f4ebb5e 100644 --- a/exporters/common/src/main/java/io/opentelemetry/exporter/internal/http/HttpSenderProvider.java +++ b/exporters/common/src/main/java/io/opentelemetry/exporter/internal/http/HttpSenderProvider.java @@ -23,11 +23,13 @@ public interface HttpSenderProvider { /** Returns a {@link HttpSender} configured with the provided parameters. */ + @SuppressWarnings("TooManyParameters") HttpSender createSender( String endpoint, boolean compressionEnabled, String contentType, long timeoutNanos, + long connectTimeout, Supplier> headerSupplier, @Nullable Authenticator authenticator, @Nullable RetryPolicy retryPolicy, diff --git a/exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/http/logs/OtlpHttpLogRecordExporterBuilder.java b/exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/http/logs/OtlpHttpLogRecordExporterBuilder.java index cb74e9859a8..8ddcb4fad57 100644 --- a/exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/http/logs/OtlpHttpLogRecordExporterBuilder.java +++ b/exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/http/logs/OtlpHttpLogRecordExporterBuilder.java @@ -59,6 +59,26 @@ public OtlpHttpLogRecordExporterBuilder setTimeout(Duration timeout) { return setTimeout(timeout.toNanos(), TimeUnit.NANOSECONDS); } + /** + * Sets the maximum time to wait for new connections to be established. If unset, defaults to + * {@value HttpExporterBuilder#DEFAULT_CONNECT_TIMEOUT_SECS}s. + */ + public OtlpHttpLogRecordExporterBuilder setConnectTimeout(long timeout, TimeUnit unit) { + requireNonNull(unit, "unit"); + checkArgument(timeout >= 0, "timeout must be non-negative"); + delegate.setConnectTimeout(timeout, unit); + return this; + } + + /** + * Sets the maximum time to wait for new connections to be established. If unset, defaults to + * {@value HttpExporterBuilder#DEFAULT_CONNECT_TIMEOUT_SECS}s. + */ + public OtlpHttpLogRecordExporterBuilder setConnectTimeout(Duration timeout) { + requireNonNull(timeout, "timeout"); + return setConnectTimeout(timeout.toNanos(), TimeUnit.NANOSECONDS); + } + /** * Sets the OTLP endpoint to connect to. If unset, defaults to {@value DEFAULT_ENDPOINT}. The * endpoint must start with either http:// or https://, and include the full HTTP path. diff --git a/exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/http/metrics/OtlpHttpMetricExporterBuilder.java b/exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/http/metrics/OtlpHttpMetricExporterBuilder.java index c9466d51819..a4002d87325 100644 --- a/exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/http/metrics/OtlpHttpMetricExporterBuilder.java +++ b/exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/http/metrics/OtlpHttpMetricExporterBuilder.java @@ -71,6 +71,26 @@ public OtlpHttpMetricExporterBuilder setTimeout(Duration timeout) { return setTimeout(timeout.toNanos(), TimeUnit.NANOSECONDS); } + /** + * Sets the maximum time to wait for new connections to be established. If unset, defaults to + * {@value HttpExporterBuilder#DEFAULT_CONNECT_TIMEOUT_SECS}s. + */ + public OtlpHttpMetricExporterBuilder setConnectTimeout(long timeout, TimeUnit unit) { + requireNonNull(unit, "unit"); + checkArgument(timeout >= 0, "timeout must be non-negative"); + delegate.setConnectTimeout(timeout, unit); + return this; + } + + /** + * Sets the maximum time to wait for new connections to be established. If unset, defaults to + * {@value HttpExporterBuilder#DEFAULT_CONNECT_TIMEOUT_SECS}s. + */ + public OtlpHttpMetricExporterBuilder setConnectTimeout(Duration timeout) { + requireNonNull(timeout, "timeout"); + return setConnectTimeout(timeout.toNanos(), TimeUnit.NANOSECONDS); + } + /** * Sets the OTLP endpoint to connect to. If unset, defaults to {@value DEFAULT_ENDPOINT}. The * endpoint must start with either http:// or https://, and include the full HTTP path. diff --git a/exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/http/trace/OtlpHttpSpanExporterBuilder.java b/exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/http/trace/OtlpHttpSpanExporterBuilder.java index 3cf5b85b1cf..e6849a201a6 100644 --- a/exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/http/trace/OtlpHttpSpanExporterBuilder.java +++ b/exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/http/trace/OtlpHttpSpanExporterBuilder.java @@ -59,6 +59,26 @@ public OtlpHttpSpanExporterBuilder setTimeout(Duration timeout) { return setTimeout(timeout.toNanos(), TimeUnit.NANOSECONDS); } + /** + * Sets the maximum time to wait for new connections to be established. If unset, defaults to + * {@value HttpExporterBuilder#DEFAULT_CONNECT_TIMEOUT_SECS}s. + */ + public OtlpHttpSpanExporterBuilder setConnectTimeout(long timeout, TimeUnit unit) { + requireNonNull(unit, "unit"); + checkArgument(timeout >= 0, "timeout must be non-negative"); + delegate.setConnectTimeout(timeout, unit); + return this; + } + + /** + * Sets the maximum time to wait for new connections to be established. If unset, defaults to + * {@value HttpExporterBuilder#DEFAULT_CONNECT_TIMEOUT_SECS}s. + */ + public OtlpHttpSpanExporterBuilder setConnectTimeout(Duration timeout) { + requireNonNull(timeout, "timeout"); + return setConnectTimeout(timeout.toNanos(), TimeUnit.NANOSECONDS); + } + /** * Sets the OTLP endpoint to connect to. If unset, defaults to {@value DEFAULT_ENDPOINT}. The * endpoint must start with either http:// or https://, and include the full HTTP path. diff --git a/exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/AbstractHttpTelemetryExporterTest.java b/exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/AbstractHttpTelemetryExporterTest.java index be8e5e773fc..c306e4fbc2f 100644 --- a/exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/AbstractHttpTelemetryExporterTest.java +++ b/exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/AbstractHttpTelemetryExporterTest.java @@ -464,6 +464,28 @@ public Stream provideArguments(ExtensionContext context) th } } + @Test + @SuppressLogger(HttpExporter.class) + void connectTimeout() { + TelemetryExporter exporter = + exporterBuilder() + // Connecting to a non-routable IP address to trigger connection error + .setEndpoint("http://10.255.255.1") + .setConnectTimeout(Duration.ofMillis(1)) + .build(); + try { + long startTimeMillis = System.currentTimeMillis(); + CompletableResultCode result = + exporter.export(Collections.singletonList(generateFakeTelemetry())); + assertThat(result.join(10, TimeUnit.SECONDS).isSuccess()).isFalse(); + // Assert that the export request fails well before the default connect timeout of 10s + assertThat(System.currentTimeMillis() - startTimeMillis) + .isLessThan(TimeUnit.SECONDS.toMillis(1)); + } finally { + exporter.shutdown(); + } + } + @Test void deadlineSetPerExport() throws InterruptedException { TelemetryExporter exporter = @@ -602,6 +624,15 @@ void validConfig() { assertThatCode(() -> exporterBuilder().setTimeout(Duration.ofMillis(10))) .doesNotThrowAnyException(); + assertThatCode(() -> exporterBuilder().setConnectTimeout(0, TimeUnit.MILLISECONDS)) + .doesNotThrowAnyException(); + assertThatCode(() -> exporterBuilder().setConnectTimeout(Duration.ofMillis(0))) + .doesNotThrowAnyException(); + assertThatCode(() -> exporterBuilder().setConnectTimeout(10, TimeUnit.MILLISECONDS)) + .doesNotThrowAnyException(); + assertThatCode(() -> exporterBuilder().setConnectTimeout(Duration.ofMillis(10))) + .doesNotThrowAnyException(); + assertThatCode(() -> exporterBuilder().setEndpoint("http://localhost:4318")) .doesNotThrowAnyException(); assertThatCode(() -> exporterBuilder().setEndpoint("http://localhost/")) @@ -635,6 +666,16 @@ void invalidConfig() { .isInstanceOf(NullPointerException.class) .hasMessage("timeout"); + assertThatThrownBy(() -> exporterBuilder().setConnectTimeout(-1, TimeUnit.MILLISECONDS)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("timeout must be non-negative"); + assertThatThrownBy(() -> exporterBuilder().setConnectTimeout(1, null)) + .isInstanceOf(NullPointerException.class) + .hasMessage("unit"); + assertThatThrownBy(() -> exporterBuilder().setConnectTimeout(null)) + .isInstanceOf(NullPointerException.class) + .hasMessage("timeout"); + assertThatThrownBy(() -> exporterBuilder().setEndpoint(null)) .isInstanceOf(NullPointerException.class) .hasMessage("endpoint"); @@ -666,6 +707,7 @@ void toBuilderEquality() TelemetryExporter exporter = exporterBuilder() .setTimeout(Duration.ofSeconds(5)) + .setConnectTimeout(Duration.ofSeconds(4)) .setEndpoint("http://localhost:4318") .setCompression("gzip") .addHeader("foo", "bar") @@ -728,6 +770,9 @@ void stringRepresentation() throws IOException, CertificateEncodingException { + "timeoutNanos=" + TimeUnit.SECONDS.toNanos(10) + ", " + + "connectTimeoutNanos=" + + TimeUnit.SECONDS.toNanos(10) + + ", " + "compressionEnabled=false, " + "exportAsJson=false, " + "headers=Headers\\{User-Agent=OBFUSCATED\\}" @@ -739,6 +784,7 @@ void stringRepresentation() throws IOException, CertificateEncodingException { telemetryExporter = exporterBuilder() .setTimeout(Duration.ofSeconds(5)) + .setConnectTimeout(Duration.ofSeconds(4)) .setEndpoint("http://example:4318/v1/logs") .setCompression("gzip") .addHeader("foo", "bar") @@ -764,6 +810,9 @@ void stringRepresentation() throws IOException, CertificateEncodingException { + "timeoutNanos=" + TimeUnit.SECONDS.toNanos(5) + ", " + + "connectTimeoutNanos=" + + TimeUnit.SECONDS.toNanos(4) + + ", " + "compressionEnabled=true, " + "exportAsJson=false, " + "headers=Headers\\{.*foo=OBFUSCATED.*\\}, " diff --git a/exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/GrpcLogRecordExporterBuilderWrapper.java b/exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/GrpcLogRecordExporterBuilderWrapper.java index 993b13312f3..916f15e6b2f 100644 --- a/exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/GrpcLogRecordExporterBuilderWrapper.java +++ b/exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/GrpcLogRecordExporterBuilderWrapper.java @@ -40,6 +40,16 @@ public TelemetryExporterBuilder setTimeout(Duration timeout) { return this; } + @Override + public TelemetryExporterBuilder setConnectTimeout(long timeout, TimeUnit unit) { + throw new UnsupportedOperationException(); + } + + @Override + public TelemetryExporterBuilder setConnectTimeout(Duration timeout) { + throw new UnsupportedOperationException(); + } + @Override public TelemetryExporterBuilder setCompression(String compression) { builder.setCompression(compression); diff --git a/exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/GrpcMetricExporterBuilderWrapper.java b/exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/GrpcMetricExporterBuilderWrapper.java index 0cfa4401ab1..821baaa729d 100644 --- a/exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/GrpcMetricExporterBuilderWrapper.java +++ b/exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/GrpcMetricExporterBuilderWrapper.java @@ -40,6 +40,16 @@ public TelemetryExporterBuilder setTimeout(Duration timeout) { return this; } + @Override + public TelemetryExporterBuilder setConnectTimeout(long timeout, TimeUnit unit) { + throw new UnsupportedOperationException(); + } + + @Override + public TelemetryExporterBuilder setConnectTimeout(Duration timeout) { + throw new UnsupportedOperationException(); + } + @Override public TelemetryExporterBuilder setCompression(String compression) { builder.setCompression(compression); diff --git a/exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/GrpcSpanExporterBuilderWrapper.java b/exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/GrpcSpanExporterBuilderWrapper.java index bc4a10aa8f1..2f86433672e 100644 --- a/exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/GrpcSpanExporterBuilderWrapper.java +++ b/exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/GrpcSpanExporterBuilderWrapper.java @@ -41,6 +41,16 @@ public TelemetryExporterBuilder setTimeout(Duration timeout) { return this; } + @Override + public TelemetryExporterBuilder setConnectTimeout(long timeout, TimeUnit unit) { + throw new UnsupportedOperationException(); + } + + @Override + public TelemetryExporterBuilder setConnectTimeout(Duration timeout) { + throw new UnsupportedOperationException(); + } + @Override public TelemetryExporterBuilder setCompression(String compression) { builder.setCompression(compression); diff --git a/exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/HttpLogRecordExporterBuilderWrapper.java b/exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/HttpLogRecordExporterBuilderWrapper.java index 7e007fa6e7b..5fc42608acd 100644 --- a/exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/HttpLogRecordExporterBuilderWrapper.java +++ b/exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/HttpLogRecordExporterBuilderWrapper.java @@ -41,6 +41,18 @@ public TelemetryExporterBuilder setTimeout(Duration timeout) { return this; } + @Override + public TelemetryExporterBuilder setConnectTimeout(long timeout, TimeUnit unit) { + builder.setConnectTimeout(timeout, unit); + return this; + } + + @Override + public TelemetryExporterBuilder setConnectTimeout(Duration timeout) { + builder.setConnectTimeout(timeout); + return this; + } + @Override public TelemetryExporterBuilder setCompression(String compression) { builder.setCompression(compression); diff --git a/exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/HttpMetricExporterBuilderWrapper.java b/exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/HttpMetricExporterBuilderWrapper.java index e3d9d1cc20e..e4357378a86 100644 --- a/exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/HttpMetricExporterBuilderWrapper.java +++ b/exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/HttpMetricExporterBuilderWrapper.java @@ -40,6 +40,18 @@ public TelemetryExporterBuilder setTimeout(Duration timeout) { return this; } + @Override + public TelemetryExporterBuilder setConnectTimeout(long timeout, TimeUnit unit) { + builder.setConnectTimeout(timeout, unit); + return this; + } + + @Override + public TelemetryExporterBuilder setConnectTimeout(Duration timeout) { + builder.setConnectTimeout(timeout); + return this; + } + @Override public TelemetryExporterBuilder setCompression(String compression) { builder.setCompression(compression); diff --git a/exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/HttpSpanExporterBuilderWrapper.java b/exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/HttpSpanExporterBuilderWrapper.java index 63660c4314b..550d25c26e1 100644 --- a/exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/HttpSpanExporterBuilderWrapper.java +++ b/exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/HttpSpanExporterBuilderWrapper.java @@ -40,6 +40,18 @@ public TelemetryExporterBuilder setTimeout(Duration timeout) { return this; } + @Override + public TelemetryExporterBuilder setConnectTimeout(long timeout, TimeUnit unit) { + builder.setConnectTimeout(timeout, unit); + return this; + } + + @Override + public TelemetryExporterBuilder setConnectTimeout(Duration timeout) { + builder.setConnectTimeout(timeout); + return this; + } + @Override public TelemetryExporterBuilder setCompression(String compression) { builder.setCompression(compression); diff --git a/exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/ManagedChannelTelemetryExporterBuilder.java b/exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/ManagedChannelTelemetryExporterBuilder.java index 79781eb5809..fbad7272a76 100644 --- a/exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/ManagedChannelTelemetryExporterBuilder.java +++ b/exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/ManagedChannelTelemetryExporterBuilder.java @@ -76,6 +76,18 @@ public TelemetryExporterBuilder setTimeout(Duration timeout) { return this; } + @Override + public TelemetryExporterBuilder setConnectTimeout(long timeout, TimeUnit unit) { + delegate.setConnectTimeout(timeout, unit); + return this; + } + + @Override + public TelemetryExporterBuilder setConnectTimeout(Duration timeout) { + delegate.setConnectTimeout(timeout); + return this; + } + @Override public TelemetryExporterBuilder setCompression(String compression) { delegate.setCompression(compression); diff --git a/exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/TelemetryExporterBuilder.java b/exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/TelemetryExporterBuilder.java index 95b62e88a28..6fbe78149e5 100644 --- a/exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/TelemetryExporterBuilder.java +++ b/exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/TelemetryExporterBuilder.java @@ -38,6 +38,10 @@ static TelemetryExporterBuilder wrap(OtlpGrpcLogRecordExporterBui TelemetryExporterBuilder setTimeout(Duration timeout); + TelemetryExporterBuilder setConnectTimeout(long timeout, TimeUnit unit); + + TelemetryExporterBuilder setConnectTimeout(Duration timeout); + TelemetryExporterBuilder setCompression(String compression); TelemetryExporterBuilder addHeader(String key, String value); diff --git a/exporters/sender/jdk/src/main/java/io/opentelemetry/exporter/sender/jdk/internal/JdkHttpSender.java b/exporters/sender/jdk/src/main/java/io/opentelemetry/exporter/sender/jdk/internal/JdkHttpSender.java index fda90c6a8c7..7739d3c02b3 100644 --- a/exporters/sender/jdk/src/main/java/io/opentelemetry/exporter/sender/jdk/internal/JdkHttpSender.java +++ b/exporters/sender/jdk/src/main/java/io/opentelemetry/exporter/sender/jdk/internal/JdkHttpSender.java @@ -84,11 +84,12 @@ public final class JdkHttpSender implements HttpSender { boolean compressionEnabled, String contentType, long timeoutNanos, + long connectTimeoutNanos, Supplier> headerSupplier, @Nullable RetryPolicy retryPolicy, @Nullable SSLContext sslContext) { this( - configureClient(sslContext), + configureClient(sslContext, connectTimeoutNanos), endpoint, compressionEnabled, contentType, @@ -97,12 +98,10 @@ public final class JdkHttpSender implements HttpSender { retryPolicy); } - private static HttpClient configureClient(@Nullable SSLContext sslContext) { + private static HttpClient configureClient( + @Nullable SSLContext sslContext, long connectionTimeoutNanos) { HttpClient.Builder builder = - HttpClient.newBuilder() - // Aligned with OkHttpClient default connect timeout - // TODO (jack-berg): Consider making connect timeout configurable - .connectTimeout(Duration.ofSeconds(10)); + HttpClient.newBuilder().connectTimeout(Duration.ofNanos(connectionTimeoutNanos)); if (sslContext != null) { builder.sslContext(sslContext); } diff --git a/exporters/sender/jdk/src/main/java/io/opentelemetry/exporter/sender/jdk/internal/JdkHttpSenderProvider.java b/exporters/sender/jdk/src/main/java/io/opentelemetry/exporter/sender/jdk/internal/JdkHttpSenderProvider.java index dbe93eef3a3..5e3496b1496 100644 --- a/exporters/sender/jdk/src/main/java/io/opentelemetry/exporter/sender/jdk/internal/JdkHttpSenderProvider.java +++ b/exporters/sender/jdk/src/main/java/io/opentelemetry/exporter/sender/jdk/internal/JdkHttpSenderProvider.java @@ -29,6 +29,7 @@ public HttpSender createSender( boolean compressionEnabled, String contentType, long timeoutNanos, + long connectTimeout, Supplier> headerSupplier, @Nullable Authenticator authenticator, @Nullable RetryPolicy retryPolicy, @@ -39,6 +40,7 @@ public HttpSender createSender( compressionEnabled, contentType, timeoutNanos, + connectTimeout, headerSupplier, retryPolicy, sslContext); diff --git a/exporters/sender/jdk/src/test/java/io/opentelemetry/exporter/sender/jdk/internal/JdkHttpSenderTest.java b/exporters/sender/jdk/src/test/java/io/opentelemetry/exporter/sender/jdk/internal/JdkHttpSenderTest.java index 2691bb1ca3f..585648c2c1a 100644 --- a/exporters/sender/jdk/src/test/java/io/opentelemetry/exporter/sender/jdk/internal/JdkHttpSenderTest.java +++ b/exporters/sender/jdk/src/test/java/io/opentelemetry/exporter/sender/jdk/internal/JdkHttpSenderTest.java @@ -20,6 +20,7 @@ import java.net.http.HttpConnectTimeoutException; import java.time.Duration; import java.util.Collections; +import java.util.concurrent.TimeUnit; import org.assertj.core.api.InstanceOfAssertFactories; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -81,10 +82,17 @@ void sendInternal_NonRetryableException() throws IOException, InterruptedExcepti } @Test - void defaultConnectTimeout() { + void connectTimeout() { sender = new JdkHttpSender( - "http://localhost", true, "text/plain", 1, Collections::emptyMap, null, null); + "http://localhost", + true, + "text/plain", + 1, + TimeUnit.SECONDS.toNanos(10), + Collections::emptyMap, + null, + null); assertThat(sender) .extracting("client", as(InstanceOfAssertFactories.type(HttpClient.class))) diff --git a/exporters/sender/okhttp/src/main/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpHttpSender.java b/exporters/sender/okhttp/src/main/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpHttpSender.java index 306c7b97163..d8d06c97187 100644 --- a/exporters/sender/okhttp/src/main/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpHttpSender.java +++ b/exporters/sender/okhttp/src/main/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpHttpSender.java @@ -46,11 +46,13 @@ public final class OkHttpHttpSender implements HttpSender { private final MediaType mediaType; /** Create a sender. */ + @SuppressWarnings("TooManyParameters") public OkHttpHttpSender( String endpoint, boolean compressionEnabled, String contentType, long timeoutNanos, + long connectionTimeoutNanos, Supplier> headerSupplier, @Nullable Authenticator authenticator, @Nullable RetryPolicy retryPolicy, @@ -59,6 +61,7 @@ public OkHttpHttpSender( OkHttpClient.Builder builder = new OkHttpClient.Builder() .dispatcher(OkHttpUtil.newDispatcher()) + .connectTimeout(Duration.ofNanos(connectionTimeoutNanos)) .callTimeout(Duration.ofNanos(timeoutNanos)); if (authenticator != null) { diff --git a/exporters/sender/okhttp/src/main/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpHttpSenderProvider.java b/exporters/sender/okhttp/src/main/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpHttpSenderProvider.java index edf5e9cf45e..16a767f5c62 100644 --- a/exporters/sender/okhttp/src/main/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpHttpSenderProvider.java +++ b/exporters/sender/okhttp/src/main/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpHttpSenderProvider.java @@ -29,6 +29,7 @@ public HttpSender createSender( boolean compressionEnabled, String contentType, long timeoutNanos, + long connectTimeout, Supplier> headerSupplier, @Nullable Authenticator authenticator, @Nullable RetryPolicy retryPolicy, @@ -39,6 +40,7 @@ public HttpSender createSender( compressionEnabled, contentType, timeoutNanos, + connectTimeout, headerSupplier, authenticator, retryPolicy, From c9e5a5d502dee5f79dcf7326ee101fb05c3c51d1 Mon Sep 17 00:00:00 2001 From: Jack Berg Date: Thu, 7 Dec 2023 15:48:32 -0600 Subject: [PATCH 2/2] Fix merge --- .../sender/okhttp/internal/OkHttpHttpSuppressionTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpHttpSuppressionTest.java b/exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpHttpSuppressionTest.java index 22c2c4a0dd0..87da2a18e84 100644 --- a/exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpHttpSuppressionTest.java +++ b/exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpHttpSuppressionTest.java @@ -34,6 +34,6 @@ void send(OkHttpHttpSender sender, Runnable onSuccess, Runnable onFailure) { @Override OkHttpHttpSender createSender(String endpoint) { return new OkHttpHttpSender( - endpoint, false, "text/plain", 10L, Collections::emptyMap, null, null, null, null); + endpoint, false, "text/plain", 10L, 10L, Collections::emptyMap, null, null, null, null); } }