Skip to content

Commit e470a0f

Browse files
committed
Fix bug for miscalculating remaining capacity of a batch
1 parent 95370fa commit e470a0f

2 files changed

Lines changed: 78 additions & 20 deletions

File tree

sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/export/MetricExportBatcher.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,11 @@ Collection<Collection<MetricData>> batchMetrics(Collection<MetricData> metrics)
9393
*/
9494
private MetricDataSplitOperationResult prepareExportBatches(
9595
MetricData metricData, Collection<MetricData> currentBatch) {
96-
int remainingCapacityInCurrentBatch = maxExportBatchSize - currentBatch.size();
96+
int currentBatchPoints = 0;
97+
for (MetricData m : currentBatch) {
98+
currentBatchPoints += m.getData().getPoints().size();
99+
}
100+
int remainingCapacityInCurrentBatch = maxExportBatchSize - currentBatchPoints;
97101
int totalPointsInMetricData = metricData.getData().getPoints().size();
98102

99103
if (remainingCapacityInCurrentBatch >= totalPointsInMetricData) {

sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/export/MetricExportBatcherTest.java

Lines changed: 73 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -114,27 +114,27 @@ void batchMetrics_SplitsDoubleGauge_LastBatchPartiallyFilled() {
114114
assertThat(secondBatch.size()).isEqualTo(1);
115115
assertThat(thirdBatch.size()).isEqualTo(1);
116116

117-
MetricData firsBatch_m1 = firstBatch.iterator().next();
118-
assertThat(firsBatch_m1.getType()).isEqualByComparingTo(MetricDataType.DOUBLE_GAUGE);
119-
assertThat(firsBatch_m1.getName()).isEqualTo("name");
120-
assertThat(firsBatch_m1.getDescription()).isEqualTo("desc");
121-
assertThat(firsBatch_m1.getUnit()).isEqualTo("1");
122-
assertThat(firsBatch_m1.getDoubleGaugeData().getPoints()).containsExactly(p1, p2);
123-
124-
MetricData secondBatch_m1 = secondBatch.iterator().next();
125-
assertThat(secondBatch_m1.getType()).isEqualByComparingTo(MetricDataType.DOUBLE_GAUGE);
126-
assertThat(secondBatch_m1.getName()).isEqualTo("name");
127-
assertThat(secondBatch_m1.getDescription()).isEqualTo("desc");
128-
assertThat(secondBatch_m1.getUnit()).isEqualTo("1");
129-
assertThat(secondBatch_m1.getDoubleGaugeData().getPoints()).containsExactly(p3, p4);
117+
MetricData b1m1 = firstBatch.iterator().next();
118+
assertThat(b1m1.getType()).isEqualByComparingTo(MetricDataType.DOUBLE_GAUGE);
119+
assertThat(b1m1.getName()).isEqualTo("name");
120+
assertThat(b1m1.getDescription()).isEqualTo("desc");
121+
assertThat(b1m1.getUnit()).isEqualTo("1");
122+
assertThat(b1m1.getDoubleGaugeData().getPoints()).containsExactly(p1, p2);
123+
124+
MetricData b2m1 = secondBatch.iterator().next();
125+
assertThat(b2m1.getType()).isEqualByComparingTo(MetricDataType.DOUBLE_GAUGE);
126+
assertThat(b2m1.getName()).isEqualTo("name");
127+
assertThat(b2m1.getDescription()).isEqualTo("desc");
128+
assertThat(b2m1.getUnit()).isEqualTo("1");
129+
assertThat(b2m1.getDoubleGaugeData().getPoints()).containsExactly(p3, p4);
130130

131131
// Last batch is partially filled.
132-
MetricData thirdBatch_m1 = thirdBatch.iterator().next();
133-
assertThat(thirdBatch_m1.getType()).isEqualByComparingTo(MetricDataType.DOUBLE_GAUGE);
134-
assertThat(thirdBatch_m1.getName()).isEqualTo("name");
135-
assertThat(thirdBatch_m1.getDescription()).isEqualTo("desc");
136-
assertThat(thirdBatch_m1.getUnit()).isEqualTo("1");
137-
assertThat(thirdBatch_m1.getDoubleGaugeData().getPoints()).containsExactly(p5);
132+
MetricData b3m1 = thirdBatch.iterator().next();
133+
assertThat(b3m1.getType()).isEqualByComparingTo(MetricDataType.DOUBLE_GAUGE);
134+
assertThat(b3m1.getName()).isEqualTo("name");
135+
assertThat(b3m1.getDescription()).isEqualTo("desc");
136+
assertThat(b3m1.getUnit()).isEqualTo("1");
137+
assertThat(b3m1.getDoubleGaugeData().getPoints()).containsExactly(p5);
138138
}
139139

140140
@Test
@@ -396,6 +396,60 @@ void batchMetrics_MultipleMetricsExactCapacityMatch() {
396396
assertThat(res2.getLongGaugeData().getPoints()).containsExactly(p3, p4);
397397
}
398398

399+
@Test
400+
void batchMetrics_SplitsLongGauge_MultipleMetrics_ExceedsCapacity() {
401+
MetricExportBatcher batcher = new MetricExportBatcher(4);
402+
LongPointData p1 = ImmutableLongPointData.create(1, 2, Attributes.empty(), 1L);
403+
LongPointData p2 = ImmutableLongPointData.create(1, 2, Attributes.empty(), 2L);
404+
LongPointData p3 = ImmutableLongPointData.create(1, 2, Attributes.empty(), 3L);
405+
LongPointData p4 = ImmutableLongPointData.create(1, 2, Attributes.empty(), 4L);
406+
LongPointData p5 = ImmutableLongPointData.create(1, 2, Attributes.empty(), 5L);
407+
LongPointData p6 = ImmutableLongPointData.create(1, 2, Attributes.empty(), 6L);
408+
409+
MetricData m1 =
410+
ImmutableMetricData.createLongGauge(
411+
Resource.empty(),
412+
InstrumentationScopeInfo.empty(),
413+
"name_1",
414+
"desc",
415+
"1",
416+
ImmutableGaugeData.create(Arrays.asList(p1, p2, p3)));
417+
MetricData m2 =
418+
ImmutableMetricData.createLongGauge(
419+
Resource.empty(),
420+
InstrumentationScopeInfo.empty(),
421+
"name_2",
422+
"desc",
423+
"1",
424+
ImmutableGaugeData.create(Arrays.asList(p4, p5, p6)));
425+
426+
Collection<Collection<MetricData>> batches = batcher.batchMetrics(Arrays.asList(m1, m2));
427+
428+
assertThat(batches).hasSize(2);
429+
430+
Collection<MetricData> firstBatch = batches.iterator().next();
431+
assertThat(firstBatch).hasSize(2);
432+
MetricData b1m1 = firstBatch.iterator().next();
433+
MetricData b1m2 = firstBatch.stream().skip(1).findFirst().get();
434+
assertThat(b1m1.getName()).isEqualTo("name_1");
435+
assertThat(b1m1.getDescription()).isEqualTo("desc");
436+
assertThat(b1m1.getUnit()).isEqualTo("1");
437+
assertThat(b1m1.getLongGaugeData().getPoints()).containsExactly(p1, p2, p3);
438+
439+
assertThat(b1m2.getName()).isEqualTo("name_2");
440+
assertThat(b1m2.getDescription()).isEqualTo("desc");
441+
assertThat(b1m2.getUnit()).isEqualTo("1");
442+
assertThat(b1m2.getLongGaugeData().getPoints()).containsExactly(p4);
443+
444+
Collection<MetricData> secondBatch = batches.stream().skip(1).findFirst().get();
445+
assertThat(secondBatch).hasSize(1);
446+
MetricData b2m1 = secondBatch.iterator().next();
447+
assertThat(b2m1.getName()).isEqualTo("name_2");
448+
assertThat(b2m1.getDescription()).isEqualTo("desc");
449+
assertThat(b2m1.getUnit()).isEqualTo("1");
450+
assertThat(b2m1.getLongGaugeData().getPoints()).containsExactly(p5, p6);
451+
}
452+
399453
@Test
400454
void batchMetrics_SplitsExponentialHistogram_MultipleBatchesCompletelyFilled_SingleMetric() {
401455
MetricExportBatcher batcher = new MetricExportBatcher(1);

0 commit comments

Comments
 (0)