From 3c0c06e9d07c8895ae9a4df98a6206380bb40f6d Mon Sep 17 00:00:00 2001 From: Gregor Zeitlinger Date: Tue, 28 Jan 2025 18:59:30 +0100 Subject: [PATCH 1/3] 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 d8708dfba8c1586053572b3d63608209d414371f Mon Sep 17 00:00:00 2001 From: Gregor Zeitlinger Date: Wed, 29 Jan 2025 14:19:01 +0100 Subject: [PATCH 2/3] 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(); From b19f64b6cda5e63f9a13c080ea5974d827d3e270 Mon Sep 17 00:00:00 2001 From: Gregor Zeitlinger Date: Mon, 3 Feb 2025 11:06:08 +0100 Subject: [PATCH 3/3] cleanup --- .../testing/InstrumentationTestRunner.java | 37 +++++++++++++++- .../testing/internal/AwaitUtil.java | 44 ------------------- .../junit/InstrumentationExtension.java | 8 +--- 3 files changed, 37 insertions(+), 52 deletions(-) delete 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 2bc3027da2b3..d67b0dbfd0a7 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,8 +5,8 @@ package io.opentelemetry.instrumentation.testing; -import static io.opentelemetry.instrumentation.testing.internal.AwaitUtil.awaitUntilAsserted; 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.util.TelemetryDataUtil; @@ -18,6 +18,7 @@ import io.opentelemetry.sdk.testing.assertj.TraceAssert; import io.opentelemetry.sdk.testing.assertj.TracesAssert; import io.opentelemetry.sdk.trace.data.SpanData; +import java.time.Duration; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -29,6 +30,8 @@ import java.util.stream.Collectors; import javax.annotation.Nullable; import org.assertj.core.api.ListAssert; +import org.awaitility.core.ConditionFactory; +import org.awaitility.core.ConditionTimeoutException; /** * This interface defines a common set of operations for interaction with OpenTelemetry SDK and @@ -171,6 +174,13 @@ public final void waitAndAssertMetrics( }); } + public final List waitForLogRecords(int numberOfLogRecords) { + awaitUntilAsserted( + () -> assertThat(getExportedLogRecords().size()).isEqualTo(numberOfLogRecords), + await().timeout(Duration.ofSeconds(20))); + return getExportedLogRecords(); + } + private List instrumentationMetrics(String instrumentationName) { return getExportedMetrics().stream() .filter(m -> m.getInstrumentationScopeInfo().getName().equals(instrumentationName)) @@ -250,4 +260,29 @@ public final T runWithNonRecordingSpan(ThrowingSupplier throws E { return testInstrumenters.runWithNonRecordingSpan(callback); } + + private static void awaitUntilAsserted(Runnable runnable) { + awaitUntilAsserted(runnable, await()); + } + + private 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/internal/AwaitUtil.java b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/internal/AwaitUtil.java deleted file mode 100644 index 33fff185e8cd..000000000000 --- a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/internal/AwaitUtil.java +++ /dev/null @@ -1,44 +0,0 @@ -/* - * 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 final 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 cf3e60694300..b417b7d38091 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,9 +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; import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.context.ContextStorage; @@ -23,7 +21,6 @@ import io.opentelemetry.sdk.testing.assertj.MetricAssert; import io.opentelemetry.sdk.testing.assertj.TraceAssert; import io.opentelemetry.sdk.trace.data.SpanData; -import java.time.Duration; import java.util.ArrayList; import java.util.Arrays; import java.util.Comparator; @@ -126,10 +123,7 @@ public List> waitForTraces(int numberOfTraces) { * This waits up to 20 seconds, then times out. */ public List waitForLogRecords(int numberOfLogRecords) { - awaitUntilAsserted( - () -> assertThat(testRunner.getExportedLogRecords().size()).isEqualTo(numberOfLogRecords), - await().timeout(Duration.ofSeconds(20))); - return testRunner.getExportedLogRecords(); + return testRunner.waitForLogRecords(numberOfLogRecords); } @SafeVarargs