From 426adfcc6605cacce3a2c4ca7f23dfb5b1cdb419 Mon Sep 17 00:00:00 2001 From: Robert Niedziela <175605712+robsunday@users.noreply.github.com> Date: Wed, 18 Dec 2024 17:41:06 +0100 Subject: [PATCH] Fix HBase test flakiness (#1615) --- .../jmxscraper/assertions/MetricAssert.java | 4 +- .../target_systems/HBaseIntegrationTest.java | 9 ++-- .../target_systems/MetricsVerifier.java | 6 ++- .../TargetSystemIntegrationTest.java | 49 +------------------ 4 files changed, 14 insertions(+), 54 deletions(-) diff --git a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/assertions/MetricAssert.java b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/assertions/MetricAssert.java index c086c74a7..31e0bf22b 100644 --- a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/assertions/MetricAssert.java +++ b/jmx-scraper/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/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/HBaseIntegrationTest.java b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/HBaseIntegrationTest.java index d8cf66eaa..803137539 100644 --- a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/HBaseIntegrationTest.java +++ b/jmx-scraper/src/integrationTest/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/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/MetricsVerifier.java b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/MetricsVerifier.java index 7a0b13792..4269ed284 100644 --- a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/MetricsVerifier.java +++ b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/MetricsVerifier.java @@ -114,7 +114,11 @@ private void verifyAllExpectedMetricsWereReceived(List 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/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/TargetSystemIntegrationTest.java b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/TargetSystemIntegrationTest.java index 3623a0146..ddfc0567f 100644 --- a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/TargetSystemIntegrationTest.java +++ b/jmx-scraper/src/integrationTest/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> assertions) { - await() - .atMost(Duration.ofSeconds(30)) - .untilAsserted( - () -> { - List receivedMetrics = otlpServer.getMetrics(); - assertThat(receivedMetrics).describedAs("no metric received").isNotEmpty(); - - List 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 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... 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) {