Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add connectTimeout configuration option OtlpHttp{Signal}Exporters #5941

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,2 +1,13 @@
Comparing source compatibility of against
No changes.
*** 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)
Comment on lines +2 to +13
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -37,6 +36,7 @@
@SuppressWarnings("checkstyle:JavadocMethod")
public final class HttpExporterBuilder<T extends Marshaler> {
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());

Expand All @@ -46,6 +46,7 @@ public final class HttpExporterBuilder<T extends Marshaler> {
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<String, String> headers;
Expand All @@ -67,8 +68,9 @@ public HttpExporterBuilder<T> setTimeout(long timeout, TimeUnit unit) {
return this;
}

public HttpExporterBuilder<T> setTimeout(Duration timeout) {
return setTimeout(timeout.toNanos(), TimeUnit.NANOSECONDS);
public HttpExporterBuilder<T> setConnectTimeout(long timeout, TimeUnit unit) {
connectTimeoutNanos = unit.toNanos(timeout);
return this;
}

public HttpExporterBuilder<T> setEndpoint(String endpoint) {
Expand Down Expand Up @@ -132,6 +134,7 @@ public HttpExporterBuilder<T> copy() {
HttpExporterBuilder<T> copy = new HttpExporterBuilder<>(exporterName, type, endpoint);
copy.endpoint = endpoint;
copy.timeoutNanos = timeoutNanos;
copy.connectTimeoutNanos = connectTimeoutNanos;
copy.exportAsJson = exportAsJson;
copy.compressionEnabled = compressionEnabled;
if (headers != null) {
Expand All @@ -157,6 +160,7 @@ public HttpExporter<T> build() {
compressionEnabled,
exportAsJson ? "application/json" : "application/x-protobuf",
timeoutNanos,
connectTimeoutNanos,
headerSupplier,
authenticator,
retryPolicy,
Expand All @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<Map<String, String>> headerSupplier,
@Nullable Authenticator authenticator,
@Nullable RetryPolicy retryPolicy,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -464,6 +464,28 @@ public Stream<? extends Arguments> provideArguments(ExtensionContext context) th
}
}

@Test
@SuppressLogger(HttpExporter.class)
void connectTimeout() {
TelemetryExporter<T> 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<T> exporter =
Expand Down Expand Up @@ -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/"))
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -666,6 +707,7 @@ void toBuilderEquality()
TelemetryExporter<T> exporter =
exporterBuilder()
.setTimeout(Duration.ofSeconds(5))
.setConnectTimeout(Duration.ofSeconds(4))
.setEndpoint("http://localhost:4318")
.setCompression("gzip")
.addHeader("foo", "bar")
Expand Down Expand Up @@ -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\\}"
Expand All @@ -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")
Expand All @@ -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.*\\}, "
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,16 @@ public TelemetryExporterBuilder<LogRecordData> setTimeout(Duration timeout) {
return this;
}

@Override
public TelemetryExporterBuilder<LogRecordData> setConnectTimeout(long timeout, TimeUnit unit) {
throw new UnsupportedOperationException();
}

@Override
public TelemetryExporterBuilder<LogRecordData> setConnectTimeout(Duration timeout) {
throw new UnsupportedOperationException();
}

@Override
public TelemetryExporterBuilder<LogRecordData> setCompression(String compression) {
builder.setCompression(compression);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,16 @@ public TelemetryExporterBuilder<MetricData> setTimeout(Duration timeout) {
return this;
}

@Override
public TelemetryExporterBuilder<MetricData> setConnectTimeout(long timeout, TimeUnit unit) {
throw new UnsupportedOperationException();
}

@Override
public TelemetryExporterBuilder<MetricData> setConnectTimeout(Duration timeout) {
throw new UnsupportedOperationException();
}

@Override
public TelemetryExporterBuilder<MetricData> setCompression(String compression) {
builder.setCompression(compression);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,16 @@ public TelemetryExporterBuilder<SpanData> setTimeout(Duration timeout) {
return this;
}

@Override
public TelemetryExporterBuilder<SpanData> setConnectTimeout(long timeout, TimeUnit unit) {
throw new UnsupportedOperationException();
}

@Override
public TelemetryExporterBuilder<SpanData> setConnectTimeout(Duration timeout) {
throw new UnsupportedOperationException();
}

@Override
public TelemetryExporterBuilder<SpanData> setCompression(String compression) {
builder.setCompression(compression);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,18 @@ public TelemetryExporterBuilder<LogRecordData> setTimeout(Duration timeout) {
return this;
}

@Override
public TelemetryExporterBuilder<LogRecordData> setConnectTimeout(long timeout, TimeUnit unit) {
builder.setConnectTimeout(timeout, unit);
return this;
}

@Override
public TelemetryExporterBuilder<LogRecordData> setConnectTimeout(Duration timeout) {
builder.setConnectTimeout(timeout);
return this;
}

@Override
public TelemetryExporterBuilder<LogRecordData> setCompression(String compression) {
builder.setCompression(compression);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,18 @@ public TelemetryExporterBuilder<MetricData> setTimeout(Duration timeout) {
return this;
}

@Override
public TelemetryExporterBuilder<MetricData> setConnectTimeout(long timeout, TimeUnit unit) {
builder.setConnectTimeout(timeout, unit);
return this;
}

@Override
public TelemetryExporterBuilder<MetricData> setConnectTimeout(Duration timeout) {
builder.setConnectTimeout(timeout);
return this;
}

@Override
public TelemetryExporterBuilder<MetricData> setCompression(String compression) {
builder.setCompression(compression);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,18 @@ public TelemetryExporterBuilder<SpanData> setTimeout(Duration timeout) {
return this;
}

@Override
public TelemetryExporterBuilder<SpanData> setConnectTimeout(long timeout, TimeUnit unit) {
builder.setConnectTimeout(timeout, unit);
return this;
}

@Override
public TelemetryExporterBuilder<SpanData> setConnectTimeout(Duration timeout) {
builder.setConnectTimeout(timeout);
return this;
}

@Override
public TelemetryExporterBuilder<SpanData> setCompression(String compression) {
builder.setCompression(compression);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,18 @@ public TelemetryExporterBuilder<T> setTimeout(Duration timeout) {
return this;
}

@Override
public TelemetryExporterBuilder<T> setConnectTimeout(long timeout, TimeUnit unit) {
delegate.setConnectTimeout(timeout, unit);
return this;
}

@Override
public TelemetryExporterBuilder<T> setConnectTimeout(Duration timeout) {
delegate.setConnectTimeout(timeout);
return this;
}

@Override
public TelemetryExporterBuilder<T> setCompression(String compression) {
delegate.setCompression(compression);
Expand Down
Loading