From 5ad8be793ecb94a9b730668482bc5c430a8491c3 Mon Sep 17 00:00:00 2001 From: Jack Berg Date: Tue, 5 Nov 2024 13:44:23 -0600 Subject: [PATCH] Require exporter timeouts to be positive --- .../logs/OtlpHttpLogRecordExporterBuilder.java | 4 ++-- .../metrics/OtlpHttpMetricExporterBuilder.java | 4 ++-- .../http/trace/OtlpHttpSpanExporterBuilder.java | 4 ++-- .../logs/OtlpGrpcLogRecordExporterBuilder.java | 4 ++-- .../metrics/OtlpGrpcMetricExporterBuilder.java | 4 ++-- .../otlp/trace/OtlpGrpcSpanExporterBuilder.java | 4 ++-- .../AbstractGrpcTelemetryExporterTest.java | 8 ++++---- .../AbstractHttpTelemetryExporterTest.java | 16 ++++++++-------- .../zipkin/ZipkinSpanExporterBuilder.java | 2 +- .../exporter/zipkin/ZipkinSpanExporterTest.java | 4 ++-- .../export/BatchLogRecordProcessorBuilder.java | 2 +- .../logs/export/BatchLogRecordProcessorTest.java | 4 ++-- .../trace/export/BatchSpanProcessorBuilder.java | 2 +- .../sdk/trace/export/BatchSpanProcessorTest.java | 4 ++-- 14 files changed, 33 insertions(+), 33 deletions(-) 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 0602cf2b506..ca44d563066 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 @@ -56,7 +56,7 @@ public final class OtlpHttpLogRecordExporterBuilder { */ public OtlpHttpLogRecordExporterBuilder setTimeout(long timeout, TimeUnit unit) { requireNonNull(unit, "unit"); - checkArgument(timeout >= 0, "timeout must be non-negative"); + checkArgument(timeout > 0, "timeout must be positive"); delegate.setTimeout(timeout, unit); return this; } @@ -78,7 +78,7 @@ public OtlpHttpLogRecordExporterBuilder setTimeout(Duration timeout) { */ public OtlpHttpLogRecordExporterBuilder setConnectTimeout(long timeout, TimeUnit unit) { requireNonNull(unit, "unit"); - checkArgument(timeout >= 0, "timeout must be non-negative"); + checkArgument(timeout > 0, "timeout must be positive"); delegate.setConnectTimeout(timeout, unit); return this; } 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 e0cb6f4a493..f4197959af6 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 @@ -70,7 +70,7 @@ public final class OtlpHttpMetricExporterBuilder { */ public OtlpHttpMetricExporterBuilder setTimeout(long timeout, TimeUnit unit) { requireNonNull(unit, "unit"); - checkArgument(timeout >= 0, "timeout must be non-negative"); + checkArgument(timeout > 0, "timeout must be positive"); delegate.setTimeout(timeout, unit); return this; } @@ -92,7 +92,7 @@ public OtlpHttpMetricExporterBuilder setTimeout(Duration timeout) { */ public OtlpHttpMetricExporterBuilder setConnectTimeout(long timeout, TimeUnit unit) { requireNonNull(unit, "unit"); - checkArgument(timeout >= 0, "timeout must be non-negative"); + checkArgument(timeout > 0, "timeout must be positive"); delegate.setConnectTimeout(timeout, unit); return this; } 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 fb547af942a..7ab86348e86 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 @@ -56,7 +56,7 @@ public final class OtlpHttpSpanExporterBuilder { */ public OtlpHttpSpanExporterBuilder setTimeout(long timeout, TimeUnit unit) { requireNonNull(unit, "unit"); - checkArgument(timeout >= 0, "timeout must be non-negative"); + checkArgument(timeout > 0, "timeout must be positive"); delegate.setTimeout(timeout, unit); return this; } @@ -78,7 +78,7 @@ public OtlpHttpSpanExporterBuilder setTimeout(Duration timeout) { */ public OtlpHttpSpanExporterBuilder setConnectTimeout(long timeout, TimeUnit unit) { requireNonNull(unit, "unit"); - checkArgument(timeout >= 0, "timeout must be non-negative"); + checkArgument(timeout > 0, "timeout must be positive"); delegate.setConnectTimeout(timeout, unit); return this; } diff --git a/exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/logs/OtlpGrpcLogRecordExporterBuilder.java b/exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/logs/OtlpGrpcLogRecordExporterBuilder.java index 2ab6e29be09..fe41bfac0a1 100644 --- a/exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/logs/OtlpGrpcLogRecordExporterBuilder.java +++ b/exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/logs/OtlpGrpcLogRecordExporterBuilder.java @@ -92,7 +92,7 @@ public OtlpGrpcLogRecordExporterBuilder setChannel(ManagedChannel channel) { */ public OtlpGrpcLogRecordExporterBuilder setTimeout(long timeout, TimeUnit unit) { requireNonNull(unit, "unit"); - checkArgument(timeout >= 0, "timeout must be non-negative"); + checkArgument(timeout > 0, "timeout must be positive"); delegate.setTimeout(timeout, unit); return this; } @@ -115,7 +115,7 @@ public OtlpGrpcLogRecordExporterBuilder setTimeout(Duration timeout) { */ public OtlpGrpcLogRecordExporterBuilder setConnectTimeout(long timeout, TimeUnit unit) { requireNonNull(unit, "unit"); - checkArgument(timeout >= 0, "timeout must be non-negative"); + checkArgument(timeout > 0, "timeout must be positive"); delegate.setConnectTimeout(timeout, unit); return this; } diff --git a/exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/metrics/OtlpGrpcMetricExporterBuilder.java b/exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/metrics/OtlpGrpcMetricExporterBuilder.java index c6f02dec4c7..a2230d4c188 100644 --- a/exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/metrics/OtlpGrpcMetricExporterBuilder.java +++ b/exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/metrics/OtlpGrpcMetricExporterBuilder.java @@ -106,7 +106,7 @@ public OtlpGrpcMetricExporterBuilder setChannel(ManagedChannel channel) { */ public OtlpGrpcMetricExporterBuilder setTimeout(long timeout, TimeUnit unit) { requireNonNull(unit, "unit"); - checkArgument(timeout >= 0, "timeout must be non-negative"); + checkArgument(timeout > 0, "timeout must be positive"); delegate.setTimeout(timeout, unit); return this; } @@ -129,7 +129,7 @@ public OtlpGrpcMetricExporterBuilder setTimeout(Duration timeout) { */ public OtlpGrpcMetricExporterBuilder setConnectTimeout(long timeout, TimeUnit unit) { requireNonNull(unit, "unit"); - checkArgument(timeout >= 0, "timeout must be non-negative"); + checkArgument(timeout > 0, "timeout must be positive"); delegate.setConnectTimeout(timeout, unit); return this; } diff --git a/exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/trace/OtlpGrpcSpanExporterBuilder.java b/exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/trace/OtlpGrpcSpanExporterBuilder.java index 0a24398eeed..1a4939dcfe5 100644 --- a/exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/trace/OtlpGrpcSpanExporterBuilder.java +++ b/exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/trace/OtlpGrpcSpanExporterBuilder.java @@ -88,7 +88,7 @@ public OtlpGrpcSpanExporterBuilder setChannel(ManagedChannel channel) { */ public OtlpGrpcSpanExporterBuilder setTimeout(long timeout, TimeUnit unit) { requireNonNull(unit, "unit"); - checkArgument(timeout >= 0, "timeout must be non-negative"); + checkArgument(timeout > 0, "timeout must be positive"); delegate.setTimeout(timeout, unit); return this; } @@ -111,7 +111,7 @@ public OtlpGrpcSpanExporterBuilder setTimeout(Duration timeout) { */ public OtlpGrpcSpanExporterBuilder setConnectTimeout(long timeout, TimeUnit unit) { requireNonNull(unit, "unit"); - checkArgument(timeout >= 0, "timeout must be non-negative"); + checkArgument(timeout > 0, "timeout must be positive"); delegate.setConnectTimeout(timeout, unit); return this; } diff --git a/exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/AbstractGrpcTelemetryExporterTest.java b/exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/AbstractGrpcTelemetryExporterTest.java index 4dcacfb329b..2a35470c9e7 100644 --- a/exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/AbstractGrpcTelemetryExporterTest.java +++ b/exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/AbstractGrpcTelemetryExporterTest.java @@ -814,9 +814,9 @@ void overrideHost() { @Test @SuppressWarnings("PreferJavaTimeOverload") void validConfig() { - assertThatCode(() -> exporterBuilder().setTimeout(0, TimeUnit.MILLISECONDS)) + assertThatCode(() -> exporterBuilder().setTimeout(1, TimeUnit.MILLISECONDS)) .doesNotThrowAnyException(); - assertThatCode(() -> exporterBuilder().setTimeout(Duration.ofMillis(0))) + assertThatCode(() -> exporterBuilder().setTimeout(Duration.ofMillis(1))) .doesNotThrowAnyException(); assertThatCode(() -> exporterBuilder().setTimeout(10, TimeUnit.MILLISECONDS)) .doesNotThrowAnyException(); @@ -848,9 +848,9 @@ void validConfig() { @Test @SuppressWarnings({"PreferJavaTimeOverload", "NullAway"}) void invalidConfig() { - assertThatThrownBy(() -> exporterBuilder().setTimeout(-1, TimeUnit.MILLISECONDS)) + assertThatThrownBy(() -> exporterBuilder().setTimeout(0, TimeUnit.MILLISECONDS)) .isInstanceOf(IllegalArgumentException.class) - .hasMessage("timeout must be non-negative"); + .hasMessage("timeout must be positive"); assertThatThrownBy(() -> exporterBuilder().setTimeout(1, null)) .isInstanceOf(NullPointerException.class) .hasMessage("unit"); 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 7f6bfff04b3..061647b0353 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 @@ -728,18 +728,18 @@ void proxy() { @Test @SuppressWarnings("PreferJavaTimeOverload") void validConfig() { - assertThatCode(() -> exporterBuilder().setTimeout(0, TimeUnit.MILLISECONDS)) + assertThatCode(() -> exporterBuilder().setTimeout(1, TimeUnit.MILLISECONDS)) .doesNotThrowAnyException(); - assertThatCode(() -> exporterBuilder().setTimeout(Duration.ofMillis(0))) + assertThatCode(() -> exporterBuilder().setTimeout(Duration.ofMillis(1))) .doesNotThrowAnyException(); assertThatCode(() -> exporterBuilder().setTimeout(10, TimeUnit.MILLISECONDS)) .doesNotThrowAnyException(); assertThatCode(() -> exporterBuilder().setTimeout(Duration.ofMillis(10))) .doesNotThrowAnyException(); - assertThatCode(() -> exporterBuilder().setConnectTimeout(0, TimeUnit.MILLISECONDS)) + assertThatCode(() -> exporterBuilder().setConnectTimeout(1, TimeUnit.MILLISECONDS)) .doesNotThrowAnyException(); - assertThatCode(() -> exporterBuilder().setConnectTimeout(Duration.ofMillis(0))) + assertThatCode(() -> exporterBuilder().setConnectTimeout(Duration.ofMillis(1))) .doesNotThrowAnyException(); assertThatCode(() -> exporterBuilder().setConnectTimeout(10, TimeUnit.MILLISECONDS)) .doesNotThrowAnyException(); @@ -771,9 +771,9 @@ void validConfig() { @Test @SuppressWarnings({"PreferJavaTimeOverload", "NullAway"}) void invalidConfig() { - assertThatThrownBy(() -> exporterBuilder().setTimeout(-1, TimeUnit.MILLISECONDS)) + assertThatThrownBy(() -> exporterBuilder().setTimeout(0, TimeUnit.MILLISECONDS)) .isInstanceOf(IllegalArgumentException.class) - .hasMessage("timeout must be non-negative"); + .hasMessage("timeout must be positive"); assertThatThrownBy(() -> exporterBuilder().setTimeout(1, null)) .isInstanceOf(NullPointerException.class) .hasMessage("unit"); @@ -781,9 +781,9 @@ void invalidConfig() { .isInstanceOf(NullPointerException.class) .hasMessage("timeout"); - assertThatThrownBy(() -> exporterBuilder().setConnectTimeout(-1, TimeUnit.MILLISECONDS)) + assertThatThrownBy(() -> exporterBuilder().setConnectTimeout(0, TimeUnit.MILLISECONDS)) .isInstanceOf(IllegalArgumentException.class) - .hasMessage("timeout must be non-negative"); + .hasMessage("timeout must be positive"); assertThatThrownBy(() -> exporterBuilder().setConnectTimeout(1, null)) .isInstanceOf(NullPointerException.class) .hasMessage("unit"); 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 d13f1d4a825..8c5d126b1d5 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 @@ -155,7 +155,7 @@ public ZipkinSpanExporterBuilder setCompression(String compressionMethod) { */ public ZipkinSpanExporterBuilder setReadTimeout(long timeout, TimeUnit unit) { requireNonNull(unit, "unit"); - checkArgument(timeout >= 0, "timeout must be non-negative"); + checkArgument(timeout > 0, "timeout must be positive"); this.readTimeoutMillis = unit.toMillis(timeout); return this; } 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 2c70691744c..6e1f1d3c548 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 @@ -137,9 +137,9 @@ void testShutdown() throws IOException { @SuppressWarnings({"PreferJavaTimeOverload", "deprecation"}) // we have to use the deprecated setEncoder overload to test it void invalidConfig() { - assertThatThrownBy(() -> ZipkinSpanExporter.builder().setReadTimeout(-1, TimeUnit.MILLISECONDS)) + assertThatThrownBy(() -> ZipkinSpanExporter.builder().setReadTimeout(0, TimeUnit.MILLISECONDS)) .isInstanceOf(IllegalArgumentException.class) - .hasMessage("timeout must be non-negative"); + .hasMessage("timeout must be positive"); assertThatThrownBy(() -> ZipkinSpanExporter.builder().setReadTimeout(1, null)) .isInstanceOf(NullPointerException.class) diff --git a/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/export/BatchLogRecordProcessorBuilder.java b/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/export/BatchLogRecordProcessorBuilder.java index 6aafa9525aa..86c67ca6cde 100644 --- a/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/export/BatchLogRecordProcessorBuilder.java +++ b/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/export/BatchLogRecordProcessorBuilder.java @@ -70,7 +70,7 @@ long getScheduleDelayNanos() { */ public BatchLogRecordProcessorBuilder setExporterTimeout(long timeout, TimeUnit unit) { requireNonNull(unit, "unit"); - checkArgument(timeout >= 0, "timeout must be non-negative"); + checkArgument(timeout > 0, "timeout must be positive"); exporterTimeoutNanos = unit.toNanos(timeout); return this; } diff --git a/sdk/logs/src/test/java/io/opentelemetry/sdk/logs/export/BatchLogRecordProcessorTest.java b/sdk/logs/src/test/java/io/opentelemetry/sdk/logs/export/BatchLogRecordProcessorTest.java index 4c17a1768f7..22c420ea45f 100644 --- a/sdk/logs/src/test/java/io/opentelemetry/sdk/logs/export/BatchLogRecordProcessorTest.java +++ b/sdk/logs/src/test/java/io/opentelemetry/sdk/logs/export/BatchLogRecordProcessorTest.java @@ -99,9 +99,9 @@ void builderInvalidConfig() { assertThatThrownBy( () -> BatchLogRecordProcessor.builder(mockLogRecordExporter) - .setExporterTimeout(-1, TimeUnit.MILLISECONDS)) + .setExporterTimeout(0, TimeUnit.MILLISECONDS)) .isInstanceOf(IllegalArgumentException.class) - .hasMessage("timeout must be non-negative"); + .hasMessage("timeout must be positive"); assertThatThrownBy( () -> BatchLogRecordProcessor.builder(mockLogRecordExporter).setExporterTimeout(1, null)) diff --git a/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/export/BatchSpanProcessorBuilder.java b/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/export/BatchSpanProcessorBuilder.java index c3235a60f4f..8f7458488b8 100644 --- a/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/export/BatchSpanProcessorBuilder.java +++ b/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/export/BatchSpanProcessorBuilder.java @@ -78,7 +78,7 @@ long getScheduleDelayNanos() { */ public BatchSpanProcessorBuilder setExporterTimeout(long timeout, TimeUnit unit) { requireNonNull(unit, "unit"); - checkArgument(timeout >= 0, "timeout must be non-negative"); + checkArgument(timeout > 0, "timeout must be positive"); exporterTimeoutNanos = unit.toNanos(timeout); return this; } diff --git a/sdk/trace/src/test/java/io/opentelemetry/sdk/trace/export/BatchSpanProcessorTest.java b/sdk/trace/src/test/java/io/opentelemetry/sdk/trace/export/BatchSpanProcessorTest.java index d9f48ecab17..d43d28d12e9 100644 --- a/sdk/trace/src/test/java/io/opentelemetry/sdk/trace/export/BatchSpanProcessorTest.java +++ b/sdk/trace/src/test/java/io/opentelemetry/sdk/trace/export/BatchSpanProcessorTest.java @@ -111,9 +111,9 @@ void builderInvalidConfig() { assertThatThrownBy( () -> BatchSpanProcessor.builder(mockSpanExporter) - .setExporterTimeout(-1, TimeUnit.MILLISECONDS)) + .setExporterTimeout(0, TimeUnit.MILLISECONDS)) .isInstanceOf(IllegalArgumentException.class) - .hasMessage("timeout must be non-negative"); + .hasMessage("timeout must be positive"); assertThatThrownBy( () -> BatchSpanProcessor.builder(mockSpanExporter).setExporterTimeout(1, null)) .isInstanceOf(NullPointerException.class)