From 4bd41c7aade07f6958ca484761897965c53bc585 Mon Sep 17 00:00:00 2001 From: Gregor Zeitlinger Date: Tue, 28 Jan 2025 18:59:30 +0100 Subject: [PATCH 1/2] improve assertions by showing the last result on a timeout --- .../testing/InstrumentationTestRunner.java | 68 ++++++++----------- .../testing/internal/AwaitUtil.java | 44 ++++++++++++ .../junit/InstrumentationExtension.java | 10 ++- 3 files changed, 75 insertions(+), 47 deletions(-) create mode 100644 testing-common/src/main/java/io/opentelemetry/instrumentation/testing/internal/AwaitUtil.java diff --git a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/InstrumentationTestRunner.java b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/InstrumentationTestRunner.java index 15d197f9b57e..b85e7422236c 100644 --- a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/InstrumentationTestRunner.java +++ b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/InstrumentationTestRunner.java @@ -6,9 +6,9 @@ package io.opentelemetry.instrumentation.testing; import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.assertThat; -import static org.awaitility.Awaitility.await; import io.opentelemetry.api.OpenTelemetry; +import io.opentelemetry.instrumentation.testing.internal.AwaitUtil; import io.opentelemetry.instrumentation.testing.util.TelemetryDataUtil; import io.opentelemetry.instrumentation.testing.util.ThrowingRunnable; import io.opentelemetry.instrumentation.testing.util.ThrowingSupplier; @@ -29,7 +29,6 @@ import java.util.stream.Collectors; import javax.annotation.Nullable; import org.assertj.core.api.ListAssert; -import org.awaitility.core.ConditionTimeoutException; /** * This interface defines a common set of operations for interaction with OpenTelemetry SDK and @@ -118,25 +117,8 @@ private > void waitAndAssertTraces( List assertionsList = new ArrayList<>(); assertions.forEach(assertionsList::add); - try { - await() - .untilAsserted(() -> doAssertTraces(traceComparator, assertionsList, verifyScopeVersion)); - } catch (Throwable t) { - // awaitility is doing a jmx call that is not implemented in GraalVM: - // call: - // https://github.com/awaitility/awaitility/blob/fbe16add874b4260dd240108304d5c0be84eabc8/awaitility/src/main/java/org/awaitility/core/ConditionAwaiter.java#L157 - // see https://github.com/oracle/graal/issues/6101 (spring boot graal native image) - if (t.getClass().getName().equals("com.oracle.svm.core.jdk.UnsupportedFeatureError") - || t instanceof ConditionTimeoutException) { - // Don't throw this failure since the stack is the awaitility thread, causing confusion. - // Instead, just assert one more time on the test thread, which will fail with a better - // stack trace. - // TODO: There is probably a better way to do this. - doAssertTraces(traceComparator, assertionsList, verifyScopeVersion); - } else { - throw t; - } - } + AwaitUtil.awaitUntilAsserted( + () -> doAssertTraces(traceComparator, assertionsList, verifyScopeVersion)); } private > void doAssertTraces( @@ -159,31 +141,35 @@ private > void doAssertTraces( */ public final void waitAndAssertMetrics( String instrumentationName, String metricName, Consumer> assertion) { - await() - .untilAsserted( - () -> - assertion.accept( - assertThat(getExportedMetrics()) - .filteredOn( - data -> - data.getInstrumentationScopeInfo() - .getName() - .equals(instrumentationName) - && data.getName().equals(metricName)))); + + AwaitUtil.awaitUntilAsserted( + () -> + assertion.accept( + assertThat(getExportedMetrics()) + .describedAs( + "Metrics for instrumentation %s and metric name %s", + instrumentationName, metricName) + .filteredOn( + data -> + data.getInstrumentationScopeInfo().getName().equals(instrumentationName) + && data.getName().equals(metricName)))); } @SafeVarargs public final void waitAndAssertMetrics( String instrumentationName, Consumer... assertions) { - await() - .untilAsserted( - () -> { - Collection metrics = instrumentationMetrics(instrumentationName); - assertThat(metrics).isNotEmpty(); - for (Consumer assertion : assertions) { - assertThat(metrics).anySatisfy(metric -> assertion.accept(assertThat(metric))); - } - }); + AwaitUtil.awaitUntilAsserted( + () -> { + Collection metrics = instrumentationMetrics(instrumentationName); + assertThat(metrics).isNotEmpty(); + for (int i = 0; i < assertions.length; i++) { + int index = i; + assertThat(metrics) + .describedAs( + "Metrics for instrumentation %s and assertion %d", instrumentationName, index) + .anySatisfy(metric -> assertions[index].accept(assertThat(metric))); + } + }); } private List instrumentationMetrics(String instrumentationName) { diff --git a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/internal/AwaitUtil.java b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/internal/AwaitUtil.java new file mode 100644 index 000000000000..c4900901c943 --- /dev/null +++ b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/internal/AwaitUtil.java @@ -0,0 +1,44 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.testing.internal; + +import static org.awaitility.Awaitility.await; + +import org.awaitility.core.ConditionFactory; +import org.awaitility.core.ConditionTimeoutException; + +/** + * This class is internal and is hence not for public use. Its APIs are unstable and can change at + * any time. + */ +public class AwaitUtil { + private AwaitUtil() {} + + public static void awaitUntilAsserted(Runnable runnable) { + awaitUntilAsserted(runnable, await()); + } + + public static void awaitUntilAsserted(Runnable runnable, ConditionFactory conditionFactory) { + try { + conditionFactory.untilAsserted(runnable::run); + } catch (Throwable t) { + // awaitility is doing a jmx call that is not implemented in GraalVM: + // call: + // https://github.com/awaitility/awaitility/blob/fbe16add874b4260dd240108304d5c0be84eabc8/awaitility/src/main/java/org/awaitility/core/ConditionAwaiter.java#L157 + // see https://github.com/oracle/graal/issues/6101 (spring boot graal native image) + if (t.getClass().getName().equals("com.oracle.svm.core.jdk.UnsupportedFeatureError") + || t instanceof ConditionTimeoutException) { + // Don't throw this failure since the stack is the awaitility thread, causing confusion. + // Instead, just assert one more time on the test thread, which will fail with a better + // stack trace - that is on the same thread as the test. + // TODO: There is probably a better way to do this. + runnable.run(); + } else { + throw t; + } + } + } +} diff --git a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/InstrumentationExtension.java b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/InstrumentationExtension.java index d1d8735be37a..a9bf57c5c56f 100644 --- a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/InstrumentationExtension.java +++ b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/InstrumentationExtension.java @@ -12,6 +12,7 @@ import io.opentelemetry.context.ContextStorage; import io.opentelemetry.instrumentation.testing.InstrumentationTestRunner; import io.opentelemetry.instrumentation.testing.LibraryTestRunner; +import io.opentelemetry.instrumentation.testing.internal.AwaitUtil; import io.opentelemetry.instrumentation.testing.util.ContextStorageCloser; import io.opentelemetry.instrumentation.testing.util.ThrowingRunnable; import io.opentelemetry.instrumentation.testing.util.ThrowingSupplier; @@ -125,12 +126,9 @@ public List> waitForTraces(int numberOfTraces) { * This waits up to 20 seconds, then times out. */ public List waitForLogRecords(int numberOfLogRecords) { - await() - .timeout(Duration.ofSeconds(20)) - .untilAsserted( - () -> - assertThat(testRunner.getExportedLogRecords().size()) - .isEqualTo(numberOfLogRecords)); + AwaitUtil.awaitUntilAsserted( + () -> assertThat(testRunner.getExportedLogRecords().size()).isEqualTo(numberOfLogRecords), + await().timeout(Duration.ofSeconds(20))); return testRunner.getExportedLogRecords(); } From 1c69808455ae723f08cbc4ecb93903b80f5977ec Mon Sep 17 00:00:00 2001 From: Gregor Zeitlinger Date: Wed, 29 Jan 2025 14:19:01 +0100 Subject: [PATCH 2/2] cleanup --- .../testing/InstrumentationTestRunner.java | 9 ++++----- .../instrumentation/testing/internal/AwaitUtil.java | 2 +- .../testing/junit/InstrumentationExtension.java | 4 ++-- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/InstrumentationTestRunner.java b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/InstrumentationTestRunner.java index b85e7422236c..2bc3027da2b3 100644 --- a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/InstrumentationTestRunner.java +++ b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/InstrumentationTestRunner.java @@ -5,10 +5,10 @@ package io.opentelemetry.instrumentation.testing; +import static io.opentelemetry.instrumentation.testing.internal.AwaitUtil.awaitUntilAsserted; import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.assertThat; import io.opentelemetry.api.OpenTelemetry; -import io.opentelemetry.instrumentation.testing.internal.AwaitUtil; import io.opentelemetry.instrumentation.testing.util.TelemetryDataUtil; import io.opentelemetry.instrumentation.testing.util.ThrowingRunnable; import io.opentelemetry.instrumentation.testing.util.ThrowingSupplier; @@ -117,8 +117,7 @@ private > void waitAndAssertTraces( List assertionsList = new ArrayList<>(); assertions.forEach(assertionsList::add); - AwaitUtil.awaitUntilAsserted( - () -> doAssertTraces(traceComparator, assertionsList, verifyScopeVersion)); + awaitUntilAsserted(() -> doAssertTraces(traceComparator, assertionsList, verifyScopeVersion)); } private > void doAssertTraces( @@ -142,7 +141,7 @@ private > void doAssertTraces( public final void waitAndAssertMetrics( String instrumentationName, String metricName, Consumer> assertion) { - AwaitUtil.awaitUntilAsserted( + awaitUntilAsserted( () -> assertion.accept( assertThat(getExportedMetrics()) @@ -158,7 +157,7 @@ public final void waitAndAssertMetrics( @SafeVarargs public final void waitAndAssertMetrics( String instrumentationName, Consumer... assertions) { - AwaitUtil.awaitUntilAsserted( + awaitUntilAsserted( () -> { Collection metrics = instrumentationMetrics(instrumentationName); assertThat(metrics).isNotEmpty(); diff --git a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/internal/AwaitUtil.java b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/internal/AwaitUtil.java index c4900901c943..33fff185e8cd 100644 --- a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/internal/AwaitUtil.java +++ b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/internal/AwaitUtil.java @@ -14,7 +14,7 @@ * This class is internal and is hence not for public use. Its APIs are unstable and can change at * any time. */ -public class AwaitUtil { +public final class AwaitUtil { private AwaitUtil() {} public static void awaitUntilAsserted(Runnable runnable) { diff --git a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/InstrumentationExtension.java b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/InstrumentationExtension.java index a9bf57c5c56f..cf3e60694300 100644 --- a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/InstrumentationExtension.java +++ b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/InstrumentationExtension.java @@ -5,6 +5,7 @@ package io.opentelemetry.instrumentation.testing.junit; +import static io.opentelemetry.instrumentation.testing.internal.AwaitUtil.awaitUntilAsserted; import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.assertThat; import static org.awaitility.Awaitility.await; @@ -12,7 +13,6 @@ import io.opentelemetry.context.ContextStorage; import io.opentelemetry.instrumentation.testing.InstrumentationTestRunner; import io.opentelemetry.instrumentation.testing.LibraryTestRunner; -import io.opentelemetry.instrumentation.testing.internal.AwaitUtil; import io.opentelemetry.instrumentation.testing.util.ContextStorageCloser; import io.opentelemetry.instrumentation.testing.util.ThrowingRunnable; import io.opentelemetry.instrumentation.testing.util.ThrowingSupplier; @@ -126,7 +126,7 @@ public List> waitForTraces(int numberOfTraces) { * This waits up to 20 seconds, then times out. */ public List waitForLogRecords(int numberOfLogRecords) { - AwaitUtil.awaitUntilAsserted( + awaitUntilAsserted( () -> assertThat(testRunner.getExportedLogRecords().size()).isEqualTo(numberOfLogRecords), await().timeout(Duration.ofSeconds(20))); return testRunner.getExportedLogRecords();