This is an automated email from the ASF dual-hosted git repository.
edcoleman pushed a commit to branch 2.1
in repository https://gitbox.apache.org/repos/asf/accumulo.git
The following commit(s) were added to refs/heads/2.1 by this push:
new 38d9da710b Use micrometer noop classes for default meter
initialization (#4542)
38d9da710b is described below
commit 38d9da710b13ab7e81770ca015055da9a8b3d755
Author: EdColeman <d...@etcoleman.com>
AuthorDate: Tue May 28 18:50:05 2024 -0400
Use micrometer noop classes for default meter initialization (#4542)
* Use micrometer noop classes for default meter initialization
- removes custom NoOpDistributionSummary
- adds default initializations for other meter types
-organize MetricsIT excludes to ease future merging.
---
.../server/metrics/NoOpDistributionSummary.java | 83 ----------------------
.../accumulo/server/metrics/NoopMetrics.java | 79 ++++++++++++++++++++
.../accumulo/server/metrics/ThriftMetrics.java | 4 +-
...butionSummaryTest.java => NoopMetricsTest.java} | 20 +++++-
.../accumulo/server/metrics/ThriftMetricsTest.java | 3 +-
.../manager/metrics/ReplicationMetrics.java | 3 +-
.../apache/accumulo/tserver/ScanServerMetrics.java | 7 +-
.../tserver/metrics/TabletServerMinCMetrics.java | 5 +-
.../tserver/metrics/TabletServerScanMetrics.java | 16 ++---
.../tserver/metrics/TabletServerUpdateMetrics.java | 16 ++---
.../apache/accumulo/test/metrics/MetricsIT.java | 20 ++++--
11 files changed, 140 insertions(+), 116 deletions(-)
diff --git
a/server/base/src/main/java/org/apache/accumulo/server/metrics/NoOpDistributionSummary.java
b/server/base/src/main/java/org/apache/accumulo/server/metrics/NoOpDistributionSummary.java
deleted file mode 100644
index 262dc0802a..0000000000
---
a/server/base/src/main/java/org/apache/accumulo/server/metrics/NoOpDistributionSummary.java
+++ /dev/null
@@ -1,83 +0,0 @@
-/*
- * 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
- *
- * https://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.accumulo.server.metrics;
-
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-import io.micrometer.core.instrument.DistributionSummary;
-import io.micrometer.core.instrument.Tag;
-import io.micrometer.core.instrument.Tags;
-import io.micrometer.core.instrument.distribution.HistogramSnapshot;
-
-/**
- * Provides a default DistributionSummary that does not do anything. This can
be used to prevent NPE
- * if metrics have not been initialized when a DistributionSummary is expected.
- * <p>
- * Normally DistributionSummaries are created using a builder that takes a
registry.
- *
- * <pre>
- * DistributionSummary distSum;
- * ...
- * public void registerMetrics(MeterRegistry registry) {
- * ...
- * distSum = DistributionSummary.builder("metric
name").description("...").register(registry);
- * ...
- * }
- * </pre>
- *
- * Until the registration is called, the instance variable is null. If code
then tries to update the
- * metric, a NPE is thrown. Using this class to provide a default value
prevents this from occurring
- * and on registration, it is replaced with a valid instance.
- */
-public class NoOpDistributionSummary implements DistributionSummary {
-
- private static final Logger LOG =
LoggerFactory.getLogger(NoOpDistributionSummary.class);
-
- @Override
- public void record(double v) {
- LOG.debug("record ignored - distribution summary will not be available.");
- }
-
- @Override
- public long count() {
- return 0;
- }
-
- @Override
- public double totalAmount() {
- return 0;
- }
-
- @Override
- public double max() {
- return 0;
- }
-
- @Override
- public HistogramSnapshot takeSnapshot() {
- return new HistogramSnapshot(0L, 0.0, 0.0, null, null, null);
- }
-
- @Override
- public Id getId() {
- return new Id("thrift.metrics.uninitialized", Tags.of(Tag.of("none",
"uninitialized")), null,
- "placeholder for uninitialized thrift metrics", Type.OTHER);
- }
-}
diff --git
a/server/base/src/main/java/org/apache/accumulo/server/metrics/NoopMetrics.java
b/server/base/src/main/java/org/apache/accumulo/server/metrics/NoopMetrics.java
new file mode 100644
index 0000000000..065106e2bb
--- /dev/null
+++
b/server/base/src/main/java/org/apache/accumulo/server/metrics/NoopMetrics.java
@@ -0,0 +1,79 @@
+/*
+ * 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
+ *
+ * https://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.accumulo.server.metrics;
+
+import java.util.concurrent.atomic.AtomicInteger;
+
+import io.micrometer.core.instrument.Counter;
+import io.micrometer.core.instrument.DistributionSummary;
+import io.micrometer.core.instrument.FunctionTimer;
+import io.micrometer.core.instrument.Gauge;
+import io.micrometer.core.instrument.Meter;
+import io.micrometer.core.instrument.Tags;
+import io.micrometer.core.instrument.Timer;
+import io.micrometer.core.instrument.noop.NoopCounter;
+import io.micrometer.core.instrument.noop.NoopDistributionSummary;
+import io.micrometer.core.instrument.noop.NoopFunctionTimer;
+import io.micrometer.core.instrument.noop.NoopGauge;
+import io.micrometer.core.instrument.noop.NoopTimer;
+
+/**
+ * Convenience utility class to return micrometer noop metrics. Initialization
of the metrics system
+ * registry can be delayed so that common tags with values determined at
runtime (for example, port
+ * numbers). Initializing meters that are created from the registry at
initialization with a noop
+ * implementation prevents NPEs if something tries to record a metric value
before the
+ * initialization has run.
+ */
+public class NoopMetrics {
+ private final static AtomicInteger idCount = new AtomicInteger(0);
+
+ private NoopMetrics() {}
+
+ public static Counter useNoopCounter() {
+ return new NoopCounter(noopId(Meter.Type.COUNTER));
+ }
+
+ public static Gauge useNoopGauge() {
+ return new NoopGauge(noopId(Meter.Type.GAUGE));
+ }
+
+ public static DistributionSummary useNoopDistributionSummary() {
+ return new
NoopDistributionSummary(noopId(Meter.Type.DISTRIBUTION_SUMMARY));
+ }
+
+ public static Timer useNoopTimer() {
+ return new NoopTimer(noopId(Meter.Type.TIMER));
+ }
+
+ public static FunctionTimer useNoopFunctionTimer() {
+ return new NoopFunctionTimer(noopId(Meter.Type.TIMER));
+ }
+
+ /**
+ * provide a unique id to guard against the id being used as a key.
+ */
+ private static Meter.Id noopId(final Meter.Type type) {
+ return new Meter.Id(uniqueName(), Tags.of("noop", "none"), "", "", type);
+ }
+
+ private static String uniqueName() {
+ return "noop" + idCount.incrementAndGet();
+ }
+
+}
diff --git
a/server/base/src/main/java/org/apache/accumulo/server/metrics/ThriftMetrics.java
b/server/base/src/main/java/org/apache/accumulo/server/metrics/ThriftMetrics.java
index 1d113d3e1e..c6288752ad 100644
---
a/server/base/src/main/java/org/apache/accumulo/server/metrics/ThriftMetrics.java
+++
b/server/base/src/main/java/org/apache/accumulo/server/metrics/ThriftMetrics.java
@@ -25,8 +25,8 @@ import io.micrometer.core.instrument.MeterRegistry;
public class ThriftMetrics implements MetricsProducer {
- private DistributionSummary idle = new NoOpDistributionSummary();
- private DistributionSummary execute = new NoOpDistributionSummary();
+ private DistributionSummary idle = NoopMetrics.useNoopDistributionSummary();
+ private DistributionSummary execute =
NoopMetrics.useNoopDistributionSummary();
public ThriftMetrics() {}
diff --git
a/server/base/src/test/java/org/apache/accumulo/server/metrics/NoOpDistributionSummaryTest.java
b/server/base/src/test/java/org/apache/accumulo/server/metrics/NoopMetricsTest.java
similarity index 67%
rename from
server/base/src/test/java/org/apache/accumulo/server/metrics/NoOpDistributionSummaryTest.java
rename to
server/base/src/test/java/org/apache/accumulo/server/metrics/NoopMetricsTest.java
index d374791d02..073e467b18 100644
---
a/server/base/src/test/java/org/apache/accumulo/server/metrics/NoOpDistributionSummaryTest.java
+++
b/server/base/src/test/java/org/apache/accumulo/server/metrics/NoopMetricsTest.java
@@ -19,13 +19,29 @@
package org.apache.accumulo.server.metrics;
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.junit.jupiter.api.Assertions.assertNotEquals;
import org.junit.jupiter.api.Test;
-class NoOpDistributionSummaryTest {
+import io.micrometer.core.instrument.Counter;
+import io.micrometer.core.instrument.DistributionSummary;
+import io.micrometer.core.instrument.Gauge;
+
+public class NoopMetricsTest {
+ @Test
+ public void testNames() {
+ Counter c1 = NoopMetrics.useNoopCounter();
+ Counter c2 = NoopMetrics.useNoopCounter();
+
+ Gauge g1 = NoopMetrics.useNoopGauge();
+
+ assertNotEquals(c1.getId(), c2.getId());
+ assertNotEquals(c1.getId(), g1.getId());
+ }
+
@Test
public void testNoOp() {
- NoOpDistributionSummary noop = new NoOpDistributionSummary();
+ DistributionSummary noop = NoopMetrics.useNoopDistributionSummary();
assertDoesNotThrow(() -> noop.getId());
assertDoesNotThrow(() -> noop.takeSnapshot());
assertDoesNotThrow(() -> noop.max());
diff --git
a/server/base/src/test/java/org/apache/accumulo/server/metrics/ThriftMetricsTest.java
b/server/base/src/test/java/org/apache/accumulo/server/metrics/ThriftMetricsTest.java
index 0225095976..91730d0582 100644
---
a/server/base/src/test/java/org/apache/accumulo/server/metrics/ThriftMetricsTest.java
+++
b/server/base/src/test/java/org/apache/accumulo/server/metrics/ThriftMetricsTest.java
@@ -31,6 +31,7 @@ import org.slf4j.LoggerFactory;
import io.micrometer.core.instrument.DistributionSummary;
import io.micrometer.core.instrument.MeterRegistry;
+import io.micrometer.core.instrument.noop.NoopDistributionSummary;
import io.micrometer.core.instrument.simple.SimpleMeterRegistry;
class ThriftMetricsTest {
@@ -54,7 +55,7 @@ class ThriftMetricsTest {
registry.forEachMeter(m -> {
LOG.trace("meter: {}", m.getId());
assertInstanceOf(DistributionSummary.class, m);
- assertFalse(m instanceof NoOpDistributionSummary);
+ assertFalse(m instanceof NoopDistributionSummary);
});
assertTrue(registry.get(METRICS_THRIFT_IDLE).summary().count() > 0);
assertTrue(registry.get(METRICS_THRIFT_EXECUTE).summary().count() > 0);
diff --git
a/server/manager/src/main/java/org/apache/accumulo/manager/metrics/ReplicationMetrics.java
b/server/manager/src/main/java/org/apache/accumulo/manager/metrics/ReplicationMetrics.java
index 931fb30215..181df66c69 100644
---
a/server/manager/src/main/java/org/apache/accumulo/manager/metrics/ReplicationMetrics.java
+++
b/server/manager/src/main/java/org/apache/accumulo/manager/metrics/ReplicationMetrics.java
@@ -36,6 +36,7 @@ import org.apache.accumulo.core.replication.ReplicationTable;
import org.apache.accumulo.core.replication.ReplicationTarget;
import org.apache.accumulo.core.util.threads.ThreadPools;
import org.apache.accumulo.manager.Manager;
+import org.apache.accumulo.server.metrics.NoopMetrics;
import org.apache.accumulo.server.replication.ReplicationUtil;
import org.apache.hadoop.fs.Path;
import org.slf4j.Logger;
@@ -53,7 +54,7 @@ public class ReplicationMetrics implements MetricsProducer {
private final ReplicationUtil replicationUtil;
private final Map<Path,Long> pathModTimes;
- private Timer replicationQueueTimer;
+ private Timer replicationQueueTimer = NoopMetrics.useNoopTimer();
private AtomicLong pendingFiles;
private AtomicInteger numPeers;
private AtomicInteger maxReplicationThreads;
diff --git
a/server/tserver/src/main/java/org/apache/accumulo/tserver/ScanServerMetrics.java
b/server/tserver/src/main/java/org/apache/accumulo/tserver/ScanServerMetrics.java
index 1ba7de6e33..b56bc20b1a 100644
---
a/server/tserver/src/main/java/org/apache/accumulo/tserver/ScanServerMetrics.java
+++
b/server/tserver/src/main/java/org/apache/accumulo/tserver/ScanServerMetrics.java
@@ -24,6 +24,7 @@ import java.util.concurrent.atomic.AtomicLong;
import org.apache.accumulo.core.dataImpl.KeyExtent;
import org.apache.accumulo.core.metadata.schema.TabletMetadata;
import org.apache.accumulo.core.metrics.MetricsProducer;
+import org.apache.accumulo.server.metrics.NoopMetrics;
import com.github.benmanes.caffeine.cache.LoadingCache;
import com.google.common.base.Preconditions;
@@ -36,9 +37,9 @@ import
io.micrometer.core.instrument.binder.cache.CaffeineCacheMetrics;
public class ScanServerMetrics implements MetricsProducer {
- private Timer totalReservationTimer;
- private Timer writeOutReservationTimer;
- private Counter busyTimeoutCount;
+ private Timer totalReservationTimer = NoopMetrics.useNoopTimer();
+ private Timer writeOutReservationTimer = NoopMetrics.useNoopTimer();
+ private Counter busyTimeoutCount = NoopMetrics.useNoopCounter();
private final AtomicLong reservationConflictCount = new AtomicLong(0);
private final LoadingCache<KeyExtent,TabletMetadata> tabletMetadataCache;
diff --git
a/server/tserver/src/main/java/org/apache/accumulo/tserver/metrics/TabletServerMinCMetrics.java
b/server/tserver/src/main/java/org/apache/accumulo/tserver/metrics/TabletServerMinCMetrics.java
index a7f402343c..bf95b00262 100644
---
a/server/tserver/src/main/java/org/apache/accumulo/tserver/metrics/TabletServerMinCMetrics.java
+++
b/server/tserver/src/main/java/org/apache/accumulo/tserver/metrics/TabletServerMinCMetrics.java
@@ -21,14 +21,15 @@ package org.apache.accumulo.tserver.metrics;
import java.time.Duration;
import org.apache.accumulo.core.metrics.MetricsProducer;
+import org.apache.accumulo.server.metrics.NoopMetrics;
import io.micrometer.core.instrument.MeterRegistry;
import io.micrometer.core.instrument.Timer;
public class TabletServerMinCMetrics implements MetricsProducer {
- private Timer activeMinc;
- private Timer queuedMinc;
+ private Timer activeMinc = NoopMetrics.useNoopTimer();
+ private Timer queuedMinc = NoopMetrics.useNoopTimer();
public void addActive(long value) {
activeMinc.record(Duration.ofMillis(value));
diff --git
a/server/tserver/src/main/java/org/apache/accumulo/tserver/metrics/TabletServerScanMetrics.java
b/server/tserver/src/main/java/org/apache/accumulo/tserver/metrics/TabletServerScanMetrics.java
index bf22d1d297..c2c8dd0d32 100644
---
a/server/tserver/src/main/java/org/apache/accumulo/tserver/metrics/TabletServerScanMetrics.java
+++
b/server/tserver/src/main/java/org/apache/accumulo/tserver/metrics/TabletServerScanMetrics.java
@@ -23,7 +23,7 @@ import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.LongAdder;
import org.apache.accumulo.core.metrics.MetricsProducer;
-import org.apache.accumulo.server.metrics.NoOpDistributionSummary;
+import org.apache.accumulo.server.metrics.NoopMetrics;
import io.micrometer.core.instrument.Counter;
import io.micrometer.core.instrument.DistributionSummary;
@@ -34,13 +34,13 @@ import io.micrometer.core.instrument.Timer;
public class TabletServerScanMetrics implements MetricsProducer {
private final AtomicInteger openFiles = new AtomicInteger(0);
- private Timer scans;
- private DistributionSummary resultsPerScan = new NoOpDistributionSummary();
- private DistributionSummary yields = new NoOpDistributionSummary();
- private Counter startScanCalls;
- private Counter continueScanCalls;
- private Counter closeScanCalls;
- private Counter busyTimeoutCount;
+ private Timer scans = NoopMetrics.useNoopTimer();
+ private DistributionSummary resultsPerScan =
NoopMetrics.useNoopDistributionSummary();
+ private DistributionSummary yields =
NoopMetrics.useNoopDistributionSummary();
+ private Counter startScanCalls = NoopMetrics.useNoopCounter();
+ private Counter continueScanCalls = NoopMetrics.useNoopCounter();;
+ private Counter closeScanCalls = NoopMetrics.useNoopCounter();;
+ private Counter busyTimeoutCount = NoopMetrics.useNoopCounter();;
private final LongAdder lookupCount = new LongAdder();
private final LongAdder queryResultCount = new LongAdder();
diff --git
a/server/tserver/src/main/java/org/apache/accumulo/tserver/metrics/TabletServerUpdateMetrics.java
b/server/tserver/src/main/java/org/apache/accumulo/tserver/metrics/TabletServerUpdateMetrics.java
index 09ad197b6d..a369458b6c 100644
---
a/server/tserver/src/main/java/org/apache/accumulo/tserver/metrics/TabletServerUpdateMetrics.java
+++
b/server/tserver/src/main/java/org/apache/accumulo/tserver/metrics/TabletServerUpdateMetrics.java
@@ -21,7 +21,7 @@ package org.apache.accumulo.tserver.metrics;
import java.time.Duration;
import org.apache.accumulo.core.metrics.MetricsProducer;
-import org.apache.accumulo.server.metrics.NoOpDistributionSummary;
+import org.apache.accumulo.server.metrics.NoopMetrics;
import io.micrometer.core.instrument.Counter;
import io.micrometer.core.instrument.DistributionSummary;
@@ -30,13 +30,13 @@ import io.micrometer.core.instrument.Timer;
public class TabletServerUpdateMetrics implements MetricsProducer {
- private Counter permissionErrorsCounter;
- private Counter unknownTabletErrorsCounter;
- private Counter constraintViolationsCounter;
- private Timer commitPrepStat;
- private Timer walogWriteTimeStat;
- private Timer commitTimeStat;
- private DistributionSummary mutationArraySizeStat = new
NoOpDistributionSummary();
+ private Counter permissionErrorsCounter = NoopMetrics.useNoopCounter();
+ private Counter unknownTabletErrorsCounter = NoopMetrics.useNoopCounter();
+ private Counter constraintViolationsCounter = NoopMetrics.useNoopCounter();
+ private Timer commitPrepStat = NoopMetrics.useNoopTimer();
+ private Timer walogWriteTimeStat = NoopMetrics.useNoopTimer();;
+ private Timer commitTimeStat = NoopMetrics.useNoopTimer();;
+ private DistributionSummary mutationArraySizeStat =
NoopMetrics.useNoopDistributionSummary();
public void addPermissionErrors(long value) {
permissionErrorsCounter.increment(value);
diff --git a/test/src/main/java/org/apache/accumulo/test/metrics/MetricsIT.java
b/test/src/main/java/org/apache/accumulo/test/metrics/MetricsIT.java
index 5d9c833be5..f0b53992b4 100644
--- a/test/src/main/java/org/apache/accumulo/test/metrics/MetricsIT.java
+++ b/test/src/main/java/org/apache/accumulo/test/metrics/MetricsIT.java
@@ -98,14 +98,22 @@ public class MetricsIT extends ConfigurableMacBase
implements MetricsProducer {
doWorkToGenerateMetrics();
cluster.stop();
+ // meter names sorted and formatting disabled to make it easier to diff
changes
+ // @formatter:off
+ Set<String> unexpectedMetrics =
+ Set.of(METRICS_COMPACTOR_MAJC_STUCK,
+ METRICS_REPLICATION_QUEUE,
+ METRICS_SCAN_YIELDS,
+ METRICS_UPDATE_ERRORS);
- Set<String> unexpectedMetrics = Set.of(METRICS_SCAN_YIELDS,
METRICS_UPDATE_ERRORS,
- METRICS_REPLICATION_QUEUE, METRICS_COMPACTOR_MAJC_STUCK,
METRICS_SCAN_BUSY_TIMEOUT_COUNTER);
// add sserver as flaky until scan server included in mini tests.
- Set<String> flakyMetrics = Set.of(METRICS_GC_WAL_ERRORS,
METRICS_FATE_TYPE_IN_PROGRESS,
- METRICS_SCAN_BUSY_TIMEOUT_COUNTER,
METRICS_SCAN_RESERVATION_TOTAL_TIMER,
- METRICS_SCAN_RESERVATION_WRITEOUT_TIMER,
METRICS_SCAN_RESERVATION_CONFLICT_COUNTER,
- METRICS_SCAN_TABLET_METADATA_CACHE);
+ Set<String> flakyMetrics = Set.of(METRICS_FATE_TYPE_IN_PROGRESS,
+ METRICS_SCAN_BUSY_TIMEOUT_COUNTER,
+ METRICS_SCAN_RESERVATION_CONFLICT_COUNTER,
+ METRICS_SCAN_RESERVATION_TOTAL_TIMER,
+ METRICS_SCAN_RESERVATION_WRITEOUT_TIMER,
+ METRICS_SCAN_TABLET_METADATA_CACHE);
+ // @formatter:on
Map<String,String> expectedMetricNames = this.getMetricFields();
flakyMetrics.forEach(expectedMetricNames::remove); // might not see these
