Extend Experiment to report more detail when results don't match.

This commit is contained in:
Jon Chambers 2020-06-11 12:30:10 -04:00 committed by Jon Chambers
parent 75cbfa2898
commit a0bebca1e6
2 changed files with 73 additions and 51 deletions

View File

@ -15,35 +15,49 @@ import java.util.function.Supplier;
*/ */
public class Experiment { public class Experiment {
private final Timer matchTimer; private final Timer matchTimer;
private final Timer mismatchTimer; private final Timer errorTimer;
private final Timer errorTimer;
static final String OUTCOME_TAG = "outcome"; private final Timer bothPresentMismatchTimer;
static final String MATCH_OUTCOME = "match"; private final Timer controlNullMismatchTimer;
static final String MISMATCH_OUTCOME = "mismatch"; private final Timer experimentNullMismatchTimer;
static final String ERROR_OUTCOME = "error";
private static final String OUTCOME_TAG = "outcome";
private static final String MATCH_OUTCOME = "match";
private static final String MISMATCH_OUTCOME = "mismatch";
private static final String ERROR_OUTCOME = "error";
private static final String MISMATCH_TYPE_TAG = "mismatchType";
private static final String BOTH_PRESENT_MISMATCH = "bothPresent";
private static final String CONTROL_NULL_MISMATCH = "controlResultNull";
private static final String EXPERIMENT_NULL_MISMATCH = "experimentResultNull";
public Experiment(final String... names) { public Experiment(final String... names) {
this(buildTimer(MATCH_OUTCOME, names), buildTimer(MISMATCH_OUTCOME, names), buildTimer(ERROR_OUTCOME, names)); this(buildTimer(MATCH_OUTCOME, names).register(Metrics.globalRegistry),
buildTimer(ERROR_OUTCOME, names).register(Metrics.globalRegistry),
buildTimer(MISMATCH_OUTCOME, names).tag(MISMATCH_TYPE_TAG, BOTH_PRESENT_MISMATCH).register(Metrics.globalRegistry),
buildTimer(MISMATCH_OUTCOME, names).tag(MISMATCH_TYPE_TAG, CONTROL_NULL_MISMATCH).register(Metrics.globalRegistry),
buildTimer(MISMATCH_OUTCOME, names).tag(MISMATCH_TYPE_TAG, EXPERIMENT_NULL_MISMATCH).register(Metrics.globalRegistry));
} }
private static Timer buildTimer(final String outcome, final String... names) { private static Timer.Builder buildTimer(final String outcome, final String... names) {
if (names == null || names.length == 0) { if (names == null || names.length == 0) {
throw new IllegalArgumentException("Experiments must have a name"); throw new IllegalArgumentException("Experiments must have a name");
} }
return Timer.builder(MetricRegistry.name(Experiment.class, names)) return Timer.builder(MetricRegistry.name(Experiment.class, names))
.tag(OUTCOME_TAG, outcome) .tag(OUTCOME_TAG, outcome)
.publishPercentileHistogram() .publishPercentileHistogram();
.register(Metrics.globalRegistry);
} }
@VisibleForTesting @VisibleForTesting
Experiment(final Timer matchTimer, final Timer mismatchTimer, final Timer errorTimer) { Experiment(final Timer matchTimer, final Timer errorTimer, final Timer bothPresentMismatchTimer, final Timer controlNullMismatchTimer, final Timer experimentNullMismatchTimer) {
this.matchTimer = matchTimer; this.matchTimer = matchTimer;
this.mismatchTimer = mismatchTimer;
this.errorTimer = errorTimer; this.errorTimer = errorTimer;
this.bothPresentMismatchTimer = bothPresentMismatchTimer;
this.controlNullMismatchTimer = controlNullMismatchTimer;
this.experimentNullMismatchTimer = experimentNullMismatchTimer;
} }
public <T> void compareFutureResult(final T expected, final CompletionStage<T> experimentStage) { public <T> void compareFutureResult(final T expected, final CompletionStage<T> experimentStage) {
@ -78,11 +92,18 @@ public class Experiment {
} }
private <T> void recordResult(final long durationNanos, final T expected, final T actual) { private <T> void recordResult(final long durationNanos, final T expected, final T actual) {
final boolean shouldIgnore = actual == null && expected != null; final Timer timer;
if (!shouldIgnore) { if (Objects.equals(expected, actual)) {
final Timer timer = Objects.equals(expected, actual) ? matchTimer : mismatchTimer; timer = matchTimer;
timer.record(durationNanos, TimeUnit.NANOSECONDS); } else if (expected == null) {
timer = controlNullMismatchTimer;
} else if (actual == null) {
timer = experimentNullMismatchTimer;
} else {
timer = bothPresentMismatchTimer;
} }
timer.record(durationNanos, TimeUnit.NANOSECONDS);
} }
} }

View File

@ -1,47 +1,36 @@
package org.whispersystems.textsecuregcm.experiment; package org.whispersystems.textsecuregcm.experiment;
import io.micrometer.core.instrument.Meter;
import io.micrometer.core.instrument.MeterRegistry;
import io.micrometer.core.instrument.Tag;
import io.micrometer.core.instrument.Timer; import io.micrometer.core.instrument.Timer;
import io.micrometer.core.instrument.distribution.DistributionStatisticConfig;
import org.LatencyUtils.PauseDetector;
import org.junit.Before; import org.junit.Before;
import org.junit.Test; import org.junit.Test;
import org.mockito.ArgumentCaptor;
import org.mockito.ArgumentMatchers;
import java.util.HashSet;
import java.util.Set;
import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletableFuture;
import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeUnit;
import static org.junit.Assert.*;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.ArgumentMatchers.anyLong;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.eq; import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.mock; import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoMoreInteractions;
import static org.mockito.Mockito.when;
public class ExperimentTest { public class ExperimentTest {
private Timer matchTimer; private Timer matchTimer;
private Timer mismatchTimer;
private Timer errorTimer; private Timer errorTimer;
private Timer bothPresentMismatchTimer;
private Timer controlNullMismatchTimer;
private Timer experimentNullMismatchTimer;
private Experiment experiment; private Experiment experiment;
@Before @Before
public void setUp() { public void setUp() {
matchTimer = mock(Timer.class); matchTimer = mock(Timer.class);
mismatchTimer = mock(Timer.class);
errorTimer = mock(Timer.class); errorTimer = mock(Timer.class);
bothPresentMismatchTimer = mock(Timer.class);
controlNullMismatchTimer = mock(Timer.class);
experimentNullMismatchTimer = mock(Timer.class);
experiment = new Experiment(matchTimer, mismatchTimer, errorTimer); experiment = new Experiment(matchTimer, errorTimer, bothPresentMismatchTimer, controlNullMismatchTimer, experimentNullMismatchTimer);
} }
@Test @Test
@ -51,9 +40,21 @@ public class ExperimentTest {
} }
@Test @Test
public void compareFutureResultMismatch() { public void compareFutureResultMismatchBothPresent() {
experiment.compareFutureResult(12, CompletableFuture.completedFuture(77)); experiment.compareFutureResult(12, CompletableFuture.completedFuture(77));
verify(mismatchTimer).record(anyLong(), eq(TimeUnit.NANOSECONDS)); verify(bothPresentMismatchTimer).record(anyLong(), eq(TimeUnit.NANOSECONDS));
}
@Test
public void compareFutureResultMismatchControlNull() {
experiment.compareFutureResult(null, CompletableFuture.completedFuture(77));
verify(controlNullMismatchTimer).record(anyLong(), eq(TimeUnit.NANOSECONDS));
}
@Test
public void compareFutureResultMismatchExperimentNull() {
experiment.compareFutureResult(12, CompletableFuture.completedFuture(null));
verify(experimentNullMismatchTimer).record(anyLong(), eq(TimeUnit.NANOSECONDS));
} }
@Test @Test
@ -62,12 +63,6 @@ public class ExperimentTest {
verify(errorTimer).record(anyLong(), eq(TimeUnit.NANOSECONDS)); verify(errorTimer).record(anyLong(), eq(TimeUnit.NANOSECONDS));
} }
@Test
public void compareFutureResultNoExperimentData() {
experiment.compareFutureResult(12, CompletableFuture.completedFuture(null));
verifyNoMoreInteractions(matchTimer, mismatchTimer, errorTimer);
}
@Test @Test
public void compareSupplierResultMatch() { public void compareSupplierResultMatch() {
experiment.compareSupplierResult(12, () -> 12); experiment.compareSupplierResult(12, () -> 12);
@ -75,9 +70,21 @@ public class ExperimentTest {
} }
@Test @Test
public void compareSupplierResultMismatch() { public void compareSupplierResultMismatchBothPresent() {
experiment.compareSupplierResult(12, () -> 77); experiment.compareSupplierResult(12, () -> 77);
verify(mismatchTimer).record(anyLong(), eq(TimeUnit.NANOSECONDS)); verify(bothPresentMismatchTimer).record(anyLong(), eq(TimeUnit.NANOSECONDS));
}
@Test
public void compareSupplierResultMismatchControlNull() {
experiment.compareSupplierResult(null, () -> 77);
verify(controlNullMismatchTimer).record(anyLong(), eq(TimeUnit.NANOSECONDS));
}
@Test
public void compareSupplierResultMismatchExperimentNull() {
experiment.compareSupplierResult(12, () -> null);
verify(experimentNullMismatchTimer).record(anyLong(), eq(TimeUnit.NANOSECONDS));
} }
@Test @Test
@ -85,10 +92,4 @@ public class ExperimentTest {
experiment.compareSupplierResult(12, () -> { throw new RuntimeException("OH NO"); }); experiment.compareSupplierResult(12, () -> { throw new RuntimeException("OH NO"); });
verify(errorTimer).record(anyLong(), eq(TimeUnit.NANOSECONDS)); verify(errorTimer).record(anyLong(), eq(TimeUnit.NANOSECONDS));
} }
@Test
public void compareSupplierResultNoExperimentData() {
experiment.compareSupplierResult(12, () -> null);
verifyNoMoreInteractions(mismatchTimer, mismatchTimer, errorTimer);
}
} }