Skip to content
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.

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

Follow spec on span limits, batch processors #7029

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
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
Loading