Github user joshelser commented on a diff in the pull request: https://github.com/apache/phoenix/pull/315#discussion_r206597366 --- Diff: phoenix-core/src/it/java/org/apache/phoenix/monitoring/PhoenixMetricsIT.java --- @@ -204,6 +230,45 @@ private static void resetGlobalMetrics() { } } + // Phoenix Client Metrics are transported via Hadoop-metrics2 sink + // The test sink is defined at GlobalPhoenixMetricsTestSink + // Configuration for Hadoop-metrics2 comes from hadoop-metrics2.properties file located in test/resources + private boolean verifyMetricsFromSink() throws InterruptedException { + Map<String, Long> expectedMetrics = new HashMap<>(); + for (GlobalMetric m : PhoenixRuntime.getGlobalPhoenixClientMetrics()) { + expectedMetrics.put(m.getMetricType().name(), m.getTotalSum()); + } + + for (int i = 0; i < MAX_RETRIES; i++) { + LOG.info("Verifying Global Metrics from Hadoop Sink, Retry: " + (i + 1)); + if (verifyMetricsFromSinkOnce(expectedMetrics)) { + LOG.info("Values from Hadoop Metrics Sink match actual values"); + return true; + } + Thread.sleep(1000); + } + return false; + } + + private boolean verifyMetricsFromSinkOnce(Map<String, Long> expectedMetrics) { + synchronized (GlobalPhoenixMetricsTestSink.lock) { + for (AbstractMetric metric : GlobalPhoenixMetricsTestSink.metrics) { + if (expectedMetrics.containsKey(metric.name())) { --- End diff -- I thought there was an issue with unboxing the value from `expectedMetrics.get(metric.name())`. I think it would be cleaner to do: ``` Long value = expectedMetrics.get(metric.name()); if (value != null) { long expectedValue = value.longValue(); ... ```
---