open-telemetry
/
opentelemetry-java
mirror of https://github.com/open-telemetry/opentelemetry-java.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Interpret timeout zero value as no limit (#7023)

This commit is contained in:
jack-berg 2025-01-16 18:02:31 -06:00 committed by GitHub
parent 4eeef33524
commit 31869a3fc6
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
10 changed files with 131 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
exporters/common/src/main/java/io/opentelemetry/exporter/internal/grpc/GrpcExporterBuilder.java
View File

@ -87,7 +87,7 @@ public class GrpcExporterBuilder<T extends Marshaler> {
}
public GrpcExporterBuilder<T> setTimeout(long timeout, TimeUnit unit) {
timeoutNanos = unit.toNanos(timeout);
timeoutNanos = timeout == 0 ? Long.MAX_VALUE : unit.toNanos(timeout);
return this;
}
@ -96,7 +96,7 @@ public class GrpcExporterBuilder<T extends Marshaler> {
}
public GrpcExporterBuilder<T> setConnectTimeout(long timeout, TimeUnit unit) {
connectTimeoutNanos = unit.toNanos(timeout);
connectTimeoutNanos = timeout == 0 ? Long.MAX_VALUE : unit.toNanos(timeout);
return this;
}

4
exporters/common/src/main/java/io/opentelemetry/exporter/internal/http/HttpExporterBuilder.java
View File

@ -69,12 +69,12 @@ public final class HttpExporterBuilder<T extends Marshaler> {
}
public HttpExporterBuilder<T> setTimeout(long timeout, TimeUnit unit) {
timeoutNanos = unit.toNanos(timeout);
timeoutNanos = timeout == 0 ? Long.MAX_VALUE : unit.toNanos(timeout);
return this;
}
public HttpExporterBuilder<T> setConnectTimeout(long timeout, TimeUnit unit) {
connectTimeoutNanos = unit.toNanos(timeout);
connectTimeoutNanos = timeout == 0 ? Long.MAX_VALUE : unit.toNanos(timeout);
return this;
}

58
exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/AbstractGrpcTelemetryExporterTest.java
View File

