Skip to content

Follow spec on span limits, batch processors #7030

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.

Sign up for GitHub

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

Merged
jack-berg merged 3 commits into from
Jan 28, 2025
Merged

Follow spec on span limits, batch processors #7030

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
03d5657
Follow spec on span limits, batch processors
onurkybsi Jan 17, 2025
a7d9819
Formatting fixed
onurkybsi Jan 17, 2025
e6e26f9
PR comments addressed
onurkybsi Jan 28, 2025
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
4 changes: 2 additions & 2 deletions ...ure/src/test/java/io/opentelemetry/sdk/autoconfigure/TracerProviderConfigurationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ void configureBatchSpanProcessor_configured() {
Map<String, String> 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 =
Expand All @@ -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(
Expand Down
4 changes: 2 additions & 2 deletions ...stFullConfig/java/io/opentelemetry/sdk/autoconfigure/LoggerProviderConfigurationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ void configureBatchLogRecordProcessor() {
Map<String, String> 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 =
Expand All @@ -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(
Expand Down
4 changes: 2 additions & 2 deletions sdk/logs/src/main/java/io/opentelemetry/sdk/logs/LogLimitsBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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;
}
Expand Down
2 changes: 2 additions & 0 deletions sdk/logs/src/main/java/io/opentelemetry/sdk/logs/export/BatchLogRecordProcessorBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
11 changes: 9 additions & 2 deletions sdk/logs/src/test/java/io/opentelemetry/sdk/logs/LogLimitsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
package io.opentelemetry.sdk.logs;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatCode;
import static org.assertj.core.api.Assertions.assertThatThrownBy;

import org.junit.jupiter.api.Test;
Expand Down Expand Up @@ -33,11 +34,17 @@ void updateLogLimits_All() {

@Test
void invalidLogLimits() {
assertThatThrownBy(() -> LogLimits.builder().setMaxNumberOfAttributes(0))
.isInstanceOf(IllegalArgumentException.class);
assertThatThrownBy(() -> LogLimits.builder().setMaxNumberOfAttributes(-1))
.isInstanceOf(IllegalArgumentException.class);
assertThatThrownBy(() -> LogLimits.builder().setMaxAttributeValueLength(-1))
.isInstanceOf(IllegalArgumentException.class);
}

@Test
void validLogLimits() {
assertThatCode(() -> LogLimits.builder().setMaxNumberOfAttributes(0))
.doesNotThrowAnyException();
assertThatCode(() -> LogLimits.builder().setMaxAttributeValueLength(0))
.doesNotThrowAnyException();
}
}
5 changes: 5 additions & 0 deletions sdk/logs/src/test/java/io/opentelemetry/sdk/logs/export/BatchLogRecordProcessorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,10 @@ 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.");
}

@Test
Expand Down Expand Up @@ -337,6 +341,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();
Expand Down
12 changes: 6 additions & 6 deletions sdk/trace/src/main/java/io/opentelemetry/sdk/trace/SpanLimitsBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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;
}
Expand All @@ -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;
}
Expand All @@ -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;
}
Expand All @@ -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;
}
Expand All @@ -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;
}
Expand Down
2 changes: 2 additions & 0 deletions sdk/trace/src/main/java/io/opentelemetry/sdk/trace/export/BatchSpanProcessorBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
25 changes: 15 additions & 10 deletions sdk/trace/src/test/java/io/opentelemetry/sdk/trace/config/SpanLimitsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
package io.opentelemetry.sdk.trace.config;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatCode;
import static org.assertj.core.api.Assertions.assertThatThrownBy;

import io.opentelemetry.sdk.trace.SpanLimits;
Expand Down Expand Up @@ -46,27 +47,31 @@ void updateSpanLimits_All() {

@Test
void invalidSpanLimits() {
assertThatThrownBy(() -> SpanLimits.builder().setMaxNumberOfAttributes(0))
.isInstanceOf(IllegalArgumentException.class);
assertThatThrownBy(() -> SpanLimits.builder().setMaxNumberOfAttributes(-1))

@jack-berg jack-berg Jan 28, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks like each of these test cases now duplicates a test case directly below it. Same comment as this.

.isInstanceOf(IllegalArgumentException.class);
assertThatThrownBy(() -> SpanLimits.builder().setMaxNumberOfEvents(0))
.isInstanceOf(IllegalArgumentException.class);
assertThatThrownBy(() -> SpanLimits.builder().setMaxNumberOfEvents(-1))
.isInstanceOf(IllegalArgumentException.class);
assertThatThrownBy(() -> SpanLimits.builder().setMaxNumberOfLinks(0))
.isInstanceOf(IllegalArgumentException.class);
assertThatThrownBy(() -> SpanLimits.builder().setMaxNumberOfLinks(-1))
.isInstanceOf(IllegalArgumentException.class);
assertThatThrownBy(() -> SpanLimits.builder().setMaxNumberOfAttributesPerEvent(0))
.isInstanceOf(IllegalArgumentException.class);
assertThatThrownBy(() -> SpanLimits.builder().setMaxNumberOfAttributesPerEvent(-1))
.isInstanceOf(IllegalArgumentException.class);
assertThatThrownBy(() -> SpanLimits.builder().setMaxNumberOfAttributesPerLink(0))
.isInstanceOf(IllegalArgumentException.class);
assertThatThrownBy(() -> SpanLimits.builder().setMaxNumberOfAttributesPerLink(-1))
.isInstanceOf(IllegalArgumentException.class);
assertThatThrownBy(() -> SpanLimits.builder().setMaxAttributeValueLength(-1))
.isInstanceOf(IllegalArgumentException.class);
}

@Test
void validSpanLimits() {
assertThatCode(() -> SpanLimits.builder().setMaxNumberOfAttributes(0))
.doesNotThrowAnyException();
assertThatCode(() -> SpanLimits.builder().setMaxNumberOfEvents(0)).doesNotThrowAnyException();
assertThatCode(() -> SpanLimits.builder().setMaxNumberOfLinks(0)).doesNotThrowAnyException();
assertThatCode(() -> SpanLimits.builder().setMaxNumberOfAttributesPerEvent(0))
.doesNotThrowAnyException();
assertThatCode(() -> SpanLimits.builder().setMaxNumberOfAttributesPerLink(0))
.doesNotThrowAnyException();
assertThatCode(() -> SpanLimits.builder().setMaxAttributeValueLength(0))
.doesNotThrowAnyException();
}
}
4 changes: 4 additions & 0 deletions sdk/trace/src/test/java/io/opentelemetry/sdk/trace/export/BatchSpanProcessorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,9 @@ 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.");
}

@Test
Expand Down Expand Up @@ -419,6 +422,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();

Expand Down