Fix flaky InstrumentGarbageCollectionBenchmarkTest (#6221)

This commit is contained in:
Asaf Mesika 2024-02-12 17:35:03 +02:00 committed by GitHub
parent 695ed5350b
commit 0e84508905
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 42 additions and 22 deletions

View File

@ -32,6 +32,7 @@ jobs:
- os: ubuntu-20.04 - os: ubuntu-20.04
test-java-version: 17 test-java-version: 17
coverage: true coverage: true
jmh-based-tests: true
steps: steps:
- uses: actions/checkout@v4 - uses: actions/checkout@v4
@ -58,6 +59,8 @@ jobs:
-Porg.gradle.java.installations.paths=${{ steps.setup-java-test.outputs.path }},${{ steps.setup-java.outputs.path }} -Porg.gradle.java.installations.paths=${{ steps.setup-java-test.outputs.path }},${{ steps.setup-java.outputs.path }}
env: env:
GRADLE_ENTERPRISE_ACCESS_KEY: ${{ secrets.GRADLE_ENTERPRISE_ACCESS_KEY }} GRADLE_ENTERPRISE_ACCESS_KEY: ${{ secrets.GRADLE_ENTERPRISE_ACCESS_KEY }}
# JMH-based tests run only if this environment variable is set to true
RUN_JMH_BASED_TESTS: ${{ matrix.jmh-based-tests }}
- name: Check for diff - name: Check for diff
# The jApiCmp diff compares current to latest, which isn't appropriate for release branches # The jApiCmp diff compares current to latest, which isn't appropriate for release branches

View File

@ -1,9 +1,2 @@
Comparing source compatibility of against Comparing source compatibility of against
*** MODIFIED CLASS: PUBLIC FINAL io.opentelemetry.exporter.zipkin.ZipkinSpanExporterBuilder (not serializable) No changes.
=== CLASS FILE FORMAT VERSION: 52.0 <- 52.0
=== UNCHANGED METHOD: PUBLIC io.opentelemetry.exporter.zipkin.ZipkinSpanExporterBuilder setEncoder(zipkin2.codec.BytesEncoder<zipkin2.Span><zipkin2.Span>)
+++ NEW ANNOTATION: java.lang.Deprecated
+++ NEW METHOD: PUBLIC(+) io.opentelemetry.exporter.zipkin.ZipkinSpanExporterBuilder setEncoder(zipkin2.reporter.BytesEncoder<zipkin2.Span>)
=== UNCHANGED METHOD: PUBLIC io.opentelemetry.exporter.zipkin.ZipkinSpanExporterBuilder setSender(zipkin2.reporter.Sender)
+++ NEW ANNOTATION: java.lang.Deprecated
+++ NEW METHOD: PUBLIC(+) io.opentelemetry.exporter.zipkin.ZipkinSpanExporterBuilder setSender(zipkin2.reporter.BytesMessageSender)

View File

@ -45,11 +45,12 @@ public class InstrumentGarbageCollectionBenchmarkTest {
@SuppressWarnings("rawtypes") @SuppressWarnings("rawtypes")
@Test @Test
public void normalizedAllocationRateTest() throws RunnerException { public void normalizedAllocationRateTest() throws RunnerException {
// GitHub CI has an environment variable (CI=true). We can use it to skip // OTel GitHub CI Workflow (see .github/) sets an environment variable
// this test since it's a lengthy one (roughly 10 seconds) and have it running // (RUN_JMH_BASED_TESTS=true).
// only in GitHub CI // We set it only there since it's a lengthy test (roughly 2.5min)
// and we want to run it only in CI.
Assumptions.assumeTrue( Assumptions.assumeTrue(
"true".equals(System.getenv("CI")), "true".equals(System.getenv("RUN_JMH_BASED_TESTS")),
"This test should only run in GitHub CI since it's long"); "This test should only run in GitHub CI since it's long");
// Runs InstrumentGarbageCollectionBenchmark // Runs InstrumentGarbageCollectionBenchmark
@ -91,7 +92,7 @@ public class InstrumentGarbageCollectionBenchmarkTest {
} }
testInstrumentTypeResultsMap.forEach( testInstrumentTypeResultsMap.forEach(
(testInstrumentType, testInstrumentTypeResults) -> { (testInstrumentTypeString, testInstrumentTypeResults) -> {
Map<String, Map<String, Double>> resultMap = Map<String, Map<String, Double>> resultMap =
testInstrumentTypeResults.aggregationTemporalityToMemoryModeResult; testInstrumentTypeResults.aggregationTemporalityToMemoryModeResult;
assertThat(resultMap).hasSameSizeAs(AggregationTemporality.values()); assertThat(resultMap).hasSameSizeAs(AggregationTemporality.values());
@ -108,9 +109,11 @@ public class InstrumentGarbageCollectionBenchmarkTest {
assertThat(immutableDataAllocRate).isNotNull().isNotZero(); assertThat(immutableDataAllocRate).isNotNull().isNotZero();
assertThat(reusableDataAllocRate).isNotNull().isNotZero(); assertThat(reusableDataAllocRate).isNotNull().isNotZero();
TestInstrumentType testInstrumentType =
TestInstrumentType.valueOf(testInstrumentTypeString);
float dataAllocRateReductionPercentage = float dataAllocRateReductionPercentage =
TestInstrumentType.valueOf(testInstrumentType) testInstrumentType.getDataAllocRateReductionPercentage();
.getDataAllocRateReductionPercentage(); double allowedOffset = testInstrumentType.getAllowedPercentOffset();
// If this test suddenly fails for you this means you have changed the code in a way // If this test suddenly fails for you this means you have changed the code in a way
// that allocates more memory than before. You can find out where, by running // that allocates more memory than before. You can find out where, by running
@ -119,8 +122,8 @@ public class InstrumentGarbageCollectionBenchmarkTest {
assertThat(100 - (reusableDataAllocRate / immutableDataAllocRate) * 100) assertThat(100 - (reusableDataAllocRate / immutableDataAllocRate) * 100)
.describedAs( .describedAs(
"Aggregation temporality = %s, testInstrumentType = %s", "Aggregation temporality = %s, testInstrumentType = %s",
aggregationTemporality, testInstrumentType) aggregationTemporality, testInstrumentTypeString)
.isCloseTo(dataAllocRateReductionPercentage, Offset.offset(2.0)); .isCloseTo(dataAllocRateReductionPercentage, Offset.offset(allowedOffset));
}); });
}); });
} }

