Fix HBase test flakiness (#1615)

open-telemetry · Dec 18, 2024 · 426adfc · 426adfc
1 parent 49d738d
commit 426adfc
Show file tree

Hide file tree

Showing 4 changed files with 14 additions and 54 deletions.
diff --git a/...src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/assertions/MetricAssert.java b/...src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/assertions/MetricAssert.java
@@ -249,8 +249,8 @@ public final MetricAssert hasDataPointsWithAttributes(AttributeMatcherGroup... m
             }
 
             info.description(
-                "data point attributes '%s' for metric '%s' must match exactly one of the attribute sets '%s'",
-                dataPointAttributes, actual.getName(), Arrays.asList(matcherGroups));
+                "data point attributes '%s' for metric '%s' must match exactly one of the attribute sets '%s'.\nActual data points: %s",
+                dataPointAttributes, actual.getName(), Arrays.asList(matcherGroups), dataPoints);
             integers.assertEqual(info, matchCount, 1);
           }
 

diff --git a/...ionTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/HBaseIntegrationTest.java b/...ionTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/HBaseIntegrationTest.java
@@ -16,15 +16,16 @@
 import org.testcontainers.containers.wait.strategy.Wait;
 
 public class HBaseIntegrationTest extends TargetSystemIntegrationTest {
-  private static final int DEFAULT_MASTER_SERVICE_PORT = 16000;
-
   @Override
   protected GenericContainer<?> createTargetContainer(int jmxPort) {
     return new GenericContainer<>("dajobe/hbase")
         .withEnv("HBASE_MASTER_OPTS", genericJmxJvmArguments(jmxPort))
         .withStartupTimeout(Duration.ofMinutes(2))
-        .withExposedPorts(jmxPort, DEFAULT_MASTER_SERVICE_PORT)
-        .waitingFor(Wait.forListeningPorts(jmxPort, DEFAULT_MASTER_SERVICE_PORT));
+        .withExposedPorts(jmxPort)
+        // HBase initialization process is finished a long time after all the ports are opened.
+        // Because of this it is necessary to wait for log message confirming that startup process
+        // is complete.
+        .waitingFor(Wait.forLogMessage(".+Master has completed initialization.+", 1));
   }
 
   @Override

diff --git a/...egrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/MetricsVerifier.java b/...egrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/MetricsVerifier.java
@@ -114,7 +114,11 @@ private void verifyAllExpectedMetricsWereReceived(List<Metric> metrics) {
 
     assertionNames.removeAll(receivedMetricNames);
     if (!assertionNames.isEmpty()) {
-      fail("Metrics expected but not received: " + assertionNames);
+      fail(
+          "Metrics expected but not received: "
+              + assertionNames
+              + "\nReceived only: "
+              + receivedMetricNames);
     }
   }
 }
diff --git a/.../java/io/opentelemetry/contrib/jmxscraper/target_systems/TargetSystemIntegrationTest.java b/.../java/io/opentelemetry/contrib/jmxscraper/target_systems/TargetSystemIntegrationTest.java
@@ -21,13 +21,11 @@
 import java.nio.file.Path;
 import java.time.Duration;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Collection;
 import java.util.List;
 import java.util.concurrent.BlockingQueue;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.LinkedBlockingDeque;
-import java.util.function.Consumer;
 import java.util.stream.Collectors;
 import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.AfterEach;
@@ -125,45 +123,6 @@ void endToEndTest(@TempDir Path tmpDir) {
     verifyMetrics();
   }
 
-  // TODO: This implementation is DEPRECATED and will be removed once all integration tests are
-  // migrated to MetricsVerifier
-  protected void waitAndAssertMetrics(Iterable<Consumer<Metric>> assertions) {
-    await()
-        .atMost(Duration.ofSeconds(30))
-        .untilAsserted(
-            () -> {
-              List<ExportMetricsServiceRequest> receivedMetrics = otlpServer.getMetrics();
-              assertThat(receivedMetrics).describedAs("no metric received").isNotEmpty();
-
-              List<Metric> metrics =
-                  receivedMetrics.stream()
-                      .map(ExportMetricsServiceRequest::getResourceMetricsList)
-                      .flatMap(rm -> rm.stream().map(ResourceMetrics::getScopeMetricsList))
-                      .flatMap(Collection::stream)
-                      .filter(
-                          // TODO: disabling batch span exporter might help remove unwanted metrics
-                          sm -> sm.getScope().getName().equals("io.opentelemetry.jmx"))
-                      .flatMap(sm -> sm.getMetricsList().stream())
-                      .collect(Collectors.toList());
-
-              assertThat(metrics)
-                  .describedAs("metrics reported but none from JMX scraper")
-                  .isNotEmpty();
-
-              for (Consumer<Metric> assertion : assertions) {
-                assertThat(metrics).anySatisfy(assertion);
-              }
-            });
-  }
-
-  // TODO: This implementation is DEPRECATED and will be removed once all integration tests are
-  // migrated to MetricsVerifier
-  @SafeVarargs
-  @SuppressWarnings("varargs")
-  protected final void waitAndAssertMetrics(Consumer<Metric>... assertions) {
-    waitAndAssertMetrics(Arrays.asList(assertions));
-  }
-
   protected void verifyMetrics() {
     MetricsVerifier metricsVerifier = createMetricsVerifier();
     await()
@@ -185,18 +144,14 @@ protected void verifyMetrics() {
                       .collect(Collectors.toList());
 
               assertThat(metrics)
-                  .describedAs("metrics reported but none from JMX scraper")
+                  .describedAs("Metrics received but not suitable for JMX scraper")
                   .isNotEmpty();
 
               metricsVerifier.verify(metrics);
             });
   }
 
-  // TODO: This method is going to be abstract once all integration tests are migrated to
-  // MetricsVerifier
-  protected MetricsVerifier createMetricsVerifier() {
-    return MetricsVerifier.create();
-  }
+  protected abstract MetricsVerifier createMetricsVerifier();
 
   protected JmxScraperContainer customizeScraperContainer(
       JmxScraperContainer scraper, GenericContainer<?> target, Path tempDir) {