Skip to content

Commit e62c969

Browse files
igorbernstein2gcf-owl-bot[bot]
igorbernstein2
and
gcf-owl-bot[bot]
authored
test: deflake metrics unit tests (#2253)
* test: deflake metrics unit tests Change-Id: I65774cd89908b986600bba8feff609090aa74dc3 * fixed off by one error & re-added checks for name collisions Change-Id: Ib971f7ac7c117fde274b66ea8b2845cfbdd62b0f * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * remove unnecessary variables Change-Id: If60e63dfd13edc85750de36f9f547d7ffc5abce8 * better diagnostic message Change-Id: I796e1234e7cebd3408e227b9e876a19ddb3a6b6a --------- Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
1 parent da703db commit e62c969

File tree

4 files changed

+60
-58
lines changed

4 files changed

+60
-58
lines changed

‎google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/it/BuiltinMetricsIT.java‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,6 @@
6464
import io.opentelemetry.sdk.testing.exporter.InMemoryMetricReader;
6565
import java.io.IOException;
6666
import java.util.ArrayList;
67-
import java.util.Collection;
6867
import java.util.List;
6968
import java.util.Map;
7069
import java.util.concurrent.TimeUnit;
@@ -268,8 +267,6 @@ public void testBuiltinMetricsWithCustomOTEL() throws Exception {
268267

269268
ProjectName name = ProjectName.of(testEnvRule.env().getProjectId());
270269

271-
Collection<MetricData> fromMetricReader = metricReader.collectAllMetrics();
272-
273270
// Interval is set in the monarch request when query metric timestamps.
274271
// Restrict it to before we send to request and 3 minute after we send the request. If
275272
// it turns out to be still flaky we can increase the filter range.
@@ -285,7 +282,7 @@ public void testBuiltinMetricsWithCustomOTEL() throws Exception {
285282
if (view.equals("application_blocking_latencies")) {
286283
otelMetricName = "application_latencies";
287284
}
288-
MetricData dataFromReader = getMetricData(fromMetricReader, otelMetricName);
285+
MetricData dataFromReader = getMetricData(metricReader, otelMetricName);
289286

290287
// Filter on instance and method name
291288
// Verify that metrics are correct for MutateRows request

‎google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTestUtils.java‎

Lines changed: 37 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16,35 +16,63 @@
1616
package com.google.cloud.bigtable.data.v2.stub.metrics;
1717

1818
import static com.google.common.truth.Truth.assertThat;
19+
import static com.google.common.truth.Truth.assertWithMessage;
1920

2021
import com.google.api.core.InternalApi;
22+
import com.google.common.truth.Correspondence;
2123
import com.google.protobuf.Timestamp;
2224
import com.google.protobuf.util.Timestamps;
2325
import io.opentelemetry.api.common.Attributes;
2426
import io.opentelemetry.sdk.metrics.data.HistogramPointData;
2527
import io.opentelemetry.sdk.metrics.data.LongPointData;
2628
import io.opentelemetry.sdk.metrics.data.MetricData;
29+
import io.opentelemetry.sdk.testing.exporter.InMemoryMetricReader;
2730
import java.util.Collection;
31+
import java.util.Collections;
2832
import java.util.List;
2933
import java.util.stream.Collectors;
3034
import org.junit.Assert;
3135

3236
@InternalApi
3337
public class BuiltinMetricsTestUtils {
38+
private static final Correspondence<MetricData, String> METRIC_DATA_BY_NAME =
39+
Correspondence.transforming(MetricData::getName, "MetricData name");
3440

3541
private BuiltinMetricsTestUtils() {}
3642

37-
public static MetricData getMetricData(Collection<MetricData> allMetricData, String metricName) {
38-
List<MetricData> metricDataList =
39-
allMetricData.stream()
40-
.filter(md -> md.getName().equals(BuiltinMetricsConstants.METER_NAME + metricName))
41-
.collect(Collectors.toList());
42-
if (metricDataList.size() == 0) {
43-
allMetricData.stream().forEach(md -> System.out.println(md.getName()));
43+
public static MetricData getMetricData(InMemoryMetricReader reader, String metricName) {
44+
String fullMetricName = BuiltinMetricsConstants.METER_NAME + metricName;
45+
Collection<MetricData> allMetricData = Collections.emptyList();
46+
47+
// Fetch the MetricData with retries
48+
for (int attemptsLeft = 10; attemptsLeft > 0; attemptsLeft--) {
49+
allMetricData = reader.collectAllMetrics();
50+
List<MetricData> matchingMetadata =
51+
allMetricData.stream()
52+
.filter(md -> METRIC_DATA_BY_NAME.compare(md, fullMetricName))
53+
.collect(Collectors.toList());
54+
assertWithMessage(
55+
"Found multiple MetricData with the same name: %s, in: %s",
56+
fullMetricName, matchingMetadata)
57+
.that(matchingMetadata.size())
58+
.isAtMost(1);
59+
60+
if (!matchingMetadata.isEmpty()) {
61+
return matchingMetadata.get(0);
62+
}
63+
64+
try {
65+
Thread.sleep(100);
66+
} catch (InterruptedException interruptedException) {
67+
Thread.currentThread().interrupt();
68+
throw new RuntimeException(interruptedException);
69+
}
4470
}
45-
assertThat(metricDataList.size()).isEqualTo(1);
4671

47-
return metricDataList.get(0);
72+
// MetricData was not found, assert on original collection to get a descriptive error message
73+
assertThat(allMetricData).comparingElementsUsing(METRIC_DATA_BY_NAME).contains(fullMetricName);
74+
throw new IllegalStateException(
75+
"MetricData was missing then appeared, this should never happen");
4876
}
4977

5078
public static long getAggregatedValue(MetricData metricData, Attributes attributes) {

‎google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerTest.java‎

Lines changed: 18 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,6 @@
9797
import io.opentelemetry.sdk.testing.exporter.InMemoryMetricReader;
9898
import java.nio.charset.Charset;
9999
import java.util.ArrayList;
100-
import java.util.Collection;
101100
import java.util.Collections;
102101
import java.util.Iterator;
103102
import java.util.List;
@@ -298,9 +297,7 @@ public void testReadRowsOperationLatencies() {
298297
.put(CLIENT_NAME_KEY, CLIENT_NAME)
299298
.build();
300299

301-
Collection<MetricData> allMetricData = metricReader.collectAllMetrics();
302-
303-
MetricData metricData = getMetricData(allMetricData, OPERATION_LATENCIES_NAME);
300+
MetricData metricData = getMetricData(metricReader, OPERATION_LATENCIES_NAME);
304301

305302
long value = getAggregatedValue(metricData, expectedAttributes);
306303
assertThat(value).isIn(Range.closed(SERVER_LATENCY, elapsed));
@@ -326,9 +323,7 @@ public void testReadRowsOperationLatenciesOnAuthorizedView() {
326323
.put(CLIENT_NAME_KEY, CLIENT_NAME)
327324
.build();
328325

329-
Collection<MetricData> allMetricData = metricReader.collectAllMetrics();
330-
331-
MetricData metricData = getMetricData(allMetricData, OPERATION_LATENCIES_NAME);
326+
MetricData metricData = getMetricData(metricReader, OPERATION_LATENCIES_NAME);
332327
long value = getAggregatedValue(metricData, expectedAttributes);
333328
assertThat(value).isIn(Range.closed(SERVER_LATENCY, elapsed));
334329
}
@@ -348,15 +343,13 @@ public void testGfeMetrics() {
348343
.put(METHOD_KEY, "Bigtable.ReadRows")
349344
.build();
350345

351-
Collection<MetricData> allMetricData = metricReader.collectAllMetrics();
352-
353-
MetricData serverLatenciesMetricData = getMetricData(allMetricData, SERVER_LATENCIES_NAME);
346+
MetricData serverLatenciesMetricData = getMetricData(metricReader, SERVER_LATENCIES_NAME);
354347

355348
long serverLatencies = getAggregatedValue(serverLatenciesMetricData, expectedAttributes);
356349
assertThat(serverLatencies).isEqualTo(FAKE_SERVER_TIMING);
357350

358351
MetricData connectivityErrorCountMetricData =
359-
getMetricData(allMetricData, CONNECTIVITY_ERROR_COUNT_NAME);
352+
getMetricData(metricReader, CONNECTIVITY_ERROR_COUNT_NAME);
360353
Attributes expected1 =
361354
baseAttributes
362355
.toBuilder()
@@ -420,9 +413,8 @@ public void onComplete() {
420413

421414
assertThat(counter.get()).isEqualTo(fakeService.getResponseCounter().get());
422415

423-
Collection<MetricData> allMetricData = metricReader.collectAllMetrics();
424416
MetricData applicationLatency =
425-
getMetricData(allMetricData, APPLICATION_BLOCKING_LATENCIES_NAME);
417+
getMetricData(metricReader, APPLICATION_BLOCKING_LATENCIES_NAME);
426418

427419
Attributes expectedAttributes =
428420
baseAttributes
@@ -437,7 +429,7 @@ public void onComplete() {
437429

438430
assertThat(value).isAtLeast((APPLICATION_LATENCY - SLEEP_VARIABILITY) * counter.get());
439431

440-
MetricData operationLatency = getMetricData(allMetricData, OPERATION_LATENCIES_NAME);
432+
MetricData operationLatency = getMetricData(metricReader, OPERATION_LATENCIES_NAME);
441433
long operationLatencyValue =
442434
getAggregatedValue(
443435
operationLatency,
@@ -457,9 +449,8 @@ public void testReadRowsApplicationLatencyWithManualFlowControl() throws Excepti
457449
rows.next();
458450
}
459451

460-
Collection<MetricData> allMetricData = metricReader.collectAllMetrics();
461452
MetricData applicationLatency =
462-
getMetricData(allMetricData, APPLICATION_BLOCKING_LATENCIES_NAME);
453+
getMetricData(metricReader, APPLICATION_BLOCKING_LATENCIES_NAME);
463454

464455
Attributes expectedAttributes =
465456
baseAttributes
@@ -477,7 +468,7 @@ public void testReadRowsApplicationLatencyWithManualFlowControl() throws Excepti
477468
assertThat(counter).isEqualTo(fakeService.getResponseCounter().get());
478469
assertThat(value).isAtLeast(APPLICATION_LATENCY * (counter - 1) - SERVER_LATENCY);
479470

480-
MetricData operationLatency = getMetricData(allMetricData, OPERATION_LATENCIES_NAME);
471+
MetricData operationLatency = getMetricData(metricReader, OPERATION_LATENCIES_NAME);
481472
long operationLatencyValue =
482473
getAggregatedValue(
483474
operationLatency,
@@ -490,8 +481,7 @@ public void testRetryCount() throws InterruptedException {
490481
stub.mutateRowCallable()
491482
.call(RowMutation.create(TABLE, "random-row").setCell("cf", "q", "value"));
492483

493-
Collection<MetricData> allMetricData = metricReader.collectAllMetrics();
494-
MetricData metricData = getMetricData(allMetricData, RETRY_COUNT_NAME);
484+
MetricData metricData = getMetricData(metricReader, RETRY_COUNT_NAME);
495485
Attributes expectedAttributes =
496486
baseAttributes
497487
.toBuilder()
@@ -512,8 +502,7 @@ public void testMutateRowAttemptsTagValues() {
512502
stub.mutateRowCallable()
513503
.call(RowMutation.create(TABLE, "random-row").setCell("cf", "q", "value"));
514504

515-
Collection<MetricData> allMetricData = metricReader.collectAllMetrics();
516-
MetricData metricData = getMetricData(allMetricData, ATTEMPT_LATENCIES_NAME);
505+
MetricData metricData = getMetricData(metricReader, ATTEMPT_LATENCIES_NAME);
517506

518507
Attributes expected1 =
519508
baseAttributes
@@ -554,8 +543,7 @@ public void testMutateRowsPartialError() throws InterruptedException {
554543

555544
Assert.assertThrows(BatchingException.class, batcher::close);
556545

557-
Collection<MetricData> allMetricData = metricReader.collectAllMetrics();
558-
MetricData metricData = getMetricData(allMetricData, ATTEMPT_LATENCIES_NAME);
546+
MetricData metricData = getMetricData(metricReader, ATTEMPT_LATENCIES_NAME);
559547

560548
Attributes expected =
561549
baseAttributes
@@ -584,8 +572,7 @@ public void testMutateRowsRpcError() {
584572

585573
Assert.assertThrows(BatchingException.class, batcher::close);
586574

587-
Collection<MetricData> allMetricData = metricReader.collectAllMetrics();
588-
MetricData metricData = getMetricData(allMetricData, ATTEMPT_LATENCIES_NAME);
575+
MetricData metricData = getMetricData(metricReader, ATTEMPT_LATENCIES_NAME);
589576

590577
Attributes expected =
591578
baseAttributes
@@ -606,8 +593,7 @@ public void testMutateRowsRpcError() {
606593
public void testReadRowsAttemptsTagValues() {
607594
Lists.newArrayList(stub.readRowsCallable().call(Query.create("fake-table")).iterator());
608595

609-
Collection<MetricData> allMetricData = metricReader.collectAllMetrics();
610-
MetricData metricData = getMetricData(allMetricData, ATTEMPT_LATENCIES_NAME);
596+
MetricData metricData = getMetricData(metricReader, ATTEMPT_LATENCIES_NAME);
611597

612598
Attributes expected1 =
613599
baseAttributes
@@ -649,8 +635,7 @@ public void testBatchBlockingLatencies() throws InterruptedException {
649635

650636
int expectedNumRequests = 6 / batchElementCount;
651637

652-
Collection<MetricData> allMetricData = metricReader.collectAllMetrics();
653-
MetricData applicationLatency = getMetricData(allMetricData, CLIENT_BLOCKING_LATENCIES_NAME);
638+
MetricData applicationLatency = getMetricData(metricReader, CLIENT_BLOCKING_LATENCIES_NAME);
654639

655640
Attributes expectedAttributes =
656641
baseAttributes
@@ -675,8 +660,7 @@ public void testBatchBlockingLatencies() throws InterruptedException {
675660
public void testQueuedOnChannelServerStreamLatencies() {
676661
stub.readRowsCallable().all().call(Query.create(TABLE));
677662

678-
Collection<MetricData> allMetricData = metricReader.collectAllMetrics();
679-
MetricData clientLatency = getMetricData(allMetricData, CLIENT_BLOCKING_LATENCIES_NAME);
663+
MetricData clientLatency = getMetricData(metricReader, CLIENT_BLOCKING_LATENCIES_NAME);
680664

681665
Attributes attributes =
682666
baseAttributes
@@ -697,8 +681,7 @@ public void testQueuedOnChannelUnaryLatencies() {
697681

698682
stub.mutateRowCallable().call(RowMutation.create(TABLE, "a-key").setCell("f", "q", "v"));
699683

700-
Collection<MetricData> allMetricData = metricReader.collectAllMetrics();
701-
MetricData clientLatency = getMetricData(allMetricData, CLIENT_BLOCKING_LATENCIES_NAME);
684+
MetricData clientLatency = getMetricData(metricReader, CLIENT_BLOCKING_LATENCIES_NAME);
702685

703686
Attributes attributes =
704687
baseAttributes
@@ -723,8 +706,7 @@ public void testPermanentFailure() {
723706
} catch (NotFoundException e) {
724707
}
725708

726-
Collection<MetricData> allMetricData = metricReader.collectAllMetrics();
727-
MetricData attemptLatency = getMetricData(allMetricData, ATTEMPT_LATENCIES_NAME);
709+
MetricData attemptLatency = getMetricData(metricReader, ATTEMPT_LATENCIES_NAME);
728710

729711
Attributes expected =
730712
baseAttributes
@@ -740,7 +722,7 @@ public void testPermanentFailure() {
740722

741723
verifyAttributes(attemptLatency, expected);
742724

743-
MetricData opLatency = getMetricData(allMetricData, OPERATION_LATENCIES_NAME);
725+
MetricData opLatency = getMetricData(metricReader, OPERATION_LATENCIES_NAME);
744726
verifyAttributes(opLatency, expected);
745727
}
746728

‎google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/ErrorCountPerConnectionTest.java‎

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@
4343
import io.opentelemetry.sdk.metrics.data.MetricData;
4444
import io.opentelemetry.sdk.testing.exporter.InMemoryMetricReader;
4545
import java.util.ArrayList;
46-
import java.util.Collection;
4746
import java.util.List;
4847
import java.util.Map;
4948
import java.util.concurrent.ScheduledExecutorService;
@@ -138,10 +137,9 @@ public void readWithOneChannel() throws Exception {
138137

139138
runInterceptorTasksAndAssertCount();
140139

141-
Collection<MetricData> allMetrics = metricReader.collectAllMetrics();
142140
MetricData metricData =
143141
BuiltinMetricsTestUtils.getMetricData(
144-
allMetrics, BuiltinMetricsConstants.PER_CONNECTION_ERROR_COUNT_NAME);
142+
metricReader, BuiltinMetricsConstants.PER_CONNECTION_ERROR_COUNT_NAME);
145143

146144
// Make sure the correct bucket is updated with the correct number of data points
147145
ArrayList<HistogramPointData> histogramPointData =
@@ -179,10 +177,9 @@ public void readWithTwoChannels() throws Exception {
179177

180178
long errorCountPerChannel = totalErrorCount / 2;
181179

182-
Collection<MetricData> allMetrics = metricReader.collectAllMetrics();
183180
MetricData metricData =
184181
BuiltinMetricsTestUtils.getMetricData(
185-
allMetrics, BuiltinMetricsConstants.PER_CONNECTION_ERROR_COUNT_NAME);
182+
metricReader, BuiltinMetricsConstants.PER_CONNECTION_ERROR_COUNT_NAME);
186183

187184
// The 2 channels should get equal amount of errors, so the totalErrorCount / 2 bucket is
188185
// updated twice.
@@ -234,10 +231,9 @@ public void readOverTwoPeriods() throws Exception {
234231

235232
runInterceptorTasksAndAssertCount();
236233

237-
Collection<MetricData> allMetrics = metricReader.collectAllMetrics();
238234
MetricData metricData =
239235
BuiltinMetricsTestUtils.getMetricData(
240-
allMetrics, BuiltinMetricsConstants.PER_CONNECTION_ERROR_COUNT_NAME);
236+
metricReader, BuiltinMetricsConstants.PER_CONNECTION_ERROR_COUNT_NAME);
241237

242238
ArrayList<HistogramPointData> histogramPointData =
243239
new ArrayList<>(metricData.getHistogramData().getPoints());
@@ -261,10 +257,9 @@ public void noFailedRequests() throws Exception {
261257
}
262258
}
263259
runInterceptorTasksAndAssertCount();
264-
Collection<MetricData> allMetrics = metricReader.collectAllMetrics();
265260
MetricData metricData =
266261
BuiltinMetricsTestUtils.getMetricData(
267-
allMetrics, BuiltinMetricsConstants.PER_CONNECTION_ERROR_COUNT_NAME);
262+
metricReader, BuiltinMetricsConstants.PER_CONNECTION_ERROR_COUNT_NAME);
268263
long value = BuiltinMetricsTestUtils.getAggregatedValue(metricData, attributes);
269264
assertThat(value).isEqualTo(0);
270265
}

0 commit comments

Comments
 (0)