View File

@ -24,31 +24,52 @@ public enum TestInstrumentType {
ASYNC_COUNTER(AsyncCounterTester::new), ASYNC_COUNTER(AsyncCounterTester::new),
EXPONENTIAL_HISTOGRAM(ExponentialHistogramTester::new), EXPONENTIAL_HISTOGRAM(ExponentialHistogramTester::new),
EXPLICIT_BUCKET(ExplicitBucketHistogramTester::new), EXPLICIT_BUCKET(ExplicitBucketHistogramTester::new),
LONG_SUM(LongSumTester::new, /* dataAllocRateReductionPercentage= */ 97.3f), LONG_SUM(
DOUBLE_SUM(DoubleSumTester::new, /* dataAllocRateReductionPercentage= */ 97.3f), LongSumTester::new,
LONG_LAST_VALUE(LongLastValueTester::new, /* dataAllocRateReductionPercentage= */ 97.3f), /* dataAllocRateReductionPercentage= */ 97.3f,
DOUBLE_LAST_VALUE(DoubleLastValueTester::new, /* dataAllocRateReductionPercentage= */ 97.3f); /* allowedPercentOffset= */ 4.0f),
DOUBLE_SUM(
DoubleSumTester::new,
/* dataAllocRateReductionPercentage= */ 97.3f,
/* allowedPercentOffset= */ 2.0f),
LONG_LAST_VALUE(
LongLastValueTester::new,
/* dataAllocRateReductionPercentage= */ 97.3f,
/* allowedPercentOffset= */ 4.0f),
DOUBLE_LAST_VALUE(
DoubleLastValueTester::new,
/* dataAllocRateReductionPercentage= */ 97.3f,
/* allowedPercentOffset= */ 4.0f);
private final Supplier<? extends InstrumentTester> instrumentTesterInitializer; private final Supplier<? extends InstrumentTester> instrumentTesterInitializer;
private final float dataAllocRateReductionPercentage; private final float dataAllocRateReductionPercentage;
private final double allowedPercentOffset;
@SuppressWarnings("unused")
TestInstrumentType(Supplier<? extends InstrumentTester> instrumentTesterInitializer) { TestInstrumentType(Supplier<? extends InstrumentTester> instrumentTesterInitializer) {
this.dataAllocRateReductionPercentage = 99.8f; // default this.dataAllocRateReductionPercentage = 99.8f; // default
this.instrumentTesterInitializer = instrumentTesterInitializer; this.instrumentTesterInitializer = instrumentTesterInitializer;
this.allowedPercentOffset = 2.0f;
} }
// Some instruments have different reduction percentage. // Some instruments have different reduction percentage.
TestInstrumentType( TestInstrumentType(
Supplier<? extends InstrumentTester> instrumentTesterInitializer, Supplier<? extends InstrumentTester> instrumentTesterInitializer,
float dataAllocRateReductionPercentage) { float dataAllocRateReductionPercentage,
float allowedPercentOffset) {
this.instrumentTesterInitializer = instrumentTesterInitializer; this.instrumentTesterInitializer = instrumentTesterInitializer;
this.dataAllocRateReductionPercentage = dataAllocRateReductionPercentage; this.dataAllocRateReductionPercentage = dataAllocRateReductionPercentage;
this.allowedPercentOffset = allowedPercentOffset;
} }
float getDataAllocRateReductionPercentage() { float getDataAllocRateReductionPercentage() {
return dataAllocRateReductionPercentage; return dataAllocRateReductionPercentage;
} }
public double getAllowedPercentOffset() {
return allowedPercentOffset;
}
InstrumentTester createInstrumentTester() { InstrumentTester createInstrumentTester() {
return instrumentTesterInitializer.get(); return instrumentTesterInitializer.get();
} }