From 03d56577af2f72627e4ff4bc8b3f30bb157b6046 Mon Sep 17 00:00:00 2001 From: Onur Kayabasi Date: Fri, 17 Jan 2025 07:26:27 +0100 Subject: [PATCH] Follow spec on span limits, batch processors --- .../TracerProviderConfigurationTest.java | 4 ++-- .../LoggerProviderConfigurationTest.java | 4 ++-- .../io/opentelemetry/sdk/logs/LogLimitsBuilder.java | 4 ++-- .../logs/export/BatchLogRecordProcessorBuilder.java | 4 ++++ .../io/opentelemetry/sdk/logs/LogLimitsTest.java | 2 +- .../sdk/logs/export/BatchLogRecordProcessorTest.java | 8 ++++++++ .../opentelemetry/sdk/trace/SpanLimitsBuilder.java | 12 ++++++------ .../sdk/trace/export/BatchSpanProcessorBuilder.java | 4 ++++ .../sdk/trace/config/SpanLimitsTest.java | 10 +++++----- .../sdk/trace/export/BatchSpanProcessorTest.java | 7 +++++++ 10 files changed, 41 insertions(+), 18 deletions(-) diff --git a/sdk-extensions/autoconfigure/src/test/java/io/opentelemetry/sdk/autoconfigure/TracerProviderConfigurationTest.java b/sdk-extensions/autoconfigure/src/test/java/io/opentelemetry/sdk/autoconfigure/TracerProviderConfigurationTest.java index d12ab1744f4..e0ddcb438fe 100644 --- a/sdk-extensions/autoconfigure/src/test/java/io/opentelemetry/sdk/autoconfigure/TracerProviderConfigurationTest.java +++ b/sdk-extensions/autoconfigure/src/test/java/io/opentelemetry/sdk/autoconfigure/TracerProviderConfigurationTest.java @@ -126,7 +126,7 @@ void configureBatchSpanProcessor_configured() { Map properties = new HashMap<>(); properties.put("otel.bsp.schedule.delay", "100000"); properties.put("otel.bsp.max.queue.size", "2"); - properties.put("otel.bsp.max.export.batch.size", "3"); + properties.put("otel.bsp.max.export.batch.size", "2"); properties.put("otel.bsp.export.timeout", "4"); try (BatchSpanProcessor processor = @@ -144,7 +144,7 @@ void configureBatchSpanProcessor_configured() { assertThat(worker) .extracting("exporterTimeoutNanos") .isEqualTo(TimeUnit.MILLISECONDS.toNanos(4)); - assertThat(worker).extracting("maxExportBatchSize").isEqualTo(3); + assertThat(worker).extracting("maxExportBatchSize").isEqualTo(2); assertThat(worker) .extracting("queue") .isInstanceOfSatisfying( diff --git a/sdk-extensions/autoconfigure/src/testFullConfig/java/io/opentelemetry/sdk/autoconfigure/LoggerProviderConfigurationTest.java b/sdk-extensions/autoconfigure/src/testFullConfig/java/io/opentelemetry/sdk/autoconfigure/LoggerProviderConfigurationTest.java index d3856cd93c2..7a5473e8dea 100644 --- a/sdk-extensions/autoconfigure/src/testFullConfig/java/io/opentelemetry/sdk/autoconfigure/LoggerProviderConfigurationTest.java +++ b/sdk-extensions/autoconfigure/src/testFullConfig/java/io/opentelemetry/sdk/autoconfigure/LoggerProviderConfigurationTest.java @@ -118,7 +118,7 @@ void configureBatchLogRecordProcessor() { Map properties = new HashMap<>(); properties.put("otel.blrp.schedule.delay", "100000"); properties.put("otel.blrp.max.queue.size", "2"); - properties.put("otel.blrp.max.export.batch.size", "3"); + properties.put("otel.blrp.max.export.batch.size", "2"); properties.put("otel.blrp.export.timeout", "4"); try (BatchLogRecordProcessor processor = @@ -136,7 +136,7 @@ void configureBatchLogRecordProcessor() { assertThat(worker) .extracting("exporterTimeoutNanos") .isEqualTo(TimeUnit.MILLISECONDS.toNanos(4)); - assertThat(worker).extracting("maxExportBatchSize").isEqualTo(3); + assertThat(worker).extracting("maxExportBatchSize").isEqualTo(2); assertThat(worker) .extracting("queue") .isInstanceOfSatisfying( diff --git a/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/LogLimitsBuilder.java b/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/LogLimitsBuilder.java index 22650d4d896..f4f74bf5ac3 100644 --- a/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/LogLimitsBuilder.java +++ b/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/LogLimitsBuilder.java @@ -32,7 +32,7 @@ public final class LogLimitsBuilder { * @throws IllegalArgumentException if {@code maxNumberOfAttributes} is not positive. */ public LogLimitsBuilder setMaxNumberOfAttributes(int maxNumberOfAttributes) { - Utils.checkArgument(maxNumberOfAttributes > 0, "maxNumberOfAttributes must be greater than 0"); + Utils.checkArgument(maxNumberOfAttributes >= 0, "maxNumberOfAttributes must be non-negative"); this.maxNumAttributes = maxNumberOfAttributes; return this; } @@ -48,7 +48,7 @@ public LogLimitsBuilder setMaxNumberOfAttributes(int maxNumberOfAttributes) { */ public LogLimitsBuilder setMaxAttributeValueLength(int maxAttributeValueLength) { Utils.checkArgument( - maxAttributeValueLength > -1, "maxAttributeValueLength must be non-negative"); + maxAttributeValueLength >= 0, "maxAttributeValueLength must be non-negative"); this.maxAttributeValueLength = maxAttributeValueLength; return this; } 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..9810a3af230 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 @@ -98,9 +98,11 @@ long getExporterTimeoutNanos() { * @param maxQueueSize the maximum number of Logs that are kept in the queue before start * dropping. * @return this. + * @throws IllegalArgumentException if {@code maxQueueSize} is not positive. * @see BatchLogRecordProcessorBuilder#DEFAULT_MAX_QUEUE_SIZE */ public BatchLogRecordProcessorBuilder setMaxQueueSize(int maxQueueSize) { + checkArgument(maxQueueSize > 0, "maxQueueSize must be positive."); this.maxQueueSize = maxQueueSize; return this; } @@ -146,8 +148,10 @@ int getMaxExportBatchSize() { * {@code logRecordExporter}. * * @return a new {@link BatchLogRecordProcessor}. + * @throws IllegalArgumentException if {@code maxExportBatchSize} is greater than {@code maxQueueSize}. */ public BatchLogRecordProcessor build() { + checkArgument(maxExportBatchSize <= maxQueueSize, "maxExportBatchSize must be smaller or equal to maxQueueSize."); return new BatchLogRecordProcessor( logRecordExporter, meterProvider, diff --git a/sdk/logs/src/test/java/io/opentelemetry/sdk/logs/LogLimitsTest.java b/sdk/logs/src/test/java/io/opentelemetry/sdk/logs/LogLimitsTest.java index 250d49f8402..823387965cc 100644 --- a/sdk/logs/src/test/java/io/opentelemetry/sdk/logs/LogLimitsTest.java +++ b/sdk/logs/src/test/java/io/opentelemetry/sdk/logs/LogLimitsTest.java @@ -33,7 +33,7 @@ void updateLogLimits_All() { @Test void invalidLogLimits() { - assertThatThrownBy(() -> LogLimits.builder().setMaxNumberOfAttributes(0)) + assertThatThrownBy(() -> LogLimits.builder().setMaxNumberOfAttributes(-1)) .isInstanceOf(IllegalArgumentException.class); assertThatThrownBy(() -> LogLimits.builder().setMaxNumberOfAttributes(-1)) .isInstanceOf(IllegalArgumentException.class); 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..97e5b117d63 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 @@ -111,6 +111,13 @@ void builderInvalidConfig() { () -> BatchLogRecordProcessor.builder(mockLogRecordExporter).setExporterTimeout(null)) .isInstanceOf(NullPointerException.class) .hasMessage("timeout"); + assertThatThrownBy(() -> BatchLogRecordProcessor.builder(mockLogRecordExporter).setMaxQueueSize(0)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("maxQueueSize must be positive."); + assertThatThrownBy( + () -> BatchLogRecordProcessor.builder(mockLogRecordExporter).setMaxQueueSize(1).setMaxExportBatchSize(2).build()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("maxExportBatchSize must be smaller or equal to maxQueueSize."); } @Test @@ -337,6 +344,7 @@ public void continuesIfExporterTimesOut() throws InterruptedException { .setExporterTimeout(exporterTimeoutMillis, TimeUnit.MILLISECONDS) .setScheduleDelay(1, TimeUnit.MILLISECONDS) .setMaxQueueSize(1) + .setMaxExportBatchSize(1) .build(); SdkLoggerProvider sdkLoggerProvider = SdkLoggerProvider.builder().addLogRecordProcessor(blp).build(); diff --git a/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/SpanLimitsBuilder.java b/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/SpanLimitsBuilder.java index 14b5f06f1a6..353bc19b044 100644 --- a/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/SpanLimitsBuilder.java +++ b/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/SpanLimitsBuilder.java @@ -34,7 +34,7 @@ public final class SpanLimitsBuilder { * @throws IllegalArgumentException if {@code maxNumberOfAttributes} is not positive. */ public SpanLimitsBuilder setMaxNumberOfAttributes(int maxNumberOfAttributes) { - Utils.checkArgument(maxNumberOfAttributes > 0, "maxNumberOfAttributes must be greater than 0"); + Utils.checkArgument(maxNumberOfAttributes >= 0, "maxNumberOfAttributes must be non-negative"); this.maxNumAttributes = maxNumberOfAttributes; return this; } @@ -47,7 +47,7 @@ public SpanLimitsBuilder setMaxNumberOfAttributes(int maxNumberOfAttributes) { * @throws IllegalArgumentException if {@code maxNumberOfEvents} is not positive. */ public SpanLimitsBuilder setMaxNumberOfEvents(int maxNumberOfEvents) { - Utils.checkArgument(maxNumberOfEvents > 0, "maxNumberOfEvents must be greater than 0"); + Utils.checkArgument(maxNumberOfEvents >= 0, "maxNumberOfEvents must be non-negative"); this.maxNumEvents = maxNumberOfEvents; return this; } @@ -60,7 +60,7 @@ public SpanLimitsBuilder setMaxNumberOfEvents(int maxNumberOfEvents) { * @throws IllegalArgumentException if {@code maxNumberOfLinks} is not positive. */ public SpanLimitsBuilder setMaxNumberOfLinks(int maxNumberOfLinks) { - Utils.checkArgument(maxNumberOfLinks > 0, "maxNumberOfLinks must be greater than 0"); + Utils.checkArgument(maxNumberOfLinks >= 0, "maxNumberOfLinks must be non-negative"); this.maxNumLinks = maxNumberOfLinks; return this; } @@ -74,7 +74,7 @@ public SpanLimitsBuilder setMaxNumberOfLinks(int maxNumberOfLinks) { */ public SpanLimitsBuilder setMaxNumberOfAttributesPerEvent(int maxNumberOfAttributesPerEvent) { Utils.checkArgument( - maxNumberOfAttributesPerEvent > 0, "maxNumberOfAttributesPerEvent must be greater than 0"); + maxNumberOfAttributesPerEvent >= 0, "maxNumberOfAttributesPerEvent must be non-negative"); this.maxNumAttributesPerEvent = maxNumberOfAttributesPerEvent; return this; } @@ -88,7 +88,7 @@ public SpanLimitsBuilder setMaxNumberOfAttributesPerEvent(int maxNumberOfAttribu */ public SpanLimitsBuilder setMaxNumberOfAttributesPerLink(int maxNumberOfAttributesPerLink) { Utils.checkArgument( - maxNumberOfAttributesPerLink > 0, "maxNumberOfAttributesPerLink must be greater than 0"); + maxNumberOfAttributesPerLink >= 0, "maxNumberOfAttributesPerLink must be non-negative"); this.maxNumAttributesPerLink = maxNumberOfAttributesPerLink; return this; } @@ -104,7 +104,7 @@ public SpanLimitsBuilder setMaxNumberOfAttributesPerLink(int maxNumberOfAttribut */ public SpanLimitsBuilder setMaxAttributeValueLength(int maxAttributeValueLength) { Utils.checkArgument( - maxAttributeValueLength > -1, "maxAttributeValueLength must be non-negative"); + maxAttributeValueLength >= 0, "maxAttributeValueLength must be non-negative"); this.maxAttributeValueLength = maxAttributeValueLength; return this; } 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..b0365d3a1b3 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 @@ -106,9 +106,11 @@ long getExporterTimeoutNanos() { * @param maxQueueSize the maximum number of Spans that are kept in the queue before start * dropping. * @return this. + * @throws IllegalArgumentException if {@code maxQueueSize} is not positive. * @see BatchSpanProcessorBuilder#DEFAULT_MAX_QUEUE_SIZE */ public BatchSpanProcessorBuilder setMaxQueueSize(int maxQueueSize) { + checkArgument(maxQueueSize > 0, "maxQueueSize must be positive."); this.maxQueueSize = maxQueueSize; return this; } @@ -154,8 +156,10 @@ int getMaxExportBatchSize() { * forwards them to the given {@code spanExporter}. * * @return a new {@link BatchSpanProcessor}. + * @throws IllegalArgumentException if {@code maxExportBatchSize} is greater than {@code maxQueueSize}. */ public BatchSpanProcessor build() { + checkArgument(maxExportBatchSize <= maxQueueSize, "maxExportBatchSize must be smaller or equal to maxQueueSize."); return new BatchSpanProcessor( spanExporter, exportUnsampledSpans, diff --git a/sdk/trace/src/test/java/io/opentelemetry/sdk/trace/config/SpanLimitsTest.java b/sdk/trace/src/test/java/io/opentelemetry/sdk/trace/config/SpanLimitsTest.java index ab749cea0f5..59862788d64 100644 --- a/sdk/trace/src/test/java/io/opentelemetry/sdk/trace/config/SpanLimitsTest.java +++ b/sdk/trace/src/test/java/io/opentelemetry/sdk/trace/config/SpanLimitsTest.java @@ -46,23 +46,23 @@ void updateSpanLimits_All() { @Test void invalidSpanLimits() { - assertThatThrownBy(() -> SpanLimits.builder().setMaxNumberOfAttributes(0)) + assertThatThrownBy(() -> SpanLimits.builder().setMaxNumberOfAttributes(-1)) .isInstanceOf(IllegalArgumentException.class); assertThatThrownBy(() -> SpanLimits.builder().setMaxNumberOfAttributes(-1)) .isInstanceOf(IllegalArgumentException.class); - assertThatThrownBy(() -> SpanLimits.builder().setMaxNumberOfEvents(0)) + assertThatThrownBy(() -> SpanLimits.builder().setMaxNumberOfEvents(-1)) .isInstanceOf(IllegalArgumentException.class); assertThatThrownBy(() -> SpanLimits.builder().setMaxNumberOfEvents(-1)) .isInstanceOf(IllegalArgumentException.class); - assertThatThrownBy(() -> SpanLimits.builder().setMaxNumberOfLinks(0)) + assertThatThrownBy(() -> SpanLimits.builder().setMaxNumberOfLinks(-1)) .isInstanceOf(IllegalArgumentException.class); assertThatThrownBy(() -> SpanLimits.builder().setMaxNumberOfLinks(-1)) .isInstanceOf(IllegalArgumentException.class); - assertThatThrownBy(() -> SpanLimits.builder().setMaxNumberOfAttributesPerEvent(0)) + assertThatThrownBy(() -> SpanLimits.builder().setMaxNumberOfAttributesPerEvent(-1)) .isInstanceOf(IllegalArgumentException.class); assertThatThrownBy(() -> SpanLimits.builder().setMaxNumberOfAttributesPerEvent(-1)) .isInstanceOf(IllegalArgumentException.class); - assertThatThrownBy(() -> SpanLimits.builder().setMaxNumberOfAttributesPerLink(0)) + assertThatThrownBy(() -> SpanLimits.builder().setMaxNumberOfAttributesPerLink(-1)) .isInstanceOf(IllegalArgumentException.class); assertThatThrownBy(() -> SpanLimits.builder().setMaxNumberOfAttributesPerLink(-1)) .isInstanceOf(IllegalArgumentException.class); 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..d6a31a050a4 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 @@ -121,6 +121,12 @@ void builderInvalidConfig() { assertThatThrownBy(() -> BatchSpanProcessor.builder(mockSpanExporter).setExporterTimeout(null)) .isInstanceOf(NullPointerException.class) .hasMessage("timeout"); + assertThatThrownBy(() -> BatchSpanProcessor.builder(mockSpanExporter).setMaxQueueSize(0)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("maxQueueSize must be positive."); + assertThatThrownBy(() -> BatchSpanProcessor.builder(mockSpanExporter).setMaxQueueSize(1).setMaxExportBatchSize(2).build()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("maxExportBatchSize must be smaller or equal to maxQueueSize."); } @Test @@ -419,6 +425,7 @@ public void continuesIfExporterTimesOut() throws InterruptedException { .setExporterTimeout(exporterTimeoutMillis, TimeUnit.MILLISECONDS) .setScheduleDelay(1, TimeUnit.MILLISECONDS) .setMaxQueueSize(1) + .setMaxExportBatchSize(1) .build(); sdkTracerProvider = SdkTracerProvider.builder().addSpanProcessor(bsp).build();