From 16b51aee400f944d630b9d4091cc53c3c814c0e7 Mon Sep 17 00:00:00 2001
From: Dmitri Bourlatchkov <dmitri.bourlatchkov@dremio.com>
Date: Fri, 30 May 2025 16:09:51 -0400
Subject: [PATCH] fix: Improve reliability of metrics tests

CI sometimes fails with errors like "http_server_requests_seconds not found"
in the reported metrics.

This looks like a race between the Quarkus metrics producer and the
tests asking for these metrics.

This change adds a time-limited retry loop until the expected metrics
are available, before proceeding with other assertions.

Note: in normal cases the loop finishes fast because the metrics are
available. The two-minute timeout would apply only when the expected
metrics fail to be produced at all.
---
 .../quarkus/metrics/MetricsTestBase.java      | 24 +++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/metrics/MetricsTestBase.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/metrics/MetricsTestBase.java
index 590712403c..4932a831cf 100644
--- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/metrics/MetricsTestBase.java
+++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/metrics/MetricsTestBase.java
@@ -25,7 +25,9 @@
 import io.micrometer.core.instrument.MeterRegistry;
 import jakarta.inject.Inject;
 import jakarta.ws.rs.core.Response;
+import java.time.Duration;
 import java.util.Map;
+import java.util.concurrent.atomic.AtomicReference;
 import org.apache.polaris.service.quarkus.test.PolarisIntegrationTestFixture;
 import org.apache.polaris.service.quarkus.test.PolarisIntegrationTestHelper;
 import org.apache.polaris.service.quarkus.test.TestEnvironment;
@@ -41,6 +43,7 @@
 import org.junit.jupiter.api.extension.ExtendWith;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.ValueSource;
+import org.testcontainers.shaded.org.awaitility.Awaitility;
 
 @TestInstance(TestInstance.Lifecycle.PER_CLASS)
 @ExtendWith(TestEnvironmentExtension.class)
@@ -69,12 +72,26 @@ public void clearMetrics() {
     registry.clear();
   }
 
+  private Map<String, MetricFamily> fetchMetrics(String endpoint) {
+    AtomicReference<Map<String, MetricFamily>> value = new AtomicReference<>();
+    Awaitility.await()
+        .atMost(Duration.ofMinutes(2))
+        .untilAsserted(
+            () -> {
+              value.set(
+                  TestMetricsUtil.fetchMetrics(
+                      fixture.client, testEnv.baseManagementUri(), endpoint));
+              assertThat(value.get()).containsKey(API_METRIC_NAME);
+              assertThat(value.get()).containsKey(HTTP_METRIC_NAME);
+            });
+    return value.get();
+  }
+
   @ParameterizedTest
   @ValueSource(strings = {"%s/metrics", "%s/q/metrics"})
   public void testMetricsEmittedOnSuccessfulRequest(String endpoint) {
     sendSuccessfulRequest();
-    Map<String, MetricFamily> allMetrics =
-        TestMetricsUtil.fetchMetrics(fixture.client, testEnv.baseManagementUri(), endpoint);
+    Map<String, MetricFamily> allMetrics = fetchMetrics(endpoint);
     assertThat(allMetrics).containsKey(API_METRIC_NAME);
     assertThat(allMetrics.get(API_METRIC_NAME).getMetrics())
         .satisfiesOnlyOnce(
@@ -125,8 +142,7 @@ public void testMetricsEmittedOnSuccessfulRequest(String endpoint) {
   @ValueSource(strings = {"%s/metrics", "%s/q/metrics"})
   public void testMetricsEmittedOnFailedRequest(String endpoint) {
     sendFailingRequest();
-    Map<String, MetricFamily> allMetrics =
-        TestMetricsUtil.fetchMetrics(fixture.client, testEnv.baseManagementUri(), endpoint);
+    Map<String, MetricFamily> allMetrics = fetchMetrics(endpoint);
     assertThat(allMetrics).containsKey(API_METRIC_NAME);
     assertThat(allMetrics.get(API_METRIC_NAME).getMetrics())
         .satisfiesOnlyOnce(