@ -815,13 +815,39 @@ public abstract class AbstractGrpcTelemetryExporterTest<T, U extends Message> {
@Test
@SuppressWarnings("PreferJavaTimeOverload")
void validConfig() {
assertThatCode(() -> exporterBuilder().setTimeout(0, TimeUnit.MILLISECONDS))
// We must build exporters to test timeout settings, which intersect with underlying client
// implementations and may convert between Duration, int, and long, which may be susceptible to
// overflow exceptions.
assertThatCode(() -> buildAndShutdown(exporterBuilder().setTimeout(0, TimeUnit.MILLISECONDS)))
.doesNotThrowAnyException();
assertThatCode(() -> exporterBuilder().setTimeout(Duration.ofMillis(0)))
assertThatCode(() -> buildAndShutdown(exporterBuilder().setTimeout(Duration.ofMillis(0))))
.doesNotThrowAnyException();
assertThatCode(() -> exporterBuilder().setTimeout(10, TimeUnit.MILLISECONDS))
assertThatCode(
() ->
buildAndShutdown(
exporterBuilder().setTimeout(Long.MAX_VALUE, TimeUnit.NANOSECONDS)))
.doesNotThrowAnyException();
assertThatCode(() -> exporterBuilder().setTimeout(Duration.ofMillis(10)))
assertThatCode(
() -> buildAndShutdown(exporterBuilder().setTimeout(Duration.ofNanos(Long.MAX_VALUE))))
.doesNotThrowAnyException();
assertThatCode(
() -> buildAndShutdown(exporterBuilder().setTimeout(Long.MAX_VALUE, TimeUnit.SECONDS)))
.doesNotThrowAnyException();
assertThatCode(() -> buildAndShutdown(exporterBuilder().setTimeout(10, TimeUnit.MILLISECONDS)))
.doesNotThrowAnyException();
assertThatCode(() -> buildAndShutdown(exporterBuilder().setTimeout(Duration.ofMillis(10))))
.doesNotThrowAnyException();
assertThatCode(
() -> buildAndShutdown(exporterBuilder().setConnectTimeout(0, TimeUnit.MILLISECONDS)))
.doesNotThrowAnyException();
assertThatCode(
() -> buildAndShutdown(exporterBuilder().setConnectTimeout(Duration.ofMillis(0))))
.doesNotThrowAnyException();
assertThatCode(
() -> buildAndShutdown(exporterBuilder().setConnectTimeout(10, TimeUnit.MILLISECONDS)))
.doesNotThrowAnyException();
assertThatCode(
() -> buildAndShutdown(exporterBuilder().setConnectTimeout(Duration.ofMillis(10))))
.doesNotThrowAnyException();
assertThatCode(() -> exporterBuilder().setEndpoint("http://localhost:4317"))
@ -846,6 +872,11 @@ public abstract class AbstractGrpcTelemetryExporterTest<T, U extends Message> {
.doesNotThrowAnyException();
}
private void buildAndShutdown(TelemetryExporterBuilder<T> builder) {
TelemetryExporter<T> build = builder.build();
build.shutdown().join(10, TimeUnit.MILLISECONDS);
}
@Test
@SuppressWarnings({"PreferJavaTimeOverload", "NullAway"})
void invalidConfig() {
@ -858,6 +889,25 @@ public abstract class AbstractGrpcTelemetryExporterTest<T, U extends Message> {
assertThatThrownBy(() -> exporterBuilder().setTimeout(null))
.isInstanceOf(NullPointerException.class)
.hasMessage("timeout");
assertThatThrownBy(
() ->
buildAndShutdown(exporterBuilder().setTimeout(Duration.ofSeconds(Long.MAX_VALUE))))
.isInstanceOf(ArithmeticException.class);
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(
() ->
buildAndShutdown(
exporterBuilder().setConnectTimeout(Duration.ofSeconds(Long.MAX_VALUE))))
.isInstanceOf(ArithmeticException.class);
assertThatThrownBy(() -> exporterBuilder().setEndpoint(null))
.isInstanceOf(NullPointerException.class)

49
exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/AbstractHttpTelemetryExporterTest.java
View File

@ -698,22 +698,39 @@ public abstract class AbstractHttpTelemetryExporterTest<T, U extends Message> {
@Test
@SuppressWarnings("PreferJavaTimeOverload")
void validConfig() {
assertThatCode(() -> exporterBuilder().setTimeout(0, TimeUnit.MILLISECONDS))
// We must build exporters to test timeout settings, which intersect with underlying client
// implementations and may convert between Duration, int, and long, which may be susceptible to
// overflow exceptions.
assertThatCode(() -> buildAndShutdown(exporterBuilder().setTimeout(0, TimeUnit.MILLISECONDS)))
.doesNotThrowAnyException();
assertThatCode(() -> exporterBuilder().setTimeout(Duration.ofMillis(0)))
assertThatCode(() -> buildAndShutdown(exporterBuilder().setTimeout(Duration.ofMillis(0))))
.doesNotThrowAnyException();
assertThatCode(() -> exporterBuilder().setTimeout(10, TimeUnit.MILLISECONDS))
assertThatCode(
() ->
buildAndShutdown(
exporterBuilder().setTimeout(Long.MAX_VALUE, TimeUnit.NANOSECONDS)))
.doesNotThrowAnyException();
assertThatCode(() -> exporterBuilder().setTimeout(Duration.ofMillis(10)))
assertThatCode(
() -> buildAndShutdown(exporterBuilder().setTimeout(Duration.ofNanos(Long.MAX_VALUE))))
.doesNotThrowAnyException();
assertThatCode(() -> exporterBuilder().setConnectTimeout(0, TimeUnit.MILLISECONDS))
assertThatCode(
() -> buildAndShutdown(exporterBuilder().setTimeout(Long.MAX_VALUE, TimeUnit.SECONDS)))
.doesNotThrowAnyException();
assertThatCode(() -> exporterBuilder().setConnectTimeout(Duration.ofMillis(0)))
assertThatCode(() -> buildAndShutdown(exporterBuilder().setTimeout(10, TimeUnit.MILLISECONDS)))
.doesNotThrowAnyException();
assertThatCode(() -> exporterBuilder().setConnectTimeout(10, TimeUnit.MILLISECONDS))
assertThatCode(() -> buildAndShutdown(exporterBuilder().setTimeout(Duration.ofMillis(10))))
.doesNotThrowAnyException();
assertThatCode(() -> exporterBuilder().setConnectTimeout(Duration.ofMillis(10)))
assertThatCode(
() -> buildAndShutdown(exporterBuilder().setConnectTimeout(0, TimeUnit.MILLISECONDS)))
.doesNotThrowAnyException();
assertThatCode(
() -> buildAndShutdown(exporterBuilder().setConnectTimeout(Duration.ofMillis(0))))
.doesNotThrowAnyException();
assertThatCode(
() -> buildAndShutdown(exporterBuilder().setConnectTimeout(10, TimeUnit.MILLISECONDS)))
.doesNotThrowAnyException();
assertThatCode(
() -> buildAndShutdown(exporterBuilder().setConnectTimeout(Duration.ofMillis(10))))
.doesNotThrowAnyException();
assertThatCode(() -> exporterBuilder().setEndpoint("http://localhost:4318"))
@ -738,6 +755,11 @@ public abstract class AbstractHttpTelemetryExporterTest<T, U extends Message> {
.doesNotThrowAnyException();
}
private void buildAndShutdown(TelemetryExporterBuilder<T> builder) {
TelemetryExporter<T> build = builder.build();
build.shutdown().join(10, TimeUnit.MILLISECONDS);
}
@Test
@SuppressWarnings({"PreferJavaTimeOverload", "NullAway"})
void invalidConfig() {
@ -750,6 +772,10 @@ public abstract class AbstractHttpTelemetryExporterTest<T, U extends Message> {
assertThatThrownBy(() -> exporterBuilder().setTimeout(null))
.isInstanceOf(NullPointerException.class)
.hasMessage("timeout");
assertThatThrownBy(
() ->
buildAndShutdown(exporterBuilder().setTimeout(Duration.ofSeconds(Long.MAX_VALUE))))
.isInstanceOf(ArithmeticException.class);
assertThatThrownBy(() -> exporterBuilder().setConnectTimeout(-1, TimeUnit.MILLISECONDS))
.isInstanceOf(IllegalArgumentException.class)
@ -760,6 +786,11 @@ public abstract class AbstractHttpTelemetryExporterTest<T, U extends Message> {
assertThatThrownBy(() -> exporterBuilder().setConnectTimeout(null))
.isInstanceOf(NullPointerException.class)
.hasMessage("timeout");
assertThatThrownBy(
() ->
buildAndShutdown(
exporterBuilder().setConnectTimeout(Duration.ofSeconds(Long.MAX_VALUE))))
.isInstanceOf(ArithmeticException.class);
assertThatThrownBy(() -> exporterBuilder().setEndpoint(null))
.isInstanceOf(NullPointerException.class)

8
exporters/sender/okhttp/src/main/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpGrpcSender.java
View File

@ -81,11 +81,15 @@ public final class OkHttpGrpcSender<T extends Marshaler> implements GrpcSender<T
@Nullable RetryPolicy retryPolicy,
@Nullable SSLContext sslContext,
@Nullable X509TrustManager trustManager) {
int callTimeoutMillis =
(int) Math.min(Duration.ofNanos(timeoutNanos).toMillis(), Integer.MAX_VALUE);
int connectTimeoutMillis =
(int) Math.min(Duration.ofNanos(connectTimeoutNanos).toMillis(), Integer.MAX_VALUE);
OkHttpClient.Builder clientBuilder =
new OkHttpClient.Builder()
.dispatcher(OkHttpUtil.newDispatcher())
.callTimeout(Duration.ofNanos(timeoutNanos))
.connectTimeout(Duration.ofNanos(connectTimeoutNanos));
.callTimeout(Duration.ofMillis(callTimeoutMillis))
.connectTimeout(Duration.ofMillis(connectTimeoutMillis));
if (retryPolicy != null) {
clientBuilder.addInterceptor(
new RetryInterceptor(retryPolicy, OkHttpGrpcSender::isRetryable));

8
exporters/sender/okhttp/src/main/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpHttpSender.java
View File

@ -64,11 +64,15 @@ public final class OkHttpHttpSender implements HttpSender {
@Nullable RetryPolicy retryPolicy,
@Nullable SSLContext sslContext,
@Nullable X509TrustManager trustManager) {
int callTimeoutMillis =
(int) Math.min(Duration.ofNanos(timeoutNanos).toMillis(), Integer.MAX_VALUE);
int connectTimeoutMillis =
(int) Math.min(Duration.ofNanos(connectionTimeoutNanos).toMillis(), Integer.MAX_VALUE);
OkHttpClient.Builder builder =
new OkHttpClient.Builder()
.dispatcher(OkHttpUtil.newDispatcher())
.connectTimeout(Duration.ofNanos(connectionTimeoutNanos))
.callTimeout(Duration.ofNanos(timeoutNanos));
.connectTimeout(Duration.ofMillis(connectTimeoutMillis))
.callTimeout(Duration.ofMillis(callTimeoutMillis));
if (proxyOptions != null) {
builder.proxySelector(proxyOptions.getProxySelector());

7
exporters/zipkin/src/main/java/io/opentelemetry/exporter/zipkin/ZipkinSpanExporterBuilder.java
View File

@ -31,7 +31,7 @@ public final class ZipkinSpanExporterBuilder {
// compression is enabled by default, because this is the default of OkHttpSender,
// which is created when no custom sender is set (see OkHttpSender.Builder)
private boolean compressionEnabled = true;
private long readTimeoutMillis = TimeUnit.SECONDS.toMillis(10);
private int readTimeoutMillis = (int) TimeUnit.SECONDS.toMillis(10);
private Supplier<MeterProvider> meterProviderSupplier = GlobalOpenTelemetry::getMeterProvider;
/**
@ -156,7 +156,8 @@ public final class ZipkinSpanExporterBuilder {
public ZipkinSpanExporterBuilder setReadTimeout(long timeout, TimeUnit unit) {
requireNonNull(unit, "unit");
checkArgument(timeout >= 0, "timeout must be non-negative");
this.readTimeoutMillis = unit.toMillis(timeout);
long timeoutMillis = timeout == 0 ? Long.MAX_VALUE : unit.toMillis(timeout);
this.readTimeoutMillis = (int) Math.min(timeoutMillis, Integer.MAX_VALUE);
return this;
}
@ -212,7 +213,7 @@ public final class ZipkinSpanExporterBuilder {
OkHttpSender.newBuilder()
.endpoint(endpoint)
.compressionEnabled(compressionEnabled)
.readTimeout((int) readTimeoutMillis)
.readTimeout(readTimeoutMillis)
.build();
}
OtelToZipkinSpanTransformer transformer =

15
exporters/zipkin/src/test/java/io/opentelemetry/exporter/zipkin/ZipkinSpanExporterTest.java
View File

@ -231,6 +231,21 @@ class ZipkinSpanExporterTest {
}
}
@Test
@SuppressWarnings("PreferJavaTimeOverload")
void readTimeout_Zero() {
ZipkinSpanExporter exporter =
ZipkinSpanExporter.builder().setReadTimeout(0, TimeUnit.SECONDS).build();
try {
assertThat(exporter)
.extracting("sender.delegate.client.readTimeoutMillis")
.isEqualTo(Integer.MAX_VALUE);
} finally {
exporter.shutdown();
}
}
@Test
void stringRepresentation() {
try (ZipkinSpanExporter exporter = ZipkinSpanExporter.builder().build()) {

2
sdk/logs/src/main/java/io/opentelemetry/sdk/logs/export/BatchLogRecordProcessorBuilder.java
View File

@ -71,7 +71,7 @@ public final class BatchLogRecordProcessorBuilder {
public BatchLogRecordProcessorBuilder setExporterTimeout(long timeout, TimeUnit unit) {
requireNonNull(unit, "unit");
checkArgument(timeout >= 0, "timeout must be non-negative");
exporterTimeoutNanos = unit.toNanos(timeout);
exporterTimeoutNanos = timeout == 0 ? Long.MAX_VALUE : unit.toNanos(timeout);
return this;
}

2
sdk/trace/src/main/java/io/opentelemetry/sdk/trace/export/BatchSpanProcessorBuilder.java
View File

@ -79,7 +79,7 @@ public final class BatchSpanProcessorBuilder {
public BatchSpanProcessorBuilder setExporterTimeout(long timeout, TimeUnit unit) {
requireNonNull(unit, "unit");
checkArgument(timeout >= 0, "timeout must be non-negative");
exporterTimeoutNanos = unit.toNanos(timeout);
exporterTimeoutNanos = timeout == 0 ? Long.MAX_VALUE : unit.toNanos(timeout);
return this;
}