Skip to content

Commit 20f42dd

Browse files
committed
address comments and fix tests.
1 parent 6cea070 commit 20f42dd

File tree

6 files changed

+22
-69
lines changed

6 files changed

+22
-69
lines changed

api/all/src/main/java/io/opentelemetry/api/internal/Utils.java

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,6 @@
55

66
package io.opentelemetry.api.internal;
77

8-
import java.util.logging.Level;
9-
import java.util.logging.Logger;
108
import javax.annotation.concurrent.Immutable;
119

1210
/**
@@ -32,17 +30,4 @@ public static void checkArgument(boolean isValid, String errorMessage) {
3230
throw new IllegalArgumentException(errorMessage);
3331
}
3432
}
35-
36-
/**
37-
* Logs a warning message if the argument is false.
38-
*
39-
* @param logger the logger instance that writes message.
40-
* @param isValid whether the argument check passed.
41-
* @param warnMessage the message to use for the warning log.
42-
*/
43-
public static void warnOnArgument(Logger logger, boolean isValid, String warnMessage) {
44-
if (!isValid) {
45-
logger.log(Level.WARNING, warnMessage);
46-
}
47-
}
4833
}

api/all/src/test/java/io/opentelemetry/api/internal/UtilsTest.java

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,7 @@
66
package io.opentelemetry.api.internal;
77

88
import static org.assertj.core.api.Assertions.assertThatThrownBy;
9-
import static org.mockito.Mockito.mock;
10-
import static org.mockito.Mockito.times;
11-
import static org.mockito.Mockito.verify;
129

13-
import java.util.logging.Level;
14-
import java.util.logging.Logger;
1510
import org.junit.jupiter.api.Test;
1611

1712
class UtilsTest {
@@ -24,11 +19,4 @@ void checkArgument() {
2419
.isInstanceOf(IllegalArgumentException.class)
2520
.hasMessage(TEST_MESSAGE);
2621
}
27-
28-
@Test
29-
void warnOnArgument() {
30-
Logger logger = mock(Logger.class);
31-
Utils.warnOnArgument(logger, false, TEST_MESSAGE);
32-
verify(logger, times(1)).log(Level.WARNING, TEST_MESSAGE);
33-
}
3422
}

sdk/logs/src/main/java/io/opentelemetry/sdk/logs/export/BatchLogRecordProcessorBuilder.java

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
package io.opentelemetry.sdk.logs.export;
77

88
import static io.opentelemetry.api.internal.Utils.checkArgument;
9-
import static io.opentelemetry.api.internal.Utils.warnOnArgument;
109
import static java.util.Objects.requireNonNull;
1110

