diff --git a/azkaban-common/src/test/java/azkaban/metrics/CommonMetricsTest.java b/azkaban-common/src/test/java/azkaban/metrics/CommonMetricsTest.java
index 4c6287d..75ffc5a 100644
--- a/azkaban-common/src/test/java/azkaban/metrics/CommonMetricsTest.java
+++ b/azkaban-common/src/test/java/azkaban/metrics/CommonMetricsTest.java
@@ -16,48 +16,40 @@
package azkaban.metrics;
-import azkaban.metrics.MetricsTestUtility.DummyReporter;
-
-import java.util.concurrent.TimeUnit;
-import java.time.Duration;
-
-import org.junit.After;
import org.junit.Before;
-import org.junit.Ignore;
import org.junit.Test;
-@Ignore
-//todo: HappyRay Find a way to fix these flaky tests.
-public class CommonMetricsTest {
-
- private DummyReporter dr;
-
- @Before
- public void setup() {
- dr = new DummyReporter(MetricsManager.INSTANCE.getRegistry());
- dr.start(Duration.ofMillis(2).toMillis(), TimeUnit.MILLISECONDS);
- }
+import static org.junit.Assert.*;
- @After
- public void shutdown() {
- if (null != dr)
- dr.stop();
- dr = null;
- }
+public class CommonMetricsTest {
+ private MetricsTestUtility testUtil;
+ private CommonMetrics metrics;
- @Test
- public void testMarkDBConnectionMetrics() {
- MetricsTestUtility.testMeter("DB-Connection-meter", dr, CommonMetrics.INSTANCE::markDBConnection);
+ @Before
+ public void setUp() {
+ // Use of global state can cause problems e.g.
+ // The state is shared among tests.
+ // e.g. we can't run a variant of the testOOMWaitingJobMetrics twice since it relies on the initial state of
+ // the registry.
+ // This can also cause problem when we run tests in parallel in the future.
+ // todo HappyRay: move MetricsManager, CommonMetrics to use Juice.
+ testUtil = new MetricsTestUtility(MetricsManager.INSTANCE.getRegistry());
+ metrics = CommonMetrics.INSTANCE;
}
@Test
public void testDBConnectionTimeMetrics() {
- MetricsTestUtility.testGauge("dbConnectionTime", dr, CommonMetrics.INSTANCE::setDBConnectionTime);
+ metrics.setDBConnectionTime(14);
+ assertEquals(14, testUtil.getGaugeValue("dbConnectionTime"));
}
@Test
public void testOOMWaitingJobMetrics() {
- MetricsTestUtility.testGauge("OOM-waiting-job-count", dr, CommonMetrics.INSTANCE::incrementOOMJobWaitCount);
+ final String metricName = "OOM-waiting-job-count";
+
+ assertEquals(0, testUtil.getGaugeValue(metricName));
+ metrics.incrementOOMJobWaitCount();
+ assertEquals(1, testUtil.getGaugeValue(metricName));
}
}
diff --git a/azkaban-common/src/test/java/azkaban/metrics/MetricsTestUtility.java b/azkaban-common/src/test/java/azkaban/metrics/MetricsTestUtility.java
index 063581e..545b0af 100644
--- a/azkaban-common/src/test/java/azkaban/metrics/MetricsTestUtility.java
+++ b/azkaban-common/src/test/java/azkaban/metrics/MetricsTestUtility.java
@@ -16,116 +16,24 @@
package azkaban.metrics;
-import com.codahale.metrics.Counter;
-import com.codahale.metrics.Gauge;
-import com.codahale.metrics.Histogram;
-import com.codahale.metrics.Meter;
-import com.codahale.metrics.MetricFilter;
import com.codahale.metrics.MetricRegistry;
-import com.codahale.metrics.ScheduledReporter;
-import com.codahale.metrics.Timer;
-import org.junit.Assert;
-
-import java.util.HashMap;
-import java.util.Map;
-import java.util.SortedMap;
-import java.util.concurrent.TimeUnit;
-import java.util.function.Consumer;
/**
* This class is designed for a utility class to test drop wizard metrics
*/
public class MetricsTestUtility {
- /**
- * One Dummy Reporter extending {@link ScheduledReporter} collects metrics in various maps,
- * which test methods are able to access easily.
- */
- public static class DummyReporter extends ScheduledReporter{
-
- private Map<String, String> gaugeMap;
-
- private Map<String, Long> meterMap;
-
- public DummyReporter(MetricRegistry registry) {
- super(registry, "dummy-reporter", MetricFilter.ALL, TimeUnit.SECONDS, TimeUnit.MILLISECONDS);
- this.gaugeMap = new HashMap<>();
- this.meterMap = new HashMap<>();
- }
-
- @Override
- public void report(SortedMap<String, Gauge> gauges,
- SortedMap<String, Counter> counters,
- SortedMap<String, Histogram> histograms,
- SortedMap<String, Meter> meters,
- SortedMap<String, Timer> timers) {
+ // todo HappyRay: move singletons to Juice.
+ // This can cause problems when we run tests in parallel in the future.
+ private MetricRegistry registry;
- for (Map.Entry<String, Gauge> entry : gauges.entrySet()) {
- gaugeMap.put(entry.getKey(), entry.getValue().getValue().toString());
- }
-
- for (Map.Entry<String, Meter> entry : meters.entrySet()) {
- meterMap.put(entry.getKey(), entry.getValue().getCount());
- }
- }
-
- private String getGauge(String key) {
- return gaugeMap.get(key);
- }
-
- private long getMeter(String key) {
- return meterMap.getOrDefault(key, 0L);
- }
+ public MetricsTestUtility(MetricRegistry registry) {
+ this.registry = registry;
}
- public static void testMeter(String meterName, DummyReporter dr, Runnable runnable) {
-
- sleepMillis(20);
- long currMeter = dr.getMeter(meterName);
- runnable.run();
- sleepMillis(20);
- Assert.assertEquals(dr.getMeter(meterName), currMeter + 1);
-
- runnable.run();
- runnable.run();
- sleepMillis(20);
- Assert.assertEquals(dr.getMeter(meterName), currMeter + 3);
- }
-
- public static void testGauge(String meterName, DummyReporter dr, Runnable runnable) {
- sleepMillis(20);
- long currMeter = Long.valueOf(dr.getGauge(meterName));
- runnable.run();
- sleepMillis(20);
- Assert.assertEquals(Long.valueOf(dr.getGauge(meterName)).longValue(), currMeter + 1);
-
- runnable.run();
- runnable.run();
- sleepMillis(20);
- Assert.assertEquals(Long.valueOf(dr.getGauge(meterName)).longValue(), currMeter + 3);
- }
-
- public static void testGauge(String GaugeName, DummyReporter dr, Consumer<Long> func) {
- func.accept(1L);
-
- // needs time to let metrics reporter receive the updated value.
- sleepMillis(50);
- Assert.assertEquals(dr.getGauge(GaugeName), "1");
- func.accept(99L);
- sleepMillis(50);
- Assert.assertEquals(dr.getGauge(GaugeName), "99");
- }
-
-
- /**
- * Helper method to sleep milli seconds.
- */
- private static void sleepMillis(int numMilli) {
- try {
- Thread.sleep(numMilli);
- } catch (InterruptedException e) {
- e.printStackTrace();
- }
+ public long getGaugeValue(String name) {
+ // Assume that the gauge value can be converted to type long.
+ return (long) registry.getGauges().get(name).getValue();
}
}
diff --git a/azkaban-web-server/src/test/java/azkaban/webapp/WebMetricsTest.java b/azkaban-web-server/src/test/java/azkaban/webapp/WebMetricsTest.java
index 46a7cb3..c668ec7 100644
--- a/azkaban-web-server/src/test/java/azkaban/webapp/WebMetricsTest.java
+++ b/azkaban-web-server/src/test/java/azkaban/webapp/WebMetricsTest.java
@@ -17,47 +17,28 @@
package azkaban.webapp;
import azkaban.metrics.MetricsManager;
-import azkaban.metrics.MetricsTestUtility.DummyReporter;
import azkaban.metrics.MetricsTestUtility;
-
-import java.time.Duration;
-import java.util.concurrent.TimeUnit;
-
-import org.junit.After;
import org.junit.Before;
import org.junit.Test;
+import static org.junit.Assert.*;
-public class WebMetricsTest{
-
- private DummyReporter dr;
- @Before
- public void setup() {
- dr = new DummyReporter(MetricsManager.INSTANCE.getRegistry());
- dr.start(Duration.ofMillis(2).toMillis(), TimeUnit.MILLISECONDS);
- }
+public class WebMetricsTest {
- @After
- public void shutdown() {
- if (null != dr)
- dr.stop();
+ private MetricsTestUtility testUtil;
+ private WebMetrics metrics;
- dr = null;
+ @Before
+ public void setUp() {
+ // todo HappyRay: move MetricsManager, WebMetrics to use Juice.
+ testUtil = new MetricsTestUtility(MetricsManager.INSTANCE.getRegistry());
+ metrics = WebMetrics.INSTANCE;
}
@Test
public void testLogFetchLatencyMetrics() {
- MetricsTestUtility.testGauge("fetchLogLatency", dr, WebMetrics.INSTANCE::setFetchLogLatency);
- }
-
- @Test
- public void testWebPostCallMeter() {
- MetricsTestUtility.testMeter("Web-Post-Call-Meter", dr, WebMetrics.INSTANCE::markWebPostCall);
- }
-
- @Test
- public void testWebGetCallMeter() {
- MetricsTestUtility.testMeter("Web-Get-Call-Meter", dr, WebMetrics.INSTANCE::markWebGetCall);
+ metrics.setFetchLogLatency(14);
+ assertEquals(14, testUtil.getGaugeValue("fetchLogLatency"));
}
}