diff --git a/pinot-common/pom.xml b/pinot-common/pom.xml index 30b366a92114..7d7481f0940a 100644 --- a/pinot-common/pom.xml +++ b/pinot-common/pom.xml @@ -55,13 +55,6 @@ false - - - **/DropwizardBrokerPrometheusMetricsTest.java - **/DropwizardServerPrometheusMetricsTest.java - **/DropwizardMinionPrometheusMetricsTest.java - **/DropwizardControllerPrometheusMetricsTest.java - @@ -282,11 +275,6 @@ - - org.apache.pinot - pinot-yammer - test - org.testng testng @@ -302,11 +290,6 @@ equalsverifier test - - org.apache.pinot - pinot-dropwizard - test - org.assertj assertj-core diff --git a/pinot-common/src/main/java/org/apache/pinot/common/metrics/AbstractMetrics.java b/pinot-common/src/main/java/org/apache/pinot/common/metrics/AbstractMetrics.java index 4c879e127868..c594188855c9 100644 --- a/pinot-common/src/main/java/org/apache/pinot/common/metrics/AbstractMetrics.java +++ b/pinot-common/src/main/java/org/apache/pinot/common/metrics/AbstractMetrics.java @@ -94,6 +94,10 @@ public PinotMetricsRegistry getMetricsRegistry() { return _metricsRegistry; } + public String getMetricPrefix() { + return _metricPrefix; + } + public interface QueryPhase { String getQueryPhaseName(); diff --git a/pinot-common/src/test/java/org/apache/pinot/common/failuredetector/ConnectionFailureDetectorTest.java b/pinot-common/src/test/java/org/apache/pinot/common/failuredetector/ConnectionFailureDetectorTest.java index 5e605e967a02..4c6945d0f702 100644 --- a/pinot-common/src/test/java/org/apache/pinot/common/failuredetector/ConnectionFailureDetectorTest.java +++ b/pinot-common/src/test/java/org/apache/pinot/common/failuredetector/ConnectionFailureDetectorTest.java @@ -26,7 +26,7 @@ import org.apache.pinot.common.metrics.BrokerMetrics; import org.apache.pinot.common.metrics.MetricValueUtils; import org.apache.pinot.spi.env.PinotConfiguration; -import org.apache.pinot.spi.metrics.PinotMetricUtils; +import org.apache.pinot.spi.metrics.NoopPinotMetricsRegistry; import org.apache.pinot.spi.utils.CommonConstants.Broker; import org.apache.pinot.util.TestUtils; import org.testng.annotations.AfterClass; @@ -53,7 +53,7 @@ public void setUp() { config.setProperty(Broker.FailureDetector.CONFIG_OF_TYPE, Broker.FailureDetector.Type.CONNECTION.name()); config.setProperty(Broker.FailureDetector.CONFIG_OF_RETRY_INITIAL_DELAY_MS, 100); config.setProperty(Broker.FailureDetector.CONFIG_OF_RETRY_DELAY_FACTOR, 1); - _brokerMetrics = new BrokerMetrics(PinotMetricUtils.getPinotMetricsRegistry()); + _brokerMetrics = new BrokerMetrics(new NoopPinotMetricsRegistry()); _failureDetector = FailureDetectorFactory.getFailureDetector(config, _brokerMetrics); assertTrue(_failureDetector instanceof ConnectionFailureDetector); _healthyServerNotifier = new HealthyServerNotifier(); diff --git a/pinot-common/src/test/java/org/apache/pinot/common/metrics/AbstractMetricsTest.java b/pinot-common/src/test/java/org/apache/pinot/common/metrics/AbstractMetricsTest.java index c6bd8d8c30b6..6ff9c4e2331a 100644 --- a/pinot-common/src/test/java/org/apache/pinot/common/metrics/AbstractMetricsTest.java +++ b/pinot-common/src/test/java/org/apache/pinot/common/metrics/AbstractMetricsTest.java @@ -18,7 +18,6 @@ */ package org.apache.pinot.common.metrics; -import com.yammer.metrics.core.MetricName; import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; @@ -28,217 +27,192 @@ import java.util.function.IntConsumer; import java.util.function.Supplier; import java.util.stream.IntStream; -import org.apache.pinot.plugin.metrics.yammer.YammerMetricsRegistry; import org.apache.pinot.spi.env.PinotConfiguration; import org.apache.pinot.spi.metrics.PinotMeter; +import org.apache.pinot.spi.metrics.PinotMetricName; import org.apache.pinot.spi.metrics.PinotMetricUtils; +import org.apache.pinot.spi.metrics.PinotMetricsRegistry; import org.testng.Assert; +import org.testng.annotations.AfterClass; import org.testng.annotations.Test; import static org.apache.pinot.spi.utils.CommonConstants.CONFIG_OF_METRICS_FACTORY_CLASS_NAME; -public class AbstractMetricsTest { +/** + * Base test for {@link AbstractMetrics} behavior. Concrete subclasses supply the {@link PinotMetricsRegistry} + * implementation, a matching {@link MetricsInspector}, and a gauge-value reader — allowing the same test suite to run + * against the in-memory fake registry (in pinot-common) and against real yammer/dropwizard registries (in their + * respective plugin modules). + * + *

Subclasses must: + *

+ */ +public abstract class AbstractMetricsTest { + + protected abstract String metricsFactoryClassName(); + + protected abstract PinotMetricsRegistry buildRegistry(); + + protected abstract MetricsInspector createInspector(PinotMetricsRegistry registry); + + protected abstract long getGaugeValue(AbstractMetrics metrics, String metricName); + + protected ControllerMetrics buildTestMetrics() { + PinotConfiguration config = new PinotConfiguration(); + config.setProperty(CONFIG_OF_METRICS_FACTORY_CLASS_NAME, metricsFactoryClassName()); + PinotMetricUtils.init(config); + return new ControllerMetrics(buildRegistry()); + } + + /** + * Tear down the PinotMetricUtils static factory after each subclass finishes. The factory installs a + * {@link org.apache.pinot.spi.metrics.PinotJmxReporter}; leaving it registered can cause cross-test JMX collisions + * (e.g. duplicate MBean names) when multiple subclasses run in the same JVM. + */ + @AfterClass + public void cleanUpMetricsFactory() { + PinotMetricUtils.cleanUp(); + } @Test public void testAddOrUpdateGauge() { - PinotConfiguration pinotConfiguration = new PinotConfiguration(); - pinotConfiguration.setProperty(CONFIG_OF_METRICS_FACTORY_CLASS_NAME, - "org.apache.pinot.plugin.metrics.yammer.YammerMetricsFactory"); - PinotMetricUtils.init(pinotConfiguration); - ControllerMetrics controllerMetrics = new ControllerMetrics(new YammerMetricsRegistry()); + ControllerMetrics controllerMetrics = buildTestMetrics(); String metricName = "test"; - // add gauge + controllerMetrics.setOrUpdateGauge(metricName, () -> 1L); - Assert.assertEquals(MetricValueUtils.getGaugeValue(controllerMetrics, metricName), 1); + Assert.assertEquals(getGaugeValue(controllerMetrics, metricName), 1); - // update gauge controllerMetrics.setOrUpdateGauge(metricName, () -> 2L); - Assert.assertEquals(MetricValueUtils.getGaugeValue(controllerMetrics, metricName), 2); + Assert.assertEquals(getGaugeValue(controllerMetrics, metricName), 2); - // remove gauge controllerMetrics.removeGauge(metricName); Assert.assertTrue(controllerMetrics.getMetricsRegistry().allMetrics().isEmpty()); } @Test - public void testMultipleUpdatesToGauge() throws InterruptedException { - PinotConfiguration pinotConfiguration = new PinotConfiguration(); - pinotConfiguration.setProperty(CONFIG_OF_METRICS_FACTORY_CLASS_NAME, - "org.apache.pinot.plugin.metrics.yammer.YammerMetricsFactory"); - PinotMetricUtils.init(pinotConfiguration); - ControllerMetrics controllerMetrics = new ControllerMetrics(new YammerMetricsRegistry()); + public void testMultipleUpdatesToGauge() { + ControllerMetrics controllerMetrics = buildTestMetrics(); String metricName = "testMultipleUpdates"; - // update and remove gauge simultaneously - IntStream.range(0, 1000).forEach(i -> { - controllerMetrics.setOrUpdateGauge(metricName, () -> (long) i); - }); + IntStream.range(0, 1000).forEach(i -> controllerMetrics.setOrUpdateGauge(metricName, () -> (long) i)); - // Verify final value - Assert.assertEquals(MetricValueUtils.getGaugeValue(controllerMetrics, metricName), 999); - // remove gauge + Assert.assertEquals(getGaugeValue(controllerMetrics, metricName), 999); controllerMetrics.removeGauge(metricName); Assert.assertTrue(controllerMetrics.getMetricsRegistry().allMetrics().isEmpty()); } @Test public void testRemoveNonExistentGauge() { - PinotConfiguration pinotConfiguration = new PinotConfiguration(); - pinotConfiguration.setProperty(CONFIG_OF_METRICS_FACTORY_CLASS_NAME, - "org.apache.pinot.plugin.metrics.yammer.YammerMetricsFactory"); - PinotMetricUtils.init(pinotConfiguration); - ControllerMetrics controllerMetrics = new ControllerMetrics(new YammerMetricsRegistry()); - String metricName = "testNonExistent"; - - // Attempt to remove a nonexistent gauge - controllerMetrics.removeGauge(metricName); + ControllerMetrics controllerMetrics = buildTestMetrics(); + controllerMetrics.removeGauge("testNonExistent"); Assert.assertTrue(controllerMetrics.getMetricsRegistry().allMetrics().isEmpty()); } @Test public void testMultipleGauges() { - PinotConfiguration pinotConfiguration = new PinotConfiguration(); - pinotConfiguration.setProperty(CONFIG_OF_METRICS_FACTORY_CLASS_NAME, - "org.apache.pinot.plugin.metrics.yammer.YammerMetricsFactory"); - PinotMetricUtils.init(pinotConfiguration); - ControllerMetrics controllerMetrics = new ControllerMetrics(new YammerMetricsRegistry()); - String metricName1 = "testMultiple1"; - String metricName2 = "testMultiple2"; - - // Add multiple gauges - controllerMetrics.setOrUpdateGauge(metricName1, () -> 1L); - controllerMetrics.setOrUpdateGauge(metricName2, () -> 2L); - - // Verify values - Assert.assertEquals(MetricValueUtils.getGaugeValue(controllerMetrics, metricName1), 1); - Assert.assertEquals(MetricValueUtils.getGaugeValue(controllerMetrics, metricName2), 2); - - // Remove gauges - controllerMetrics.removeGauge(metricName1); - controllerMetrics.removeGauge(metricName2); - Assert.assertTrue(controllerMetrics.getMetricsRegistry().allMetrics().isEmpty()); - } + ControllerMetrics controllerMetrics = buildTestMetrics(); - /** - * Creates and initializes a concrete instance of {@link AbstractMetrics} (in this case, a {@code ControllerMetrics}). - * @return a {@code ControllerMetrics} suitable for testing {@code AbstractMetrics} APIs - */ - private static ControllerMetrics buildTestMetrics() { - PinotConfiguration pinotConfiguration = new PinotConfiguration(); - pinotConfiguration.setProperty(CONFIG_OF_METRICS_FACTORY_CLASS_NAME, - "org.apache.pinot.plugin.metrics.yammer.YammerMetricsFactory"); - PinotMetricUtils.init(pinotConfiguration); - return new ControllerMetrics(new YammerMetricsRegistry()); + controllerMetrics.setOrUpdateGauge("testMultiple1", () -> 1L); + controllerMetrics.setOrUpdateGauge("testMultiple2", () -> 2L); + + Assert.assertEquals(getGaugeValue(controllerMetrics, "testMultiple1"), 1); + Assert.assertEquals(getGaugeValue(controllerMetrics, "testMultiple2"), 2); + + controllerMetrics.removeGauge("testMultiple1"); + controllerMetrics.removeGauge("testMultiple2"); + Assert.assertTrue(controllerMetrics.getMetricsRegistry().allMetrics().isEmpty()); } - /** - * Tests the {@link AbstractMetrics} APIs relating to query phases - */ @Test public void testQueryPhases() { - final ControllerMetrics testMetrics = buildTestMetrics(); - final MetricsInspector inspector = new MetricsInspector(testMetrics.getMetricsRegistry()); + ControllerMetrics testMetrics = buildTestMetrics(); + MetricsInspector inspector = createInspector(testMetrics.getMetricsRegistry()); - // Establish dummy values to be used in the test - final AbstractMetrics.QueryPhase testPhase = () -> "testPhase"; + AbstractMetrics.QueryPhase testPhase = () -> "testPhase"; Assert.assertEquals(testPhase.getDescription(), ""); Assert.assertEquals(testPhase.getQueryPhaseName(), "testPhase"); - final String testTableName = "tbl_testQueryPhases"; - final String testTableName2 = "tbl2_testQueryPhases"; + String testTableName = "tbl_testQueryPhases"; + String testTableName2 = "tbl2_testQueryPhases"; - // Add a phase timing, check for correctness testMetrics.addPhaseTiming(testTableName, testPhase, 1, TimeUnit.SECONDS); - final MetricName tbl1Metric = inspector.lastMetric(); - Assert.assertEquals(inspector.getTimer(tbl1Metric).sum(), 1000); + PinotMetricName tbl1Metric = inspector.lastMetric(); + Assert.assertEquals(inspector.getTimerSumMs(tbl1Metric), 1000); - // Add to the existing timer, using different API testMetrics.addPhaseTiming(testTableName, testPhase, 444000000 /* nanoseconds */); - Assert.assertEquals(inspector.getTimer(tbl1Metric).sum(), 1444); + Assert.assertEquals(inspector.getTimerSumMs(tbl1Metric), 1444); - // Add phase timing to a different table. Verify new timer is set up correctly, old timer is not affected testMetrics.addPhaseTiming(testTableName2, testPhase, 22, TimeUnit.MILLISECONDS); - final MetricName tbl2Metric = inspector.lastMetric(); - Assert.assertEquals(inspector.getTimer(tbl2Metric).sum(), 22); - Assert.assertEquals(inspector.getTimer(tbl1Metric).sum(), 1444); + PinotMetricName tbl2Metric = inspector.lastMetric(); + Assert.assertEquals(inspector.getTimerSumMs(tbl2Metric), 22); + Assert.assertEquals(inspector.getTimerSumMs(tbl1Metric), 1444); - // Remove both timers. Verify the metrics registry is now empty testMetrics.removePhaseTiming(testTableName, testPhase); testMetrics.removePhaseTiming(testTableName2, testPhase); Assert.assertTrue(testMetrics.getMetricsRegistry().allMetrics().isEmpty()); } - /** - * Tests the {@link AbstractMetrics} APIs relating to timer metrics - */ @Test public void testTimerMetrics() { ControllerMetrics testMetrics = buildTestMetrics(); - MetricsInspector inspector = new MetricsInspector(testMetrics.getMetricsRegistry()); + MetricsInspector inspector = createInspector(testMetrics.getMetricsRegistry()); String tableName = "tbl_testTimerMetrics"; String keyName = "keyName"; ControllerTimer timer = ControllerTimer.IDEAL_STATE_UPDATE_TIME_MS; - // Test timed table APIs testMetrics.addTimedTableValue(tableName, timer, 6, TimeUnit.SECONDS); - final MetricName t1Metric = inspector.lastMetric(); - Assert.assertEquals(inspector.getTimer(t1Metric).sum(), 6000); + PinotMetricName t1Metric = inspector.lastMetric(); + Assert.assertEquals(inspector.getTimerSumMs(t1Metric), 6000); testMetrics.addTimedTableValue(tableName, keyName, timer, 500, TimeUnit.MILLISECONDS); - final MetricName t2Metric = inspector.lastMetric(); - Assert.assertEquals(inspector.getTimer(t2Metric).sum(), 500); + PinotMetricName t2Metric = inspector.lastMetric(); + Assert.assertEquals(inspector.getTimerSumMs(t2Metric), 500); - // Test timed value APIs testMetrics.addTimedValue(timer, 40, TimeUnit.MILLISECONDS); - final MetricName t3Metric = inspector.lastMetric(); - Assert.assertEquals(inspector.getTimer(t3Metric).sum(), 40); + PinotMetricName t3Metric = inspector.lastMetric(); + Assert.assertEquals(inspector.getTimerSumMs(t3Metric), 40); testMetrics.addTimedValue(keyName, timer, 3, TimeUnit.MILLISECONDS); - final MetricName t4Metric = inspector.lastMetric(); - Assert.assertEquals(inspector.getTimer(t4Metric).sum(), 3); + PinotMetricName t4Metric = inspector.lastMetric(); + Assert.assertEquals(inspector.getTimerSumMs(t4Metric), 3); - // Remove added timers and verify the metrics registry is now empty Assert.assertEquals(testMetrics.getMetricsRegistry().allMetrics().size(), 4); - testMetrics.removeTableTimer(tableName, timer); - testMetrics.removeTimer(t2Metric.getName()); - testMetrics.removeTimer(t3Metric.getName()); - testMetrics.removeTimer(t4Metric.getName()); + testMetrics.getMetricsRegistry().allMetrics().keySet().forEach(testMetrics.getMetricsRegistry()::removeMetric); Assert.assertTrue(testMetrics.getMetricsRegistry().allMetrics().isEmpty()); } - /** - * Tests the {@link AbstractMetrics} APIs relating to metered metrics - */ @Test public void testMeteredMetrics() { - final ControllerMetrics testMetrics = buildTestMetrics(); - final MetricsInspector inspector = new MetricsInspector(testMetrics.getMetricsRegistry()); - final String tableName = "tbl_testMeteredMetrics"; - final String keyName = "keyName"; - final ControllerMeter meter = ControllerMeter.CONTROLLER_INSTANCE_POST_ERROR; - final ControllerMeter meter2 = ControllerMeter.CONTROLLER_PERIODIC_TASK_ERROR; - - // Holder for the most recently seen Metric - final MetricName[] currentMetric = new MetricName[1]; - - // When a new metric is expected we'll call this lambda to assert the metric was created, and update currentMetric - final Runnable expectNewMetric = () -> { + ControllerMetrics testMetrics = buildTestMetrics(); + MetricsInspector inspector = createInspector(testMetrics.getMetricsRegistry()); + String tableName = "tbl_testMeteredMetrics"; + String keyName = "keyName"; + ControllerMeter meter = ControllerMeter.CONTROLLER_INSTANCE_POST_ERROR; + ControllerMeter meter2 = ControllerMeter.CONTROLLER_PERIODIC_TASK_ERROR; + + PinotMetricName[] currentMetric = new PinotMetricName[1]; + Runnable expectNewMetric = () -> { Assert.assertNotEquals(inspector.lastMetric(), currentMetric[0]); currentMetric[0] = inspector.lastMetric(); }; - - // Lambda to verify that the latest metric has the expected value, and its creation was expected - final IntConsumer expectMeteredCount = expected -> { - Assert.assertEquals(inspector.getMetered(currentMetric[0]).count(), expected); + IntConsumer expectMeteredCount = expected -> { + Assert.assertEquals(inspector.getMeteredCount(currentMetric[0]), expected); Assert.assertEquals(currentMetric[0], inspector.lastMetric()); }; - // Test global meter APIs testMetrics.addMeteredGlobalValue(meter, 5); expectNewMetric.run(); expectMeteredCount.accept(5); testMetrics.addMeteredGlobalValue(meter, 4, testMetrics.getMeteredValue(meter)); expectMeteredCount.accept(9); - // Test meter with key APIs testMetrics.addMeteredValue(keyName, meter, 9); expectNewMetric.run(); expectMeteredCount.accept(9); @@ -248,14 +222,12 @@ public void testMeteredMetrics() { testMetrics.addMeteredValue(keyName, meter2, 6, reusedMeter); expectMeteredCount.accept(19); - // Test table-level meter APIs testMetrics.addMeteredTableValue(tableName, meter, 15); expectNewMetric.run(); expectMeteredCount.accept(15); testMetrics.addMeteredTableValue(tableName, meter2, 3, testMetrics.getMeteredTableValue(tableName, meter)); expectMeteredCount.accept(18); - // Test table-level meter with additional key APIs testMetrics.addMeteredTableValue(tableName, keyName, meter, 21); expectNewMetric.run(); expectMeteredCount.accept(21); @@ -265,64 +237,50 @@ public void testMeteredMetrics() { testMetrics.addMeteredTableValue(tableName, keyName, meter2, 5, reusedMeter); expectMeteredCount.accept(28); - // Test removal APIs Assert.assertEquals(testMetrics.getMetricsRegistry().allMetrics().size(), 6); - // This is the only AbstractMetrics method for removing Meter-type metrics. Should others be added? testMetrics.removeTableMeter(tableName, meter); Assert.assertEquals(testMetrics.getMetricsRegistry().allMetrics().size(), 5); - // If we do add other cleanup APIs to AbstractMetrics, they should be tested here. For now, clean the remaining - // metrics with generic APIs. testMetrics.getMetricsRegistry().allMetrics().keySet().forEach(testMetrics.getMetricsRegistry()::removeMetric); Assert.assertTrue(testMetrics.getMetricsRegistry().allMetrics().isEmpty()); } - // tests the add and remove operations on metrics concurrently while running add action longer than remove action. - public void testAsyncAddRemove(Runnable addAction, Runnable removeAction) throws ExecutionException, - InterruptedException { + private void testAsyncAddRemove(Runnable addAction, Runnable removeAction) + throws ExecutionException, InterruptedException { CountDownLatch latch = new CountDownLatch(1); - long gaugeOperationsRuntimeMs = 10; + long runtimeMs = 10; + long endTime = System.currentTimeMillis() + runtimeMs; ExecutorService executorService = Executors.newFixedThreadPool(2); - long endTime = System.currentTimeMillis() + gaugeOperationsRuntimeMs; - Future addFuture = executorService.submit(() -> { - // run addAction parallely with removeAction - while (System.currentTimeMillis() < endTime + gaugeOperationsRuntimeMs) { + while (System.currentTimeMillis() < endTime + runtimeMs) { addAction.run(); } - // wait for removeAction to stop try { - latch.await(); + latch.await(); } catch (InterruptedException e) { - throw new RuntimeException(e); + throw new RuntimeException(e); } - // run addAction after removeAction stops addAction.run(); }); - Future removeFuture = executorService.submit(() -> { - // run removeAction for a shorter duration while (System.currentTimeMillis() < endTime) { removeAction.run(); } - // signal removeAction is done latch.countDown(); }); addFuture.get(); removeFuture.get(); - executorService.shutdown(); - boolean terminated = executorService.awaitTermination(1, TimeUnit.SECONDS); - Assert.assertTrue(terminated, "Tasks should complete and executor should shut down after 1 seconds."); + Assert.assertTrue(executorService.awaitTermination(1, TimeUnit.SECONDS)); } @Test public void testGlobalGaugeMetricsAsyncAddRemove() throws ExecutionException, InterruptedException { ControllerMetrics controllerMetrics = buildTestMetrics(); testAsyncAddRemove( - () -> controllerMetrics.addValueToGlobalGauge(ControllerGauge.VERSION, 1L), - () -> controllerMetrics.removeGauge(ControllerGauge.VERSION.getGaugeName())); + () -> controllerMetrics.addValueToGlobalGauge(ControllerGauge.VERSION, 1L), + () -> controllerMetrics.removeGauge(ControllerGauge.VERSION.getGaugeName())); Assert.assertFalse(controllerMetrics.getMetricsRegistry().allMetrics().isEmpty()); Long gaugeValue = controllerMetrics.getGaugeValue(ControllerGauge.VERSION.getGaugeName()); @@ -333,10 +291,9 @@ public void testGlobalGaugeMetricsAsyncAddRemove() throws ExecutionException, In @Test public void testTableGaugeMetricsAsyncAddRemove() throws ExecutionException, InterruptedException { ControllerMetrics controllerMetrics = buildTestMetrics(); - testAsyncAddRemove( - () -> controllerMetrics.addValueToTableGauge("test_table", ControllerGauge.VERSION, 1L), - () -> controllerMetrics.removeTableGauge("test_table", ControllerGauge.VERSION)); + () -> controllerMetrics.addValueToTableGauge("test_table", ControllerGauge.VERSION, 1L), + () -> controllerMetrics.removeTableGauge("test_table", ControllerGauge.VERSION)); Assert.assertFalse(controllerMetrics.getMetricsRegistry().allMetrics().isEmpty()); Long gaugeValue = controllerMetrics.getGaugeValue(ControllerGauge.VERSION.getGaugeName() + ".test_table"); @@ -347,11 +304,9 @@ public void testTableGaugeMetricsAsyncAddRemove() throws ExecutionException, Int @Test public void testSetValueOfGaugeAsyncAddRemove() throws ExecutionException, InterruptedException { ControllerMetrics controllerMetrics = buildTestMetrics(); - testAsyncAddRemove( - () -> controllerMetrics.setValueOfGauge(1L, ControllerGauge.VERSION.getGaugeName()), - () -> controllerMetrics.removeGauge(ControllerGauge.VERSION.getGaugeName()) - ); + () -> controllerMetrics.setValueOfGauge(1L, ControllerGauge.VERSION.getGaugeName()), + () -> controllerMetrics.removeGauge(ControllerGauge.VERSION.getGaugeName())); Assert.assertFalse(controllerMetrics.getMetricsRegistry().allMetrics().isEmpty()); Long gaugeValue = controllerMetrics.getGaugeValue(ControllerGauge.VERSION.getGaugeName()); @@ -366,17 +321,14 @@ public void testInitializeGlobalMeters() { controllerMetrics.initializeGlobalMeters(); Assert.assertFalse(controllerMetrics.getMetricsRegistry().allMetrics().isEmpty()); - // test that all global meters are initialized to 0 for (ControllerMeter meter : controllerMetrics.getMeters()) { if (meter.isGlobal()) { Assert.assertEquals(0, controllerMetrics.getMeteredValue(meter).count()); } } - - // test that all global gauges are initialized to 0 for (ControllerGauge gauge : controllerMetrics.getGauges()) { if (gauge.isGlobal()) { - Assert.assertEquals(0, controllerMetrics.getGaugeValue(gauge.getGaugeName())); + Assert.assertEquals(0, (long) controllerMetrics.getGaugeValue(gauge.getGaugeName())); } } } @@ -384,26 +336,23 @@ public void testInitializeGlobalMeters() { @Test public void testSetOrUpdateGlobalGauges() { ControllerMetrics controllerMetrics = buildTestMetrics(); - MetricsInspector inspector = new MetricsInspector(controllerMetrics.getMetricsRegistry()); controllerMetrics.setOrUpdateGlobalGauge(ControllerGauge.VERSION, () -> 1L); - Assert.assertEquals(MetricValueUtils.getGaugeValue(controllerMetrics, ControllerGauge.VERSION.getGaugeName()), 1); + Assert.assertEquals(getGaugeValue(controllerMetrics, ControllerGauge.VERSION.getGaugeName()), 1); controllerMetrics.setOrUpdateGlobalGauge(ControllerGauge.VERSION, (Supplier) () -> 2L); - Assert.assertEquals(MetricValueUtils.getGaugeValue(controllerMetrics, ControllerGauge.VERSION.getGaugeName()), 2); + Assert.assertEquals(getGaugeValue(controllerMetrics, ControllerGauge.VERSION.getGaugeName()), 2); controllerMetrics.setValueOfGlobalGauge(ControllerGauge.OFFLINE_TABLE_COUNT, "suffix", 3L); - Assert.assertEquals(MetricValueUtils.getGaugeValue(controllerMetrics, - ControllerGauge.OFFLINE_TABLE_COUNT.getGaugeName() + ".suffix"), 3); + Assert.assertEquals( + getGaugeValue(controllerMetrics, ControllerGauge.OFFLINE_TABLE_COUNT.getGaugeName() + ".suffix"), 3); controllerMetrics.setValueOfGlobalGauge(ControllerGauge.OFFLINE_TABLE_COUNT, 4L); - Assert.assertEquals(MetricValueUtils.getGaugeValue(controllerMetrics, - ControllerGauge.OFFLINE_TABLE_COUNT.getGaugeName()), 4); + Assert.assertEquals(getGaugeValue(controllerMetrics, ControllerGauge.OFFLINE_TABLE_COUNT.getGaugeName()), 4); controllerMetrics.removeGauge(ControllerGauge.VERSION.getGaugeName()); controllerMetrics.removeGauge(ControllerGauge.OFFLINE_TABLE_COUNT.getGaugeName()); controllerMetrics.removeGlobalGauge("suffix", ControllerGauge.OFFLINE_TABLE_COUNT); - Assert.assertTrue(controllerMetrics.getMetricsRegistry().allMetrics().isEmpty()); } @@ -414,24 +363,24 @@ public void testSetOrUpdateTableGauges() { String key = "key"; controllerMetrics.setOrUpdateTableGauge(table, key, ControllerGauge.VERSION, () -> 1L); - Assert.assertEquals(MetricValueUtils.getGaugeValue(controllerMetrics, - ControllerGauge.VERSION.getGaugeName() + "." + table + "." + key), 1); + Assert.assertEquals( + getGaugeValue(controllerMetrics, ControllerGauge.VERSION.getGaugeName() + "." + table + "." + key), 1); controllerMetrics.setOrUpdateTableGauge(table, key, ControllerGauge.VERSION, 2L); - Assert.assertEquals(MetricValueUtils.getGaugeValue(controllerMetrics, - ControllerGauge.VERSION.getGaugeName() + "." + table + "." + key), 2); + Assert.assertEquals( + getGaugeValue(controllerMetrics, ControllerGauge.VERSION.getGaugeName() + "." + table + "." + key), 2); controllerMetrics.setOrUpdateTableGauge(table, ControllerGauge.OFFLINE_TABLE_COUNT, 3L); - Assert.assertEquals(MetricValueUtils.getGaugeValue(controllerMetrics, - ControllerGauge.OFFLINE_TABLE_COUNT.getGaugeName() + "." + table), 3); + Assert.assertEquals( + getGaugeValue(controllerMetrics, ControllerGauge.OFFLINE_TABLE_COUNT.getGaugeName() + "." + table), 3); controllerMetrics.setOrUpdateTableGauge(table, ControllerGauge.OFFLINE_TABLE_COUNT, () -> 4L); - Assert.assertEquals(MetricValueUtils.getGaugeValue(controllerMetrics, - ControllerGauge.OFFLINE_TABLE_COUNT.getGaugeName() + "." + table), 4); + Assert.assertEquals( + getGaugeValue(controllerMetrics, ControllerGauge.OFFLINE_TABLE_COUNT.getGaugeName() + "." + table), 4); controllerMetrics.setValueOfTableGauge(table, ControllerGauge.OFFLINE_TABLE_COUNT, 5L); - Assert.assertEquals(MetricValueUtils.getGaugeValue(controllerMetrics, - ControllerGauge.OFFLINE_TABLE_COUNT.getGaugeName() + "." + table), 5); + Assert.assertEquals( + getGaugeValue(controllerMetrics, ControllerGauge.OFFLINE_TABLE_COUNT.getGaugeName() + "." + table), 5); controllerMetrics.removeTableGauge(table, key, ControllerGauge.VERSION); controllerMetrics.removeTableGauge(table, ControllerGauge.OFFLINE_TABLE_COUNT); @@ -445,11 +394,13 @@ public void testPartitionGauges() { int partitionId = 1024; controllerMetrics.setValueOfPartitionGauge(table, partitionId, ControllerGauge.VERSION, 1L); - Assert.assertEquals(MetricValueUtils.getGaugeValue(controllerMetrics, + Assert.assertEquals( + getGaugeValue(controllerMetrics, ControllerGauge.VERSION.getGaugeName() + "." + table + "." + partitionId), 1); controllerMetrics.setOrUpdatePartitionGauge(table, partitionId, ControllerGauge.VERSION, () -> 2L); - Assert.assertEquals(MetricValueUtils.getGaugeValue(controllerMetrics, + Assert.assertEquals( + getGaugeValue(controllerMetrics, ControllerGauge.VERSION.getGaugeName() + "." + table + "." + partitionId), 2); controllerMetrics.removePartitionGauge(table, partitionId, ControllerGauge.VERSION); @@ -462,7 +413,7 @@ public void testAddCallbackGauges() { String table = "test_table"; controllerMetrics.addCallbackTableGaugeIfNeeded(table, ControllerGauge.VERSION, () -> 10L); - Assert.assertEquals(MetricValueUtils.getGaugeValue(controllerMetrics, - ControllerGauge.VERSION.getGaugeName() + "." + table), 10); + Assert.assertEquals( + getGaugeValue(controllerMetrics, ControllerGauge.VERSION.getGaugeName() + "." + table), 10); } } diff --git a/pinot-common/src/test/java/org/apache/pinot/common/metrics/MetricValueUtils.java b/pinot-common/src/test/java/org/apache/pinot/common/metrics/MetricValueUtils.java index 33cdf4821b76..0cfeeb3c9472 100644 --- a/pinot-common/src/test/java/org/apache/pinot/common/metrics/MetricValueUtils.java +++ b/pinot-common/src/test/java/org/apache/pinot/common/metrics/MetricValueUtils.java @@ -18,106 +18,105 @@ */ package org.apache.pinot.common.metrics; -import com.yammer.metrics.core.MetricName; -import org.apache.pinot.plugin.metrics.yammer.YammerMetricName; -import org.apache.pinot.plugin.metrics.yammer.YammerSettableGauge; import org.apache.pinot.spi.metrics.PinotMetric; +import org.apache.pinot.spi.metrics.PinotMetricName; +import org.apache.pinot.spi.metrics.PinotMetricUtils; +import org.apache.pinot.spi.metrics.SettableValue; +/** + * Test utility for reading gauge values from {@link AbstractMetrics} without depending on a specific metrics + * implementation. Reads via {@link AbstractMetrics#getGaugeValue(String)} first (covers gauges registered via the + * {@code setValueOf*Gauge}/{@code addValueTo*Gauge} family that populate the internal value map), and falls back to + * looking the metric up in the registry and reading it via the {@link SettableValue} SPI (covers pure-supplier paths + * like {@code setOrUpdateGauge(Supplier)}, {@code setOrUpdateGauge(long)}, and {@code addCallbackGauge}). + */ +@SuppressWarnings({"rawtypes", "unchecked"}) public class MetricValueUtils { private MetricValueUtils() { } public static boolean gaugeExists(AbstractMetrics metrics, String metricName) { - return extractMetric(metrics, metricName) != null; + return readGauge(metrics, metricName) != null; } public static long getGaugeValue(AbstractMetrics metrics, String metricName) { - PinotMetric pinotMetric = extractMetric(metrics, metricName); - if (pinotMetric == null) { - return 0; - } - return ((YammerSettableGauge) pinotMetric.getMetric()).value(); + Long value = readGauge(metrics, metricName); + return value != null ? value : 0L; } public static boolean globalGaugeExists(AbstractMetrics metrics, AbstractMetrics.Gauge gauge) { - return extractMetric(metrics, gauge.getGaugeName()) != null; + return readGauge(metrics, gauge.getGaugeName()) != null; } public static long getGlobalGaugeValue(AbstractMetrics metrics, AbstractMetrics.Gauge gauge) { - PinotMetric pinotMetric = extractMetric(metrics, gauge.getGaugeName()); - if (pinotMetric == null) { - return 0; - } - return ((YammerSettableGauge) pinotMetric.getMetric()).value(); + Long value = readGauge(metrics, gauge.getGaugeName()); + return value != null ? value : 0L; } public static boolean globalGaugeExists(AbstractMetrics metrics, String key, AbstractMetrics.Gauge gauge) { - return extractMetric(metrics, gauge.getGaugeName() + "." + key) != null; + return readGauge(metrics, gauge.getGaugeName() + "." + key) != null; } public static long getGlobalGaugeValue(AbstractMetrics metrics, String key, AbstractMetrics.Gauge gauge) { - PinotMetric pinotMetric = extractMetric(metrics, gauge.getGaugeName() + "." + key); - if (pinotMetric == null) { - return 0; - } - return ((YammerSettableGauge) pinotMetric.getMetric()).value(); + Long value = readGauge(metrics, gauge.getGaugeName() + "." + key); + return value != null ? value : 0L; } public static boolean tableGaugeExists(AbstractMetrics metrics, String tableName, AbstractMetrics.Gauge gauge) { - return extractMetric(metrics, gauge.getGaugeName() + "." + tableName) != null; + return readGauge(metrics, gauge.getGaugeName() + "." + tableName) != null; } public static long getTableGaugeValue(AbstractMetrics metrics, String tableName, AbstractMetrics.Gauge gauge) { - PinotMetric pinotMetric = extractMetric(metrics, gauge.getGaugeName() + "." + tableName); - if (pinotMetric == null) { - return 0; - } - return ((YammerSettableGauge) pinotMetric.getMetric()).value(); + Long value = readGauge(metrics, gauge.getGaugeName() + "." + tableName); + return value != null ? value : 0L; } public static boolean tableGaugeExists(AbstractMetrics metrics, String tableName, String key, AbstractMetrics.Gauge gauge) { - return extractMetric(metrics, gauge.getGaugeName() + "." + tableName + "." + key) != null; + return readGauge(metrics, gauge.getGaugeName() + "." + tableName + "." + key) != null; } public static long getTableGaugeValue(AbstractMetrics metrics, String tableName, String key, AbstractMetrics.Gauge gauge) { - PinotMetric pinotMetric = extractMetric(metrics, gauge.getGaugeName() + "." + tableName + "." + key); - if (pinotMetric == null) { - return 0; - } - return ((YammerSettableGauge) pinotMetric.getMetric()).value(); + Long value = readGauge(metrics, gauge.getGaugeName() + "." + tableName + "." + key); + return value != null ? value : 0L; } public static boolean partitionGaugeExists(AbstractMetrics metrics, String tableName, int partitionId, AbstractMetrics.Gauge gauge) { - return extractMetric(metrics, gauge.getGaugeName() + "." + tableName + "." + partitionId) != null; + return readGauge(metrics, gauge.getGaugeName() + "." + tableName + "." + partitionId) != null; } public static long getPartitionGaugeValue(AbstractMetrics metrics, String tableName, int partitionId, AbstractMetrics.Gauge gauge) { - PinotMetric pinotMetric = extractMetric(metrics, gauge.getGaugeName() + "." + tableName + "." + partitionId); - if (pinotMetric == null) { - return 0; - } - return ((YammerSettableGauge) pinotMetric.getMetric()).value(); + Long value = readGauge(metrics, gauge.getGaugeName() + "." + tableName + "." + partitionId); + return value != null ? value : 0L; } - private static PinotMetric extractMetric(AbstractMetrics metrics, String metricName) { - String metricPrefix; - if (metrics instanceof ControllerMetrics) { - metricPrefix = "pinot.controller."; - } else if (metrics instanceof BrokerMetrics) { - metricPrefix = "pinot.broker."; - } else if (metrics instanceof ServerMetrics) { - metricPrefix = "pinot.server."; - } else if (metrics instanceof MinionMetrics) { - metricPrefix = "pinot.minion."; - } else { - throw new RuntimeException("unsupported AbstractMetrics type: " + metrics.getClass().toString()); + private static Long readGauge(AbstractMetrics metrics, String metricName) { + // Fast path: gauges registered via setValueOf*/addValueTo* populate _gaugeValues directly. + Long direct = metrics.getGaugeValue(metricName); + if (direct != null) { + return direct; + } + // Fallback: gauges registered via pure-supplier paths (setOrUpdateGauge(Supplier), setOrUpdateGauge(long), + // addCallbackGauge) are only in the registry. Look them up and read via the SettableValue SPI, which is + // implemented by YammerSettableGauge and DropwizardSettableGauge. + PinotMetricName name = + PinotMetricUtils.makePinotMetricName(metrics.getClass(), metrics.getMetricPrefix() + metricName); + PinotMetric pinotMetric = metrics.getMetricsRegistry().allMetrics().get(name); + if (pinotMetric == null) { + return null; + } + Object inner = pinotMetric.getMetric(); + if (!(inner instanceof SettableValue)) { + return null; + } + Object value = ((SettableValue) inner).getValue(); + if (value instanceof Number) { + return ((Number) value).longValue(); } - return metrics.getMetricsRegistry().allMetrics() - .get(new YammerMetricName(new MetricName(metrics.getClass(), metricPrefix + metricName))); + return null; } } diff --git a/pinot-common/src/test/java/org/apache/pinot/common/metrics/MetricsInspector.java b/pinot-common/src/test/java/org/apache/pinot/common/metrics/MetricsInspector.java index 1dc1b8318599..b8832024bd74 100644 --- a/pinot-common/src/test/java/org/apache/pinot/common/metrics/MetricsInspector.java +++ b/pinot-common/src/test/java/org/apache/pinot/common/metrics/MetricsInspector.java @@ -18,153 +18,41 @@ */ package org.apache.pinot.common.metrics; -import com.yammer.metrics.core.Counter; -import com.yammer.metrics.core.Gauge; -import com.yammer.metrics.core.Histogram; -import com.yammer.metrics.core.Metered; -import com.yammer.metrics.core.Metric; -import com.yammer.metrics.core.MetricName; -import com.yammer.metrics.core.MetricProcessor; -import com.yammer.metrics.core.MetricsRegistryListener; -import com.yammer.metrics.core.Timer; -import java.util.HashMap; -import java.util.Map; -import java.util.function.Function; -import org.apache.pinot.spi.metrics.PinotMetricsRegistry; +import org.apache.pinot.spi.metrics.PinotMetricName; /** - * A helper for accessing metric values, agnostic of the concrete instance type of the metric. General usage is as - * follows:
- *
    - *
  • - * Construct a {@code MetricsInspector} from a {@code PinotMetricsRegistry}:
    - * 
    final MetricsInspector inspector = new MetricsInspector(pinotMetricsRegistry);
    - *
    • - *
  • - * Every time a metric is added to the registry this MetricsInspector will record it. The most recently added - * metric can be accessed with:
    - * 
    final MetricName metricName = inspector.lastMetric();
    - *
    • - *
  • - * Use the {@code MetricName} returned by {@link #_lastMetric} to query the properties of the corresponding - * metric, for example: - * 
    inspector.getTimer(metricName)
    - * It's the caller's responsibility to know the type of the metric (Timer, Meter, etc.) and call the appropriate - * getter. - *
    • - *
+ * A helper for accessing metric values from tests in an implementation-agnostic way. Concrete subclasses are provided + * by each metrics plugin (yammer, dropwizard) and by the in-memory fake registry used by pinot-common's own tests. * + *

General usage: + *

    + *
  1. Build an inspector around a {@code PinotMetricsRegistry}. The inspector wires itself as a listener so it can + * observe metric registrations as they happen.
    2. + *
  2. After invoking an {@code AbstractMetrics} mutator that registers a new metric, call {@link #lastMetric()} to + * retrieve the {@link PinotMetricName} of the most recently added metric.
    3. + *
  3. Use {@link #getTimerSumMs(PinotMetricName)} / {@link #getMeteredCount(PinotMetricName)} to read values without + * knowing the concrete metric type.
    4. + *
*/ -public class MetricsInspector { - private final Map _metricMap = new HashMap<>(); - private MetricName _lastMetric; - - public MetricsInspector(PinotMetricsRegistry registry) { - - /* We detect newly added metrics by adding a listener to the metrics registry. Callers typically don't have - direct references to the metrics they create because factory methods usually create the metric and add it to the - registry without returning it. Since there is no easy way to look up the created metric afterward, the listener - approach provides callers with a way to access the metrics they've just created. - */ - registry.addListener(() -> new MetricsRegistryListener() { - @Override - public void onMetricAdded(MetricName metricName, Metric metric) { - _metricMap.put(metricName, metric); - _lastMetric = metricName; - } - @Override - public void onMetricRemoved(MetricName metricName) { - _metricMap.remove(metricName); - } - }); - } +public abstract class MetricsInspector { /** - * @return the {@code MetricName} of the last metric that was added to the registry + * @return the {@code PinotMetricName} of the last metric that was added to the registry. */ - public MetricName lastMetric() { - return _lastMetric; - } + public abstract PinotMetricName lastMetric(); /** - * Extracts the {@code Timer} from a {@code Timer} metric. + * Total elapsed time recorded on the timer, normalized to milliseconds. * - * @param metric a {@code MetricName} returned by a previous call to {@link #_lastMetric} - * @return the {@code Timer} from the associated metric. - * @throws IllegalArgumentException if the provided {@code MetricName} is not associated with a {@code Timer} metric + * @throws IllegalArgumentException if the provided name does not correspond to a timer */ - public Timer getTimer(MetricName metric) { - return access(metric, m -> m._timer); - } + public abstract long getTimerSumMs(PinotMetricName name); /** - * Extracts the {@code Metered} from a {@code Metered} metric. + * Total count of events recorded on the metered metric (meter or timer). * - * @param metric a {@code MetricName} returned by a previous call to {@link #_lastMetric} - * @return the {@code Metered} from the associated metric. - * @throws IllegalArgumentException if the provided {@code MetricName} is not associated with a {@code Metered} metric + * @throws IllegalArgumentException if the provided name does not correspond to a metered metric */ - - public Metered getMetered(MetricName metric) { - return access(metric, m -> m._metered); - } - - private T access(MetricName metricName, Function property) { - Metric metric = _metricMap.get(metricName); - if (metric == null) { - throw new IllegalArgumentException("Metric not found: " + metricName); - } - - MetricAccessor accessor = new MetricAccessor(); - try { - metric.processWith(accessor, null, null); - } catch (Exception e) { - // Convert checked exception (from processWith API) to unchecked because our MetricProcessor doesn't throw - throw new IllegalStateException("Unexpected error processing metric: " + metric, e); - } - - T result = property.apply(accessor); - if (result == null) { - throw new IllegalArgumentException("Requested metric type not found in metric [" + metricName.getName() + "]"); - } - return result; - } - - /** - * A MetricProcessor that simply captures the internals of a {@code Metric}. For internal use only.
- */ - private static class MetricAccessor implements MetricProcessor { - - public Metered _metered; - public Counter _counter; - public Histogram _histogram; - public Timer _timer; - public Gauge _gauge; - - @Override - public void processMeter(MetricName n, Metered metered, Void v) { - _metered = metered; - } - - @Override - public void processCounter(MetricName n, Counter counter, Void v) { - _counter = counter; - } - - @Override - public void processHistogram(MetricName n, Histogram histogram, Void v) { - _histogram = histogram; - } - - @Override - public void processTimer(MetricName n, Timer timer, Void v) { - _timer = timer; - } - - @Override - public void processGauge(MetricName metricName, Gauge gauge, Void v) { - _gauge = gauge; - } - } + public abstract long getMeteredCount(PinotMetricName name); } diff --git a/pinot-common/src/test/java/org/apache/pinot/common/metrics/PinotMetricUtilsTest.java b/pinot-common/src/test/java/org/apache/pinot/common/metrics/PinotMetricUtilsTest.java index b8429e88acbf..5dd9334e5919 100644 --- a/pinot-common/src/test/java/org/apache/pinot/common/metrics/PinotMetricUtilsTest.java +++ b/pinot-common/src/test/java/org/apache/pinot/common/metrics/PinotMetricUtilsTest.java @@ -21,6 +21,7 @@ import java.util.HashMap; import java.util.Map; import java.util.concurrent.TimeUnit; +import org.apache.pinot.plugin.metrics.fake.FakeMetricsFactory; import org.apache.pinot.spi.env.PinotConfiguration; import org.apache.pinot.spi.metrics.MetricsRegistryRegistrationListener; import org.apache.pinot.spi.metrics.PinotMeter; @@ -28,23 +29,32 @@ import org.apache.pinot.spi.metrics.PinotMetricUtils; import org.apache.pinot.spi.metrics.PinotMetricsRegistry; import org.testng.Assert; +import org.testng.annotations.AfterClass; import org.testng.annotations.Test; +import static org.apache.pinot.spi.utils.CommonConstants.CONFIG_OF_METRICS_FACTORY_CLASS_NAME; + public class PinotMetricUtilsTest { + @AfterClass + public void cleanUpMetricsFactory() { + PinotMetricUtils.cleanUp(); + } + @Test public void testPinotMetricsRegistryFactory() { try { Map properties = new HashMap<>(); + properties.put(CONFIG_OF_METRICS_FACTORY_CLASS_NAME, FakeMetricsFactory.class.getName()); PinotConfiguration configuration = new PinotConfiguration(properties); PinotMetricUtils.init(configuration); } catch (Exception e) { - Assert.fail("Fail to initialize PinotMetricsRegistry of yammer"); + Assert.fail("Fail to initialize PinotMetricsRegistry with the fake factory", e); } PinotMetricsRegistry pinotMetricsRegistry = PinotMetricUtils.getPinotMetricsRegistry(); Assert.assertNotNull(pinotMetricsRegistry); - Assert.assertEquals(pinotMetricsRegistry.getClass().getSimpleName(), "YammerMetricsRegistry"); + Assert.assertEquals(pinotMetricsRegistry.getClass().getSimpleName(), "FakePinotMetricsRegistry"); } public static boolean _listenerOneOkay; @@ -70,23 +80,26 @@ public void testPinotMetricsRegistration() { _listenerTwoOkay = false; Map properties = new HashMap<>(); + properties.put(CONFIG_OF_METRICS_FACTORY_CLASS_NAME, FakeMetricsFactory.class.getName()); properties.put("pinot.broker.metrics.metricsRegistryRegistrationListeners", PinotMetricUtilsTest.ListenerOne.class.getName() + "," + PinotMetricUtilsTest.ListenerTwo.class.getName()); - // Initialize the PinotMetricUtils and create a new timer PinotConfiguration configuration = new PinotConfiguration(properties); PinotMetricUtils.init(configuration.subset("pinot.broker.metrics")); + PinotMetricUtils.init(configuration); PinotMetricsRegistry registry = PinotMetricUtils.getPinotMetricsRegistry(); PinotMetricUtils.makePinotTimer(registry, PinotMetricUtils.makePinotMetricName(PinotMetricUtilsTest.class, "dummy"), TimeUnit.MILLISECONDS, TimeUnit.MILLISECONDS); - // Check that the two listeners fired Assert.assertTrue(_listenerOneOkay); Assert.assertTrue(_listenerTwoOkay); } @Test public void testMetricValue() { + Map properties = new HashMap<>(); + properties.put(CONFIG_OF_METRICS_FACTORY_CLASS_NAME, FakeMetricsFactory.class.getName()); + PinotMetricUtils.init(new PinotConfiguration(properties)); PinotMetricsRegistry registry = PinotMetricUtils.getPinotMetricsRegistry(); PinotMeter pinotMeter = PinotMetricUtils .makePinotMeter(registry, PinotMetricUtils.makePinotMetricName(PinotMetricUtilsTest.class, "testMeter"), @@ -100,6 +113,11 @@ public void testMetricValue() { @Test public void testPinotMetricName() { + // Explicitly install the fake factory so this test doesn't rely on whichever factory a prior test left behind. + Map properties = new HashMap<>(); + properties.put(CONFIG_OF_METRICS_FACTORY_CLASS_NAME, FakeMetricsFactory.class.getName()); + PinotMetricUtils.init(new PinotConfiguration(properties)); + PinotMetricName testMetricName1 = PinotMetricUtils.makePinotMetricName(PinotMetricUtilsTest.class, "testMetricName"); PinotMetricName testMetricName2 = @@ -125,12 +143,18 @@ public void testMetricRegistryFailure() { @Test public void testCleanUp() { + Map properties = new HashMap<>(); + properties.put(CONFIG_OF_METRICS_FACTORY_CLASS_NAME, FakeMetricsFactory.class.getName()); + PinotConfiguration config = new PinotConfiguration(properties); + PinotMetricUtils.cleanUp(); + PinotMetricUtils.init(config); PinotMetricsRegistry registry = PinotMetricUtils.getPinotMetricsRegistry(); PinotMetricsRegistry registry1 = PinotMetricUtils.getPinotMetricsRegistry(); Assert.assertEquals(registry, registry1); PinotMetricUtils.cleanUp(); // after cleaning up, a new one will be created + PinotMetricUtils.init(config); PinotMetricsRegistry registry2 = PinotMetricUtils.getPinotMetricsRegistry(); Assert.assertNotEquals(registry, registry2); } diff --git a/pinot-common/src/test/java/org/apache/pinot/plugin/metrics/fake/FakeMetricsAbstractMetricsTest.java b/pinot-common/src/test/java/org/apache/pinot/plugin/metrics/fake/FakeMetricsAbstractMetricsTest.java new file mode 100644 index 000000000000..f14eecdb4e97 --- /dev/null +++ b/pinot-common/src/test/java/org/apache/pinot/plugin/metrics/fake/FakeMetricsAbstractMetricsTest.java @@ -0,0 +1,66 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.pinot.plugin.metrics.fake; + +import org.apache.pinot.common.metrics.AbstractMetrics; +import org.apache.pinot.common.metrics.AbstractMetricsTest; +import org.apache.pinot.common.metrics.MetricsInspector; +import org.apache.pinot.spi.metrics.PinotGauge; +import org.apache.pinot.spi.metrics.PinotMetric; +import org.apache.pinot.spi.metrics.PinotMetricName; +import org.apache.pinot.spi.metrics.PinotMetricUtils; +import org.apache.pinot.spi.metrics.PinotMetricsRegistry; + + +/** + * Runs the shared {@link AbstractMetricsTest} against the in-memory {@link FakePinotMetricsRegistry}. This covers + * {@code AbstractMetrics} logic in pinot-common without pulling in a real metrics plugin. + */ +public class FakeMetricsAbstractMetricsTest extends AbstractMetricsTest { + + @Override + protected String metricsFactoryClassName() { + return FakeMetricsFactory.class.getName(); + } + + @Override + protected PinotMetricsRegistry buildRegistry() { + return new FakePinotMetricsRegistry(); + } + + @Override + protected MetricsInspector createInspector(PinotMetricsRegistry registry) { + return new FakeMetricsInspector(registry); + } + + @Override + protected long getGaugeValue(AbstractMetrics metrics, String metricName) { + PinotMetricName name = + PinotMetricUtils.makePinotMetricName(metrics.getClass(), metrics.getMetricPrefix() + metricName); + PinotMetric metric = metrics.getMetricsRegistry().allMetrics().get(name); + if (!(metric instanceof PinotGauge)) { + throw new IllegalArgumentException("Not a gauge metric: " + name); + } + Object value = ((PinotGauge) metric).value(); + if (!(value instanceof Number)) { + throw new IllegalStateException("Gauge did not produce a Number: " + name + " -> " + value); + } + return ((Number) value).longValue(); + } +} diff --git a/pinot-common/src/test/java/org/apache/pinot/plugin/metrics/fake/FakeMetricsFactory.java b/pinot-common/src/test/java/org/apache/pinot/plugin/metrics/fake/FakeMetricsFactory.java new file mode 100644 index 000000000000..159b2cf4f16e --- /dev/null +++ b/pinot-common/src/test/java/org/apache/pinot/plugin/metrics/fake/FakeMetricsFactory.java @@ -0,0 +1,72 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.pinot.plugin.metrics.fake; + +import java.util.function.Function; +import org.apache.pinot.spi.annotations.metrics.MetricsFactory; +import org.apache.pinot.spi.annotations.metrics.PinotMetricsFactory; +import org.apache.pinot.spi.env.PinotConfiguration; +import org.apache.pinot.spi.metrics.PinotGauge; +import org.apache.pinot.spi.metrics.PinotJmxReporter; +import org.apache.pinot.spi.metrics.PinotMetricName; +import org.apache.pinot.spi.metrics.PinotMetricsRegistry; + + +/** + * A test-only {@link PinotMetricsFactory} backed by the in-memory {@link FakePinotMetricsRegistry}. Used by + * pinot-common tests so they can exercise {@code AbstractMetrics} without depending on a plugin module. + */ +@MetricsFactory +public class FakeMetricsFactory implements PinotMetricsFactory { + private PinotMetricsRegistry _registry; + + @Override + public void init(PinotConfiguration metricsConfiguration) { + } + + @Override + public PinotMetricsRegistry getPinotMetricsRegistry() { + if (_registry == null) { + _registry = new FakePinotMetricsRegistry(); + } + return _registry; + } + + @Override + public PinotMetricName makePinotMetricName(Class clazz, String name) { + return new FakePinotMetricName(clazz, name); + } + + @Override + public PinotGauge makePinotGauge(Function condition) { + return new FakePinotGauge<>(condition); + } + + @Override + public PinotJmxReporter makePinotJmxReporter(PinotMetricsRegistry metricsRegistry) { + // No JMX for the fake — intentional, to avoid cross-test collisions. + return () -> { + }; + } + + @Override + public String getMetricsFactoryName() { + return "Fake"; + } +} diff --git a/pinot-common/src/test/java/org/apache/pinot/plugin/metrics/fake/FakeMetricsInspector.java b/pinot-common/src/test/java/org/apache/pinot/plugin/metrics/fake/FakeMetricsInspector.java new file mode 100644 index 000000000000..42018de39ee1 --- /dev/null +++ b/pinot-common/src/test/java/org/apache/pinot/plugin/metrics/fake/FakeMetricsInspector.java @@ -0,0 +1,77 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.pinot.plugin.metrics.fake; + +import org.apache.pinot.common.metrics.MetricsInspector; +import org.apache.pinot.spi.metrics.PinotMetric; +import org.apache.pinot.spi.metrics.PinotMetricName; +import org.apache.pinot.spi.metrics.PinotMetricsRegistry; +import org.apache.pinot.spi.metrics.PinotMetricsRegistryListener; + + +public class FakeMetricsInspector extends MetricsInspector { + private final PinotMetricsRegistry _registry; + private volatile PinotMetricName _lastMetric; + + public FakeMetricsInspector(PinotMetricsRegistry registry) { + _registry = registry; + final FakePinotMetricsRegistry.Listener inner = new FakePinotMetricsRegistry.Listener() { + @Override + public void onMetricAdded(PinotMetricName name, PinotMetric metric) { + _lastMetric = name; + } + + @Override + public void onMetricRemoved(PinotMetricName name) { + } + }; + registry.addListener(new PinotMetricsRegistryListener() { + @Override + public Object getMetricsRegistryListener() { + return inner; + } + }); + } + + @Override + public PinotMetricName lastMetric() { + return _lastMetric; + } + + @Override + public long getTimerSumMs(PinotMetricName name) { + PinotMetric metric = _registry.allMetrics().get(name); + if (!(metric instanceof FakePinotTimer)) { + throw new IllegalArgumentException("Not a timer metric: " + name); + } + return ((FakePinotTimer) metric).sumMs(); + } + + @Override + public long getMeteredCount(PinotMetricName name) { + PinotMetric metric = _registry.allMetrics().get(name); + if (metric instanceof FakePinotMeter) { + return ((FakePinotMeter) metric).count(); + } + if (metric instanceof FakePinotTimer) { + return ((FakePinotTimer) metric).count(); + } + throw new IllegalArgumentException("Not a metered metric: " + name); + } +} diff --git a/pinot-common/src/test/java/org/apache/pinot/plugin/metrics/fake/FakePinotCounter.java b/pinot-common/src/test/java/org/apache/pinot/plugin/metrics/fake/FakePinotCounter.java new file mode 100644 index 000000000000..bedf0be92341 --- /dev/null +++ b/pinot-common/src/test/java/org/apache/pinot/plugin/metrics/fake/FakePinotCounter.java @@ -0,0 +1,45 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.pinot.plugin.metrics.fake; + +import java.util.concurrent.atomic.AtomicLong; +import org.apache.pinot.spi.metrics.PinotCounter; + + +public class FakePinotCounter implements PinotCounter { + private final AtomicLong _value = new AtomicLong(); + + @Override + public Object getCounter() { + return this; + } + + @Override + public Object getMetric() { + return this; + } + + public long count() { + return _value.get(); + } + + public void inc(long n) { + _value.addAndGet(n); + } +} diff --git a/pinot-common/src/test/java/org/apache/pinot/plugin/metrics/fake/FakePinotGauge.java b/pinot-common/src/test/java/org/apache/pinot/plugin/metrics/fake/FakePinotGauge.java new file mode 100644 index 000000000000..c1998fff94a9 --- /dev/null +++ b/pinot-common/src/test/java/org/apache/pinot/plugin/metrics/fake/FakePinotGauge.java @@ -0,0 +1,57 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.pinot.plugin.metrics.fake; + +import java.util.function.Function; +import java.util.function.Supplier; +import org.apache.pinot.spi.metrics.PinotGauge; + + +public class FakePinotGauge implements PinotGauge { + private Supplier _valueSupplier; + + public FakePinotGauge(Function condition) { + _valueSupplier = () -> condition.apply(null); + } + + @Override + public T value() { + return _valueSupplier.get(); + } + + @Override + public Object getGauge() { + return this; + } + + @Override + public Object getMetric() { + return this; + } + + @Override + public void setValue(T value) { + _valueSupplier = () -> value; + } + + @Override + public void setValueSupplier(Supplier valueSupplier) { + _valueSupplier = valueSupplier; + } +} diff --git a/pinot-common/src/test/java/org/apache/pinot/plugin/metrics/fake/FakePinotMeter.java b/pinot-common/src/test/java/org/apache/pinot/plugin/metrics/fake/FakePinotMeter.java new file mode 100644 index 000000000000..a4d47e4e317c --- /dev/null +++ b/pinot-common/src/test/java/org/apache/pinot/plugin/metrics/fake/FakePinotMeter.java @@ -0,0 +1,90 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.pinot.plugin.metrics.fake; + +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicLong; +import org.apache.pinot.spi.metrics.PinotMeter; + + +public class FakePinotMeter implements PinotMeter { + private final AtomicLong _count = new AtomicLong(); + private final String _eventType; + private final TimeUnit _rateUnit; + + public FakePinotMeter(String eventType, TimeUnit rateUnit) { + _eventType = eventType; + _rateUnit = rateUnit; + } + + @Override + public void mark() { + _count.incrementAndGet(); + } + + @Override + public void mark(long unitCount) { + _count.addAndGet(unitCount); + } + + @Override + public long count() { + return _count.get(); + } + + @Override + public Object getMetered() { + return this; + } + + @Override + public Object getMetric() { + return this; + } + + @Override + public TimeUnit rateUnit() { + return _rateUnit; + } + + @Override + public String eventType() { + return _eventType; + } + + @Override + public double fifteenMinuteRate() { + return 0; + } + + @Override + public double fiveMinuteRate() { + return 0; + } + + @Override + public double meanRate() { + return 0; + } + + @Override + public double oneMinuteRate() { + return 0; + } +} diff --git a/pinot-common/src/test/java/org/apache/pinot/plugin/metrics/fake/FakePinotMetricName.java b/pinot-common/src/test/java/org/apache/pinot/plugin/metrics/fake/FakePinotMetricName.java new file mode 100644 index 000000000000..d71c2f371b9f --- /dev/null +++ b/pinot-common/src/test/java/org/apache/pinot/plugin/metrics/fake/FakePinotMetricName.java @@ -0,0 +1,57 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.pinot.plugin.metrics.fake; + +import java.util.Objects; +import org.apache.pinot.spi.metrics.PinotMetricName; + + +public class FakePinotMetricName implements PinotMetricName { + private final String _name; + + public FakePinotMetricName(Class clazz, String name) { + _name = clazz.getName() + "." + name; + } + + @Override + public Object getMetricName() { + return _name; + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (!(o instanceof FakePinotMetricName)) { + return false; + } + return Objects.equals(_name, ((FakePinotMetricName) o)._name); + } + + @Override + public int hashCode() { + return Objects.hashCode(_name); + } + + @Override + public String toString() { + return _name; + } +} diff --git a/pinot-common/src/test/java/org/apache/pinot/plugin/metrics/fake/FakePinotMetricsRegistry.java b/pinot-common/src/test/java/org/apache/pinot/plugin/metrics/fake/FakePinotMetricsRegistry.java new file mode 100644 index 000000000000..ac1b8acfeba8 --- /dev/null +++ b/pinot-common/src/test/java/org/apache/pinot/plugin/metrics/fake/FakePinotMetricsRegistry.java @@ -0,0 +1,145 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.pinot.plugin.metrics.fake; + +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.CopyOnWriteArrayList; +import java.util.concurrent.TimeUnit; +import org.apache.pinot.spi.metrics.PinotCounter; +import org.apache.pinot.spi.metrics.PinotGauge; +import org.apache.pinot.spi.metrics.PinotHistogram; +import org.apache.pinot.spi.metrics.PinotMeter; +import org.apache.pinot.spi.metrics.PinotMetric; +import org.apache.pinot.spi.metrics.PinotMetricName; +import org.apache.pinot.spi.metrics.PinotMetricsRegistry; +import org.apache.pinot.spi.metrics.PinotMetricsRegistryListener; +import org.apache.pinot.spi.metrics.PinotTimer; + + +/** + * In-memory, no-JMX {@link PinotMetricsRegistry} for unit-testing {@code AbstractMetrics}-derived classes without + * requiring a real metrics plugin on the classpath. + */ +public class FakePinotMetricsRegistry implements PinotMetricsRegistry { + private final Map _metrics = new ConcurrentHashMap<>(); + private final List _listeners = new CopyOnWriteArrayList<>(); + + @Override + public PinotGauge newGauge(PinotMetricName name, PinotGauge gauge) { + @SuppressWarnings("unchecked") + PinotGauge existing = (PinotGauge) _metrics.get(name); + if (existing != null) { + return existing; + } + _metrics.put(name, gauge); + notifyAdded(name, gauge); + return gauge; + } + + @Override + public PinotMeter newMeter(PinotMetricName name, String eventType, TimeUnit unit) { + return (PinotMeter) _metrics.computeIfAbsent(name, n -> { + FakePinotMeter meter = new FakePinotMeter(eventType, unit); + notifyAdded(n, meter); + return meter; + }); + } + + @Override + public PinotCounter newCounter(PinotMetricName name) { + return (PinotCounter) _metrics.computeIfAbsent(name, n -> { + FakePinotCounter counter = new FakePinotCounter(); + notifyAdded(n, counter); + return counter; + }); + } + + @Override + public PinotTimer newTimer(PinotMetricName name, TimeUnit durationUnit, TimeUnit rateUnit) { + return (PinotTimer) _metrics.computeIfAbsent(name, n -> { + FakePinotTimer timer = new FakePinotTimer(durationUnit, rateUnit); + notifyAdded(n, timer); + return timer; + }); + } + + @Override + public PinotHistogram newHistogram(PinotMetricName name, boolean biased) { + throw new UnsupportedOperationException("FakePinotMetricsRegistry does not implement histograms; " + + "add a FakePinotHistogram if a test needs one"); + } + + @Override + public void removeMetric(PinotMetricName name) { + PinotMetric removed = _metrics.remove(name); + if (removed != null) { + notifyRemoved(name); + } + } + + @Override + public Map allMetrics() { + return Collections.unmodifiableMap(_metrics); + } + + @Override + public void addListener(PinotMetricsRegistryListener listener) { + Object inner = listener.getMetricsRegistryListener(); + if (inner instanceof Listener) { + Listener fakeListener = (Listener) inner; + _listeners.add(fakeListener); + // Replay existing metrics per the PinotMetricsRegistry contract. + for (Map.Entry entry : _metrics.entrySet()) { + fakeListener.onMetricAdded(entry.getKey(), entry.getValue()); + } + } + } + + @Override + public Object getMetricsRegistry() { + return this; + } + + @Override + public void shutdown() { + } + + private void notifyAdded(PinotMetricName name, PinotMetric metric) { + for (Listener listener : _listeners) { + listener.onMetricAdded(name, metric); + } + } + + private void notifyRemoved(PinotMetricName name) { + for (Listener listener : _listeners) { + listener.onMetricRemoved(name); + } + } + + /** + * Callback surface used by {@link PinotMetricsRegistryListener} subclasses that wrap a {@link Listener}. + */ + public interface Listener { + void onMetricAdded(PinotMetricName name, PinotMetric metric); + void onMetricRemoved(PinotMetricName name); + } +} diff --git a/pinot-common/src/test/java/org/apache/pinot/plugin/metrics/fake/FakePinotTimer.java b/pinot-common/src/test/java/org/apache/pinot/plugin/metrics/fake/FakePinotTimer.java new file mode 100644 index 000000000000..a444c20b26f1 --- /dev/null +++ b/pinot-common/src/test/java/org/apache/pinot/plugin/metrics/fake/FakePinotTimer.java @@ -0,0 +1,96 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.pinot.plugin.metrics.fake; + +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicLong; +import org.apache.pinot.spi.metrics.PinotTimer; + + +public class FakePinotTimer implements PinotTimer { + private final AtomicLong _count = new AtomicLong(); + // Accumulated duration expressed in _durationUnit. + private final AtomicLong _sumDurationMs = new AtomicLong(); + private final TimeUnit _rateUnit; + + public FakePinotTimer(TimeUnit durationUnit, TimeUnit rateUnit) { + _rateUnit = rateUnit; + } + + @Override + public void update(long duration, TimeUnit unit) { + _count.incrementAndGet(); + _sumDurationMs.addAndGet(unit.toMillis(duration)); + } + + @Override + public Object getTimer() { + return this; + } + + @Override + public Object getMetered() { + return this; + } + + @Override + public Object getMetric() { + return this; + } + + @Override + public TimeUnit rateUnit() { + return _rateUnit; + } + + @Override + public String eventType() { + return "calls"; + } + + @Override + public long count() { + return _count.get(); + } + + /** Total elapsed time recorded in milliseconds. */ + public long sumMs() { + return _sumDurationMs.get(); + } + + @Override + public double fifteenMinuteRate() { + return 0; + } + + @Override + public double fiveMinuteRate() { + return 0; + } + + @Override + public double meanRate() { + return 0; + } + + @Override + public double oneMinuteRate() { + return 0; + } +} diff --git a/pinot-plugins/pinot-metrics/pinot-dropwizard/pom.xml b/pinot-plugins/pinot-metrics/pinot-dropwizard/pom.xml index d3a05ae2b8c7..ecaf12681cc0 100644 --- a/pinot-plugins/pinot-metrics/pinot-dropwizard/pom.xml +++ b/pinot-plugins/pinot-metrics/pinot-dropwizard/pom.xml @@ -48,6 +48,39 @@ com.google.auto.service auto-service-annotations + + + + org.apache.pinot + pinot-common + test + + + org.apache.pinot + pinot-common + test-jar + test + + + org.testng + testng + test + + + io.prometheus.jmx + collector + test + + + io.prometheus + prometheus-metrics-exporter-httpserver + test + + + org.apache.commons + commons-lang3 + test + diff --git a/pinot-plugins/pinot-metrics/pinot-dropwizard/src/test/java/org/apache/pinot/plugin/metrics/dropwizard/DropwizardAbstractMetricsTest.java b/pinot-plugins/pinot-metrics/pinot-dropwizard/src/test/java/org/apache/pinot/plugin/metrics/dropwizard/DropwizardAbstractMetricsTest.java new file mode 100644 index 000000000000..eaa6bca7bf8f --- /dev/null +++ b/pinot-plugins/pinot-metrics/pinot-dropwizard/src/test/java/org/apache/pinot/plugin/metrics/dropwizard/DropwizardAbstractMetricsTest.java @@ -0,0 +1,51 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.pinot.plugin.metrics.dropwizard; + +import org.apache.pinot.common.metrics.AbstractMetrics; +import org.apache.pinot.common.metrics.AbstractMetricsTest; +import org.apache.pinot.common.metrics.MetricsInspector; +import org.apache.pinot.spi.metrics.PinotMetricsRegistry; + + +/** + * Runs the shared {@link AbstractMetricsTest} against the real {@link DropwizardMetricsRegistry}. + */ +public class DropwizardAbstractMetricsTest extends AbstractMetricsTest { + + @Override + protected String metricsFactoryClassName() { + return DropwizardMetricsFactory.class.getName(); + } + + @Override + protected PinotMetricsRegistry buildRegistry() { + return new DropwizardMetricsRegistry(); + } + + @Override + protected MetricsInspector createInspector(PinotMetricsRegistry registry) { + return new DropwizardMetricsInspector(registry); + } + + @Override + protected long getGaugeValue(AbstractMetrics metrics, String metricName) { + return DropwizardMetricValueUtils.getGaugeValue(metrics, metricName); + } +} diff --git a/pinot-plugins/pinot-metrics/pinot-dropwizard/src/test/java/org/apache/pinot/plugin/metrics/dropwizard/DropwizardMetricValueUtils.java b/pinot-plugins/pinot-metrics/pinot-dropwizard/src/test/java/org/apache/pinot/plugin/metrics/dropwizard/DropwizardMetricValueUtils.java new file mode 100644 index 000000000000..af4482c08b61 --- /dev/null +++ b/pinot-plugins/pinot-metrics/pinot-dropwizard/src/test/java/org/apache/pinot/plugin/metrics/dropwizard/DropwizardMetricValueUtils.java @@ -0,0 +1,68 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.pinot.plugin.metrics.dropwizard; + +import org.apache.pinot.common.metrics.AbstractMetrics; +import org.apache.pinot.common.metrics.BrokerMetrics; +import org.apache.pinot.common.metrics.ControllerMetrics; +import org.apache.pinot.common.metrics.MinionMetrics; +import org.apache.pinot.common.metrics.ServerMetrics; +import org.apache.pinot.spi.metrics.PinotMetric; + + +/** + * Dropwizard-specific test utility for reading gauge values directly from the Dropwizard metrics registry. Mirrors the + * Yammer-side {@code YammerMetricValueUtils}. + */ +public class DropwizardMetricValueUtils { + private DropwizardMetricValueUtils() { + } + + public static long getGaugeValue(AbstractMetrics metrics, String metricName) { + PinotMetric pinotMetric = extractMetric(metrics, metricName); + if (pinotMetric == null) { + return 0; + } + Object gauge = pinotMetric.getMetric(); + if (!(gauge instanceof DropwizardSettableGauge)) { + throw new IllegalStateException("Expected DropwizardSettableGauge for " + metricName + " but got: " + gauge); + } + Object value = ((DropwizardSettableGauge) gauge).getValue(); + if (!(value instanceof Number)) { + throw new IllegalStateException("Gauge did not produce a Number: " + metricName + " -> " + value); + } + return ((Number) value).longValue(); + } + + private static PinotMetric extractMetric(AbstractMetrics metrics, String metricName) { + String metricPrefix; + if (metrics instanceof ControllerMetrics) { + metricPrefix = "pinot.controller."; + } else if (metrics instanceof BrokerMetrics) { + metricPrefix = "pinot.broker."; + } else if (metrics instanceof ServerMetrics) { + metricPrefix = "pinot.server."; + } else if (metrics instanceof MinionMetrics) { + metricPrefix = "pinot.minion."; + } else { + throw new IllegalArgumentException("Unsupported AbstractMetrics type: " + metrics.getClass()); + } + return metrics.getMetricsRegistry().allMetrics().get(new DropwizardMetricName(metricPrefix + metricName)); + } +} diff --git a/pinot-plugins/pinot-metrics/pinot-dropwizard/src/test/java/org/apache/pinot/plugin/metrics/dropwizard/DropwizardMetricsInspector.java b/pinot-plugins/pinot-metrics/pinot-dropwizard/src/test/java/org/apache/pinot/plugin/metrics/dropwizard/DropwizardMetricsInspector.java new file mode 100644 index 000000000000..d2e1788c43e6 --- /dev/null +++ b/pinot-plugins/pinot-metrics/pinot-dropwizard/src/test/java/org/apache/pinot/plugin/metrics/dropwizard/DropwizardMetricsInspector.java @@ -0,0 +1,133 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.pinot.plugin.metrics.dropwizard; + +import com.codahale.metrics.Counter; +import com.codahale.metrics.Gauge; +import com.codahale.metrics.Histogram; +import com.codahale.metrics.Meter; +import com.codahale.metrics.Metric; +import com.codahale.metrics.MetricRegistryListener; +import com.codahale.metrics.Timer; +import java.util.HashMap; +import java.util.Map; +import java.util.concurrent.TimeUnit; +import org.apache.pinot.common.metrics.MetricsInspector; +import org.apache.pinot.spi.metrics.PinotMetricName; +import org.apache.pinot.spi.metrics.PinotMetricsRegistry; + + +/** + * Dropwizard-backed implementation of {@link MetricsInspector}. + */ +public class DropwizardMetricsInspector extends MetricsInspector { + private final Map _metricMap = new HashMap<>(); + private volatile String _lastMetric; + + public DropwizardMetricsInspector(PinotMetricsRegistry registry) { + registry.addListener(new DropwizardMetricsRegistryListener(new MetricRegistryListener() { + @Override + public void onGaugeAdded(String name, Gauge metric) { + record(name, metric); + } + + @Override + public void onGaugeRemoved(String name) { + _metricMap.remove(name); + } + + @Override + public void onCounterAdded(String name, Counter metric) { + record(name, metric); + } + + @Override + public void onCounterRemoved(String name) { + _metricMap.remove(name); + } + + @Override + public void onHistogramAdded(String name, Histogram metric) { + record(name, metric); + } + + @Override + public void onHistogramRemoved(String name) { + _metricMap.remove(name); + } + + @Override + public void onMeterAdded(String name, Meter metric) { + record(name, metric); + } + + @Override + public void onMeterRemoved(String name) { + _metricMap.remove(name); + } + + @Override + public void onTimerAdded(String name, Timer metric) { + record(name, metric); + } + + @Override + public void onTimerRemoved(String name) { + _metricMap.remove(name); + } + + private void record(String name, Metric metric) { + _metricMap.put(name, metric); + _lastMetric = name; + } + })); + } + + @Override + public PinotMetricName lastMetric() { + return _lastMetric == null ? null : new DropwizardMetricName(_lastMetric); + } + + @Override + public long getTimerSumMs(PinotMetricName name) { + Metric metric = _metricMap.get(((DropwizardMetricName) name).getMetricName()); + if (!(metric instanceof Timer)) { + throw new IllegalArgumentException("Not a timer metric: " + name); + } + // Dropwizard Timer has no sum() method — derive it from the snapshot's recorded values (in ns). + // For tests the timer uses a SlidingTimeWindowArrayReservoir so every recorded value is present. + long totalNanos = 0; + for (long ns : ((Timer) metric).getSnapshot().getValues()) { + totalNanos += ns; + } + return TimeUnit.NANOSECONDS.toMillis(totalNanos); + } + + @Override + public long getMeteredCount(PinotMetricName name) { + Metric metric = _metricMap.get(((DropwizardMetricName) name).getMetricName()); + if (metric instanceof Meter) { + return ((Meter) metric).getCount(); + } + if (metric instanceof Timer) { + return ((Timer) metric).getCount(); + } + throw new IllegalArgumentException("Not a metered metric: " + name); + } +} diff --git a/pinot-common/src/test/java/org/apache/pinot/common/metrics/prometheus/dropwizard/DropwizardBrokerPrometheusMetricsTest.java b/pinot-plugins/pinot-metrics/pinot-dropwizard/src/test/java/org/apache/pinot/plugin/metrics/dropwizard/prometheus/DropwizardBrokerPrometheusMetricsTest.java similarity index 97% rename from pinot-common/src/test/java/org/apache/pinot/common/metrics/prometheus/dropwizard/DropwizardBrokerPrometheusMetricsTest.java rename to pinot-plugins/pinot-metrics/pinot-dropwizard/src/test/java/org/apache/pinot/plugin/metrics/dropwizard/prometheus/DropwizardBrokerPrometheusMetricsTest.java index 6ba25fed7920..d74c7f63d815 100644 --- a/pinot-common/src/test/java/org/apache/pinot/common/metrics/prometheus/dropwizard/DropwizardBrokerPrometheusMetricsTest.java +++ b/pinot-plugins/pinot-metrics/pinot-dropwizard/src/test/java/org/apache/pinot/plugin/metrics/dropwizard/prometheus/DropwizardBrokerPrometheusMetricsTest.java @@ -17,7 +17,7 @@ * under the License. */ -package org.apache.pinot.common.metrics.prometheus.dropwizard; +package org.apache.pinot.plugin.metrics.dropwizard.prometheus; import org.apache.pinot.common.metrics.BrokerGauge; import org.apache.pinot.common.metrics.BrokerMeter; diff --git a/pinot-common/src/test/java/org/apache/pinot/common/metrics/prometheus/dropwizard/DropwizardControllerPrometheusMetricsTest.java b/pinot-plugins/pinot-metrics/pinot-dropwizard/src/test/java/org/apache/pinot/plugin/metrics/dropwizard/prometheus/DropwizardControllerPrometheusMetricsTest.java similarity index 97% rename from pinot-common/src/test/java/org/apache/pinot/common/metrics/prometheus/dropwizard/DropwizardControllerPrometheusMetricsTest.java rename to pinot-plugins/pinot-metrics/pinot-dropwizard/src/test/java/org/apache/pinot/plugin/metrics/dropwizard/prometheus/DropwizardControllerPrometheusMetricsTest.java index 9c4bc7b45dbb..0ae21cf9339e 100644 --- a/pinot-common/src/test/java/org/apache/pinot/common/metrics/prometheus/dropwizard/DropwizardControllerPrometheusMetricsTest.java +++ b/pinot-plugins/pinot-metrics/pinot-dropwizard/src/test/java/org/apache/pinot/plugin/metrics/dropwizard/prometheus/DropwizardControllerPrometheusMetricsTest.java @@ -17,7 +17,7 @@ * under the License. */ -package org.apache.pinot.common.metrics.prometheus.dropwizard; +package org.apache.pinot.plugin.metrics.dropwizard.prometheus; import org.apache.pinot.common.metrics.ControllerGauge; import org.apache.pinot.common.metrics.ControllerMeter; diff --git a/pinot-common/src/test/java/org/apache/pinot/common/metrics/prometheus/dropwizard/DropwizardMinionPrometheusMetricsTest.java b/pinot-plugins/pinot-metrics/pinot-dropwizard/src/test/java/org/apache/pinot/plugin/metrics/dropwizard/prometheus/DropwizardMinionPrometheusMetricsTest.java similarity index 97% rename from pinot-common/src/test/java/org/apache/pinot/common/metrics/prometheus/dropwizard/DropwizardMinionPrometheusMetricsTest.java rename to pinot-plugins/pinot-metrics/pinot-dropwizard/src/test/java/org/apache/pinot/plugin/metrics/dropwizard/prometheus/DropwizardMinionPrometheusMetricsTest.java index 3bfa51f52664..ae79ffa9bf1e 100644 --- a/pinot-common/src/test/java/org/apache/pinot/common/metrics/prometheus/dropwizard/DropwizardMinionPrometheusMetricsTest.java +++ b/pinot-plugins/pinot-metrics/pinot-dropwizard/src/test/java/org/apache/pinot/plugin/metrics/dropwizard/prometheus/DropwizardMinionPrometheusMetricsTest.java @@ -17,7 +17,7 @@ * under the License. */ -package org.apache.pinot.common.metrics.prometheus.dropwizard; +package org.apache.pinot.plugin.metrics.dropwizard.prometheus; import org.apache.pinot.common.metrics.MinionGauge; import org.apache.pinot.common.metrics.MinionMeter; diff --git a/pinot-common/src/test/java/org/apache/pinot/common/metrics/prometheus/dropwizard/DropwizardServerPrometheusMetricsTest.java b/pinot-plugins/pinot-metrics/pinot-dropwizard/src/test/java/org/apache/pinot/plugin/metrics/dropwizard/prometheus/DropwizardServerPrometheusMetricsTest.java similarity index 92% rename from pinot-common/src/test/java/org/apache/pinot/common/metrics/prometheus/dropwizard/DropwizardServerPrometheusMetricsTest.java rename to pinot-plugins/pinot-metrics/pinot-dropwizard/src/test/java/org/apache/pinot/plugin/metrics/dropwizard/prometheus/DropwizardServerPrometheusMetricsTest.java index 70768e995455..ac6dfc60252c 100644 --- a/pinot-common/src/test/java/org/apache/pinot/common/metrics/prometheus/dropwizard/DropwizardServerPrometheusMetricsTest.java +++ b/pinot-plugins/pinot-metrics/pinot-dropwizard/src/test/java/org/apache/pinot/plugin/metrics/dropwizard/prometheus/DropwizardServerPrometheusMetricsTest.java @@ -17,14 +17,13 @@ * under the License. */ -package org.apache.pinot.common.metrics.prometheus.dropwizard; +package org.apache.pinot.plugin.metrics.dropwizard.prometheus; import org.apache.pinot.common.metrics.ServerGauge; import org.apache.pinot.common.metrics.ServerMeter; import org.apache.pinot.common.metrics.ServerTimer; import org.apache.pinot.common.metrics.prometheus.ServerPrometheusMetricsTest; import org.apache.pinot.plugin.metrics.dropwizard.DropwizardMetricsFactory; -import org.apache.pinot.plugin.metrics.yammer.YammerMetricsFactory; import org.apache.pinot.spi.annotations.metrics.PinotMetricsFactory; import org.testng.annotations.Test; @@ -37,7 +36,7 @@ public class DropwizardServerPrometheusMetricsTest extends ServerPrometheusMetri @Override protected PinotMetricsFactory getPinotMetricsFactory() { - return new YammerMetricsFactory(); + return new DropwizardMetricsFactory(); } @Override diff --git a/pinot-plugins/pinot-metrics/pinot-yammer/pom.xml b/pinot-plugins/pinot-metrics/pinot-yammer/pom.xml index 8e948a414c3e..a51da0ee3da7 100644 --- a/pinot-plugins/pinot-metrics/pinot-yammer/pom.xml +++ b/pinot-plugins/pinot-metrics/pinot-yammer/pom.xml @@ -44,6 +44,39 @@ com.google.auto.service auto-service-annotations + + + + org.apache.pinot + pinot-common + test + + + org.apache.pinot + pinot-common + test-jar + test + + + org.testng + testng + test + + + io.prometheus.jmx + collector + test + + + io.prometheus + prometheus-metrics-exporter-httpserver + test + + + org.apache.commons + commons-lang3 + test + diff --git a/pinot-plugins/pinot-metrics/pinot-yammer/src/main/java/org/apache/pinot/plugin/metrics/yammer/YammerSettableGauge.java b/pinot-plugins/pinot-metrics/pinot-yammer/src/main/java/org/apache/pinot/plugin/metrics/yammer/YammerSettableGauge.java index 0fa10b39c7ea..1c0364d47dba 100644 --- a/pinot-plugins/pinot-metrics/pinot-yammer/src/main/java/org/apache/pinot/plugin/metrics/yammer/YammerSettableGauge.java +++ b/pinot-plugins/pinot-metrics/pinot-yammer/src/main/java/org/apache/pinot/plugin/metrics/yammer/YammerSettableGauge.java @@ -53,4 +53,9 @@ public void setValueSupplier(Supplier valueSupplier) { public T value() { return _valueSupplier.get(); } + + @Override + public T getValue() { + return _valueSupplier.get(); + } } diff --git a/pinot-plugins/pinot-metrics/pinot-yammer/src/test/java/org/apache/pinot/plugin/metrics/yammer/YammerAbstractMetricsTest.java b/pinot-plugins/pinot-metrics/pinot-yammer/src/test/java/org/apache/pinot/plugin/metrics/yammer/YammerAbstractMetricsTest.java new file mode 100644 index 000000000000..d47769ab89db --- /dev/null +++ b/pinot-plugins/pinot-metrics/pinot-yammer/src/test/java/org/apache/pinot/plugin/metrics/yammer/YammerAbstractMetricsTest.java @@ -0,0 +1,52 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.pinot.plugin.metrics.yammer; + +import org.apache.pinot.common.metrics.AbstractMetrics; +import org.apache.pinot.common.metrics.AbstractMetricsTest; +import org.apache.pinot.common.metrics.MetricsInspector; +import org.apache.pinot.spi.metrics.PinotMetricsRegistry; + + +/** + * Runs the shared {@link AbstractMetricsTest} against the real {@link YammerMetricsRegistry}, verifying that the + * yammer plugin respects the generic {@code AbstractMetrics} contract. + */ +public class YammerAbstractMetricsTest extends AbstractMetricsTest { + + @Override + protected String metricsFactoryClassName() { + return YammerMetricsFactory.class.getName(); + } + + @Override + protected PinotMetricsRegistry buildRegistry() { + return new YammerMetricsRegistry(); + } + + @Override + protected MetricsInspector createInspector(PinotMetricsRegistry registry) { + return new YammerMetricsInspector(registry); + } + + @Override + protected long getGaugeValue(AbstractMetrics metrics, String metricName) { + return YammerMetricValueUtils.getGaugeValue(metrics, metricName); + } +} diff --git a/pinot-plugins/pinot-metrics/pinot-yammer/src/test/java/org/apache/pinot/plugin/metrics/yammer/YammerMetricValueUtils.java b/pinot-plugins/pinot-metrics/pinot-yammer/src/test/java/org/apache/pinot/plugin/metrics/yammer/YammerMetricValueUtils.java new file mode 100644 index 000000000000..88f92753a850 --- /dev/null +++ b/pinot-plugins/pinot-metrics/pinot-yammer/src/test/java/org/apache/pinot/plugin/metrics/yammer/YammerMetricValueUtils.java @@ -0,0 +1,132 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.pinot.plugin.metrics.yammer; + +import com.yammer.metrics.core.MetricName; +import org.apache.pinot.common.metrics.AbstractMetrics; +import org.apache.pinot.common.metrics.BrokerMetrics; +import org.apache.pinot.common.metrics.ControllerMetrics; +import org.apache.pinot.common.metrics.MinionMetrics; +import org.apache.pinot.common.metrics.ServerMetrics; +import org.apache.pinot.spi.metrics.PinotMetric; + + +/** + * Yammer-specific test utility for reading gauge values directly from the Yammer metrics registry. + * Unlike the generic MetricValueUtils in pinot-common (which reads from AbstractMetrics._gaugeValues), + * this class reads via {@link YammerSettableGauge}, supporting supplier-based gauges set via + * {@link AbstractMetrics#setOrUpdateGauge}. + */ +public class YammerMetricValueUtils { + private YammerMetricValueUtils() { + } + + public static boolean gaugeExists(AbstractMetrics metrics, String metricName) { + return extractMetric(metrics, metricName) != null; + } + + public static long getGaugeValue(AbstractMetrics metrics, String metricName) { + PinotMetric pinotMetric = extractMetric(metrics, metricName); + if (pinotMetric == null) { + return 0; + } + return ((YammerSettableGauge) pinotMetric.getMetric()).value(); + } + + public static boolean globalGaugeExists(AbstractMetrics metrics, AbstractMetrics.Gauge gauge) { + return extractMetric(metrics, gauge.getGaugeName()) != null; + } + + public static long getGlobalGaugeValue(AbstractMetrics metrics, AbstractMetrics.Gauge gauge) { + PinotMetric pinotMetric = extractMetric(metrics, gauge.getGaugeName()); + if (pinotMetric == null) { + return 0; + } + return ((YammerSettableGauge) pinotMetric.getMetric()).value(); + } + + public static boolean globalGaugeExists(AbstractMetrics metrics, String key, AbstractMetrics.Gauge gauge) { + return extractMetric(metrics, gauge.getGaugeName() + "." + key) != null; + } + + public static long getGlobalGaugeValue(AbstractMetrics metrics, String key, AbstractMetrics.Gauge gauge) { + PinotMetric pinotMetric = extractMetric(metrics, gauge.getGaugeName() + "." + key); + if (pinotMetric == null) { + return 0; + } + return ((YammerSettableGauge) pinotMetric.getMetric()).value(); + } + + public static boolean tableGaugeExists(AbstractMetrics metrics, String tableName, AbstractMetrics.Gauge gauge) { + return extractMetric(metrics, gauge.getGaugeName() + "." + tableName) != null; + } + + public static long getTableGaugeValue(AbstractMetrics metrics, String tableName, AbstractMetrics.Gauge gauge) { + PinotMetric pinotMetric = extractMetric(metrics, gauge.getGaugeName() + "." + tableName); + if (pinotMetric == null) { + return 0; + } + return ((YammerSettableGauge) pinotMetric.getMetric()).value(); + } + + public static boolean tableGaugeExists(AbstractMetrics metrics, String tableName, String key, + AbstractMetrics.Gauge gauge) { + return extractMetric(metrics, gauge.getGaugeName() + "." + tableName + "." + key) != null; + } + + public static long getTableGaugeValue(AbstractMetrics metrics, String tableName, String key, + AbstractMetrics.Gauge gauge) { + PinotMetric pinotMetric = extractMetric(metrics, gauge.getGaugeName() + "." + tableName + "." + key); + if (pinotMetric == null) { + return 0; + } + return ((YammerSettableGauge) pinotMetric.getMetric()).value(); + } + + public static boolean partitionGaugeExists(AbstractMetrics metrics, String tableName, int partitionId, + AbstractMetrics.Gauge gauge) { + return extractMetric(metrics, gauge.getGaugeName() + "." + tableName + "." + partitionId) != null; + } + + public static long getPartitionGaugeValue(AbstractMetrics metrics, String tableName, int partitionId, + AbstractMetrics.Gauge gauge) { + PinotMetric pinotMetric = extractMetric(metrics, gauge.getGaugeName() + "." + tableName + "." + partitionId); + if (pinotMetric == null) { + return 0; + } + return ((YammerSettableGauge) pinotMetric.getMetric()).value(); + } + + private static PinotMetric extractMetric(AbstractMetrics metrics, String metricName) { + String metricPrefix; + if (metrics instanceof ControllerMetrics) { + metricPrefix = "pinot.controller."; + } else if (metrics instanceof BrokerMetrics) { + metricPrefix = "pinot.broker."; + } else if (metrics instanceof ServerMetrics) { + metricPrefix = "pinot.server."; + } else if (metrics instanceof MinionMetrics) { + metricPrefix = "pinot.minion."; + } else { + throw new RuntimeException("unsupported AbstractMetrics type: " + metrics.getClass().toString()); + } + return metrics.getMetricsRegistry().allMetrics() + .get(new YammerMetricName(new MetricName(metrics.getClass(), metricPrefix + metricName))); + } +} diff --git a/pinot-plugins/pinot-metrics/pinot-yammer/src/test/java/org/apache/pinot/plugin/metrics/yammer/YammerMetricsInspector.java b/pinot-plugins/pinot-metrics/pinot-yammer/src/test/java/org/apache/pinot/plugin/metrics/yammer/YammerMetricsInspector.java new file mode 100644 index 000000000000..386663736eb0 --- /dev/null +++ b/pinot-plugins/pinot-metrics/pinot-yammer/src/test/java/org/apache/pinot/plugin/metrics/yammer/YammerMetricsInspector.java @@ -0,0 +1,129 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.pinot.plugin.metrics.yammer; + +import com.yammer.metrics.core.Counter; +import com.yammer.metrics.core.Gauge; +import com.yammer.metrics.core.Histogram; +import com.yammer.metrics.core.Metered; +import com.yammer.metrics.core.Metric; +import com.yammer.metrics.core.MetricName; +import com.yammer.metrics.core.MetricProcessor; +import com.yammer.metrics.core.MetricsRegistryListener; +import com.yammer.metrics.core.Timer; +import java.util.HashMap; +import java.util.Map; +import java.util.function.Function; +import org.apache.pinot.common.metrics.MetricsInspector; +import org.apache.pinot.spi.metrics.PinotMetricName; +import org.apache.pinot.spi.metrics.PinotMetricsRegistry; + + +/** + * Yammer-backed implementation of {@link MetricsInspector}. Uses Yammer's {@link MetricsRegistryListener} to capture + * metric registrations and provides timer-sum / metered-count readings via Yammer's {@link MetricProcessor}. + */ +public class YammerMetricsInspector extends MetricsInspector { + private final Map _metricMap = new HashMap<>(); + private volatile MetricName _lastMetric; + + public YammerMetricsInspector(PinotMetricsRegistry registry) { + registry.addListener(() -> new MetricsRegistryListener() { + @Override + public void onMetricAdded(MetricName metricName, Metric metric) { + _metricMap.put(metricName, metric); + _lastMetric = metricName; + } + + @Override + public void onMetricRemoved(MetricName metricName) { + _metricMap.remove(metricName); + } + }); + } + + @Override + public PinotMetricName lastMetric() { + return _lastMetric == null ? null : new YammerMetricName(_lastMetric); + } + + @Override + public long getTimerSumMs(PinotMetricName name) { + return access(name, m -> m._timer == null ? null : (long) m._timer.sum()); + } + + @Override + public long getMeteredCount(PinotMetricName name) { + return access(name, m -> m._metered == null ? null : m._metered.count()); + } + + private T access(PinotMetricName name, Function property) { + MetricName inner = ((YammerMetricName) name).getMetricName(); + Metric metric = _metricMap.get(inner); + if (metric == null) { + throw new IllegalArgumentException("Metric not found: " + inner); + } + MetricAccessor accessor = new MetricAccessor(); + try { + metric.processWith(accessor, null, null); + } catch (Exception e) { + throw new IllegalStateException("Unexpected error processing metric: " + metric, e); + } + T result = property.apply(accessor); + if (result == null) { + throw new IllegalArgumentException("Requested metric type not found in metric [" + inner.getName() + "]"); + } + return result; + } + + private static class MetricAccessor implements MetricProcessor { + public Metered _metered; + public Counter _counter; + public Histogram _histogram; + public Timer _timer; + public Gauge _gauge; + + @Override + public void processMeter(MetricName n, Metered metered, Void v) { + _metered = metered; + } + + @Override + public void processCounter(MetricName n, Counter counter, Void v) { + _counter = counter; + } + + @Override + public void processHistogram(MetricName n, Histogram histogram, Void v) { + _histogram = histogram; + } + + @Override + public void processTimer(MetricName n, Timer timer, Void v) { + _timer = timer; + // Timer also implements Metered. + _metered = timer; + } + + @Override + public void processGauge(MetricName metricName, Gauge gauge, Void v) { + _gauge = gauge; + } + } +} diff --git a/pinot-common/src/test/java/org/apache/pinot/common/metrics/prometheus/yammer/YammerBrokerPrometheusMetricsTest.java b/pinot-plugins/pinot-metrics/pinot-yammer/src/test/java/org/apache/pinot/plugin/metrics/yammer/prometheus/YammerBrokerPrometheusMetricsTest.java similarity index 89% rename from pinot-common/src/test/java/org/apache/pinot/common/metrics/prometheus/yammer/YammerBrokerPrometheusMetricsTest.java rename to pinot-plugins/pinot-metrics/pinot-yammer/src/test/java/org/apache/pinot/plugin/metrics/yammer/prometheus/YammerBrokerPrometheusMetricsTest.java index d58a3fdd3ce0..61dac54ac93a 100644 --- a/pinot-common/src/test/java/org/apache/pinot/common/metrics/prometheus/yammer/YammerBrokerPrometheusMetricsTest.java +++ b/pinot-plugins/pinot-metrics/pinot-yammer/src/test/java/org/apache/pinot/plugin/metrics/yammer/prometheus/YammerBrokerPrometheusMetricsTest.java @@ -17,7 +17,7 @@ * under the License. */ -package org.apache.pinot.common.metrics.prometheus.yammer; +package org.apache.pinot.plugin.metrics.yammer.prometheus; import org.apache.pinot.common.metrics.prometheus.BrokerPrometheusMetricsTest; import org.apache.pinot.plugin.metrics.yammer.YammerMetricsFactory; @@ -33,6 +33,6 @@ protected PinotMetricsFactory getPinotMetricsFactory() { @Override protected String getConfigFile() { - return "../docker/images/pinot/etc/jmx_prometheus_javaagent/configs/broker.yml"; + return "../../../docker/images/pinot/etc/jmx_prometheus_javaagent/configs/broker.yml"; } } diff --git a/pinot-common/src/test/java/org/apache/pinot/common/metrics/prometheus/yammer/YammerControllerPrometheusMetricsTest.java b/pinot-plugins/pinot-metrics/pinot-yammer/src/test/java/org/apache/pinot/plugin/metrics/yammer/prometheus/YammerControllerPrometheusMetricsTest.java similarity index 89% rename from pinot-common/src/test/java/org/apache/pinot/common/metrics/prometheus/yammer/YammerControllerPrometheusMetricsTest.java rename to pinot-plugins/pinot-metrics/pinot-yammer/src/test/java/org/apache/pinot/plugin/metrics/yammer/prometheus/YammerControllerPrometheusMetricsTest.java index e7f7f11dc857..8df034510edd 100644 --- a/pinot-common/src/test/java/org/apache/pinot/common/metrics/prometheus/yammer/YammerControllerPrometheusMetricsTest.java +++ b/pinot-plugins/pinot-metrics/pinot-yammer/src/test/java/org/apache/pinot/plugin/metrics/yammer/prometheus/YammerControllerPrometheusMetricsTest.java @@ -17,7 +17,7 @@ * under the License. */ -package org.apache.pinot.common.metrics.prometheus.yammer; +package org.apache.pinot.plugin.metrics.yammer.prometheus; import org.apache.pinot.common.metrics.prometheus.ControllerPrometheusMetricsTest; import org.apache.pinot.plugin.metrics.yammer.YammerMetricsFactory; @@ -33,6 +33,6 @@ protected PinotMetricsFactory getPinotMetricsFactory() { @Override protected String getConfigFile() { - return "../docker/images/pinot/etc/jmx_prometheus_javaagent/configs/controller.yml"; + return "../../../docker/images/pinot/etc/jmx_prometheus_javaagent/configs/controller.yml"; } } diff --git a/pinot-common/src/test/java/org/apache/pinot/common/metrics/prometheus/yammer/YammerMinionPrometheusMetricsTest.java b/pinot-plugins/pinot-metrics/pinot-yammer/src/test/java/org/apache/pinot/plugin/metrics/yammer/prometheus/YammerMinionPrometheusMetricsTest.java similarity index 89% rename from pinot-common/src/test/java/org/apache/pinot/common/metrics/prometheus/yammer/YammerMinionPrometheusMetricsTest.java rename to pinot-plugins/pinot-metrics/pinot-yammer/src/test/java/org/apache/pinot/plugin/metrics/yammer/prometheus/YammerMinionPrometheusMetricsTest.java index 520c809e7ade..47c06fb2adfd 100644 --- a/pinot-common/src/test/java/org/apache/pinot/common/metrics/prometheus/yammer/YammerMinionPrometheusMetricsTest.java +++ b/pinot-plugins/pinot-metrics/pinot-yammer/src/test/java/org/apache/pinot/plugin/metrics/yammer/prometheus/YammerMinionPrometheusMetricsTest.java @@ -17,7 +17,7 @@ * under the License. */ -package org.apache.pinot.common.metrics.prometheus.yammer; +package org.apache.pinot.plugin.metrics.yammer.prometheus; import org.apache.pinot.common.metrics.prometheus.MinionPrometheusMetricsTest; import org.apache.pinot.plugin.metrics.yammer.YammerMetricsFactory; @@ -33,6 +33,6 @@ protected PinotMetricsFactory getPinotMetricsFactory() { @Override protected String getConfigFile() { - return "../docker/images/pinot/etc/jmx_prometheus_javaagent/configs/minion.yml"; + return "../../../docker/images/pinot/etc/jmx_prometheus_javaagent/configs/minion.yml"; } } diff --git a/pinot-common/src/test/java/org/apache/pinot/common/metrics/prometheus/yammer/YammerServerPrometheusMetricsTest.java b/pinot-plugins/pinot-metrics/pinot-yammer/src/test/java/org/apache/pinot/plugin/metrics/yammer/prometheus/YammerServerPrometheusMetricsTest.java similarity index 89% rename from pinot-common/src/test/java/org/apache/pinot/common/metrics/prometheus/yammer/YammerServerPrometheusMetricsTest.java rename to pinot-plugins/pinot-metrics/pinot-yammer/src/test/java/org/apache/pinot/plugin/metrics/yammer/prometheus/YammerServerPrometheusMetricsTest.java index e32228716d59..d9a8a744cd17 100644 --- a/pinot-common/src/test/java/org/apache/pinot/common/metrics/prometheus/yammer/YammerServerPrometheusMetricsTest.java +++ b/pinot-plugins/pinot-metrics/pinot-yammer/src/test/java/org/apache/pinot/plugin/metrics/yammer/prometheus/YammerServerPrometheusMetricsTest.java @@ -17,7 +17,7 @@ * under the License. */ -package org.apache.pinot.common.metrics.prometheus.yammer; +package org.apache.pinot.plugin.metrics.yammer.prometheus; import org.apache.pinot.common.metrics.prometheus.ServerPrometheusMetricsTest; import org.apache.pinot.plugin.metrics.yammer.YammerMetricsFactory; @@ -33,6 +33,6 @@ protected PinotMetricsFactory getPinotMetricsFactory() { @Override protected String getConfigFile() { - return "../docker/images/pinot/etc/jmx_prometheus_javaagent/configs/server.yml"; + return "../../../docker/images/pinot/etc/jmx_prometheus_javaagent/configs/server.yml"; } } diff --git a/pinot-spi/src/main/java/org/apache/pinot/spi/metrics/PinotGauge.java b/pinot-spi/src/main/java/org/apache/pinot/spi/metrics/PinotGauge.java index 8e511c5a9cc3..395234925cec 100644 --- a/pinot-spi/src/main/java/org/apache/pinot/spi/metrics/PinotGauge.java +++ b/pinot-spi/src/main/java/org/apache/pinot/spi/metrics/PinotGauge.java @@ -54,4 +54,9 @@ default void setValueSupplier(Supplier valueSupplier) { * @return the metric's current value */ T value(); + + @Override + default T getValue() { + return value(); + } } diff --git a/pinot-spi/src/main/java/org/apache/pinot/spi/metrics/SettableValue.java b/pinot-spi/src/main/java/org/apache/pinot/spi/metrics/SettableValue.java index 05212a6bfa0a..03f0c9650310 100644 --- a/pinot-spi/src/main/java/org/apache/pinot/spi/metrics/SettableValue.java +++ b/pinot-spi/src/main/java/org/apache/pinot/spi/metrics/SettableValue.java @@ -37,4 +37,9 @@ public interface SettableValue { * @param valueSupplier the value supplier to set. */ void setValueSupplier(Supplier valueSupplier); + + /** + * Returns the current value produced by either the set value or the value supplier. + */ + T getValue(); }