1211
import io.opentelemetry.api.metrics.MeterProvider;
@@ -108,10 +107,9 @@ long getExporterTimeoutNanos() {
108107
*/
109108
public BatchLogRecordProcessorBuilder setMaxQueueSize(int maxQueueSize) {
110109
checkArgument(maxQueueSize > 0, "maxQueueSize must be positive.");
111-
warnOnArgument(
112-
logger,
113-
maxExportBatchSize <= maxQueueSize,
114-
"maxExportBatchSize should not exceed maxQueueSize.");
110+
if (maxExportBatchSize > maxQueueSize) {
111+
logger.log(Level.WARNING, "maxExportBatchSize should not exceed maxQueueSize.");
112+
}
115113
this.maxQueueSize = maxQueueSize;
116114
return this;
117115
}
@@ -133,10 +131,9 @@ int getMaxQueueSize() {
133131
*/
134132
public BatchLogRecordProcessorBuilder setMaxExportBatchSize(int maxExportBatchSize) {
135133
checkArgument(maxExportBatchSize > 0, "maxExportBatchSize must be positive.");
136-
warnOnArgument(
137-
logger,
138-
maxExportBatchSize <= maxQueueSize,
139-
"maxExportBatchSize should not exceed maxQueueSize.");
134+
if (maxExportBatchSize > maxQueueSize) {
135+
logger.log(Level.WARNING, "maxExportBatchSize should not exceed maxQueueSize.");
136+
}
140137
this.maxExportBatchSize = maxExportBatchSize;
141138
return this;
142139
}
@@ -164,7 +161,7 @@ int getMaxExportBatchSize() {
164161
*/
165162
public BatchLogRecordProcessor build() {
166163
if (maxExportBatchSize > maxQueueSize) {
167-
maxExportBatchSize = Math.min(DEFAULT_MAX_EXPORT_BATCH_SIZE, maxQueueSize);
164+
maxExportBatchSize = maxQueueSize;
168165
logger.log(Level.FINE, "Using maxExportBatchSize: {0}", maxExportBatchSize);
169166
}
170167
return new BatchLogRecordProcessor(

sdk/logs/src/test/java/io/opentelemetry/sdk/logs/export/BatchLogRecordProcessorTest.java

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -125,22 +125,14 @@ void builderInvalidConfig() {
125125
void builderAdjustMaxBatchSize() {
126126
LogRecordExporter dummyExporter = new CompletableLogRecordExporter();
127127

128-
BatchLogRecordProcessorBuilder smallQueueBuilder =
129-
BatchLogRecordProcessor.builder(dummyExporter)
130-
.setMaxQueueSize(1)
131-
.setMaxExportBatchSize(1000);
132-
smallQueueBuilder.build();
133-
134-
assertThat(smallQueueBuilder.getMaxExportBatchSize()).isEqualTo(1);
135-
136-
BatchLogRecordProcessorBuilder largeBatchBuilder =
128+
BatchLogRecordProcessorBuilder builder =
137129
BatchLogRecordProcessor.builder(dummyExporter)
138130
.setMaxQueueSize(513)
139131
.setMaxExportBatchSize(1000);
140-
largeBatchBuilder.build();
132+
builder.build();
141133

142-
assertThat(largeBatchBuilder.getMaxExportBatchSize())
143-
.isEqualTo(BatchLogRecordProcessorBuilder.DEFAULT_MAX_EXPORT_BATCH_SIZE);
134+
assertThat(builder.getMaxExportBatchSize()).isEqualTo(513);
135+
assertThat(builder.getMaxQueueSize()).isEqualTo(513);
144136
}
145137

146138
@Test

sdk/trace/src/main/java/io/opentelemetry/sdk/trace/export/BatchSpanProcessorBuilder.java

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
package io.opentelemetry.sdk.trace.export;
77

88
import static io.opentelemetry.api.internal.Utils.checkArgument;
9-
import static io.opentelemetry.api.internal.Utils.warnOnArgument;
109
import static java.util.Objects.requireNonNull;
1110

1211
import io.opentelemetry.api.metrics.MeterProvider;
@@ -115,10 +114,9 @@ long getExporterTimeoutNanos() {
115114
*/
116115
public BatchSpanProcessorBuilder setMaxQueueSize(int maxQueueSize) {
117116
checkArgument(maxQueueSize > 0, "maxQueueSize must be positive.");
118-
warnOnArgument(
119-
logger,
120-
maxExportBatchSize <= maxQueueSize,
121-
"maxExportBatchSize should not exceed maxQueueSize.");
117+
if (maxExportBatchSize > maxQueueSize) {
118+
logger.log(Level.WARNING, "maxExportBatchSize should not exceed maxQueueSize.");
119+
}
122120
this.maxQueueSize = maxQueueSize;
123121
return this;
124122
}
@@ -140,10 +138,9 @@ int getMaxQueueSize() {
140138
*/
141139
public BatchSpanProcessorBuilder setMaxExportBatchSize(int maxExportBatchSize) {
142140
checkArgument(maxExportBatchSize > 0, "maxExportBatchSize must be positive.");
143-
warnOnArgument(
144-
logger,
145-
maxExportBatchSize <= maxQueueSize,
146-
"maxExportBatchSize should not exceed maxQueueSize.");
141+
if (maxExportBatchSize > maxQueueSize) {
142+
logger.log(Level.WARNING, "maxExportBatchSize should not exceed maxQueueSize.");
143+
}
147144
this.maxExportBatchSize = maxExportBatchSize;
148145
return this;
149146
}
@@ -171,7 +168,7 @@ int getMaxExportBatchSize() {
171168
*/
172169
public BatchSpanProcessor build() {
173170
if (maxExportBatchSize > maxQueueSize) {
174-
maxExportBatchSize = Math.min(DEFAULT_MAX_EXPORT_BATCH_SIZE, maxQueueSize);
171+
maxExportBatchSize = maxQueueSize;
175172
logger.log(Level.FINE, "Using maxExportBatchSize: {0}", maxExportBatchSize);
176173
}
177174
return new BatchSpanProcessor(

sdk/trace/src/test/java/io/opentelemetry/sdk/trace/export/BatchSpanProcessorTest.java

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -133,18 +133,12 @@ void builderInvalidConfig() {
133133
void builderAdjustMaxBatchSize() {
134134
SpanExporter dummyExporter = new CompletableSpanExporter();
135135

136-
BatchSpanProcessorBuilder smallQueueBuilder =
137-
BatchSpanProcessor.builder(dummyExporter).setMaxQueueSize(1).setMaxExportBatchSize(1000);
138-
smallQueueBuilder.build();
139-
140-
assertThat(smallQueueBuilder.getMaxExportBatchSize()).isEqualTo(1);
141-
142-
BatchSpanProcessorBuilder largeBatchBuilder =
136+
BatchSpanProcessorBuilder builder =
143137
BatchSpanProcessor.builder(dummyExporter).setMaxQueueSize(513).setMaxExportBatchSize(1000);
144-
largeBatchBuilder.build();
138+
builder.build();
145139

146-
assertThat(largeBatchBuilder.getMaxExportBatchSize())
147-
.isEqualTo(BatchSpanProcessorBuilder.DEFAULT_MAX_EXPORT_BATCH_SIZE);
140+
assertThat(builder.getMaxExportBatchSize()).isEqualTo(513);
141+
assertThat(builder.getMaxQueueSize()).isEqualTo(513);
148142
}
149143

150144
@Test

0 commit comments

Comments
 (0)