Skip to content

Commit

Permalink
Follow spec on span limits, batch processors
Browse files Browse the repository at this point in the history
  • Loading branch information
onurkybsi committed Jan 17, 2025
1 parent 492b94f commit 03d5657
Show file tree
Hide file tree
Showing 10 changed files with 41 additions and 18 deletions.
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
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
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
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 Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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();
Expand Down
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
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 Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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();

Expand Down

0 comments on commit 03d5657

Please sign in to comment.