diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/ServerMetrics.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/ServerMetrics.java index dab8446472e..d189a8785fc 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/ServerMetrics.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/ServerMetrics.java @@ -25,7 +25,6 @@ import org.apache.zookeeper.metrics.MetricsProvider; import org.apache.zookeeper.metrics.Summary; import org.apache.zookeeper.metrics.SummarySet; -import org.apache.zookeeper.metrics.impl.DefaultMetricsProvider; import org.apache.zookeeper.metrics.impl.NullMetricsProvider; import org.apache.zookeeper.server.util.QuotaMetricsUtils; import org.slf4j.Logger; @@ -40,16 +39,11 @@ public final class ServerMetrics { */ public static final ServerMetrics NULL_METRICS = new ServerMetrics(NullMetricsProvider.INSTANCE); - /** - * Dummy instance useful for tests. - */ - public static final ServerMetrics DEFAULT_METRICS_FOR_TESTS = new ServerMetrics(new DefaultMetricsProvider()); - /** * Real instance used for tracking server side metrics. The final value is * assigned after the {@link MetricsProvider} bootstrap. */ - private static volatile ServerMetrics CURRENT = DEFAULT_METRICS_FOR_TESTS; + private static volatile ServerMetrics CURRENT = NULL_METRICS; /** * Access current ServerMetrics. diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/DropwizardMetricsOptionalArchTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/DropwizardMetricsOptionalArchTest.java new file mode 100644 index 00000000000..486b5a434c3 --- /dev/null +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/DropwizardMetricsOptionalArchTest.java @@ -0,0 +1,56 @@ +/* + * 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.zookeeper; + +import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.noClasses; +import com.tngtech.archunit.base.DescribedPredicate; +import com.tngtech.archunit.core.domain.JavaClass; +import com.tngtech.archunit.core.domain.JavaClasses; +import com.tngtech.archunit.core.importer.ClassFileImporter; +import com.tngtech.archunit.core.importer.ImportOption; +import com.tngtech.archunit.lang.ArchRule; +import java.util.Collections; +import org.junit.jupiter.api.Test; + +/** + * Architectural test to enforce that Dropwizard metrics is an optional dependency. + * + *

Only {@link org.apache.zookeeper.server.metric.AvgMinMaxPercentileCounter} may depend + * on {@code com.codahale.metrics} packages. All other ZooKeeper classes must remain Dropwizard-free so that it can be + * an optional dependency. + */ +public class DropwizardMetricsOptionalArchTest { + + @Test + public void onlyAvgMinMaxPercentileCounterShouldDependOnDropwizard() { + JavaClasses importedClasses = + new ClassFileImporter(Collections.singletonList(new ImportOption.DoNotIncludeTests())).importPackages( + "org.apache.zookeeper").that(new DescribedPredicate("ZK non-Dropwizard classes") { + @Override + public boolean test(JavaClass javaClass) { + return !javaClass.getName().contains("AvgMinMaxPercentileCounter"); + } + }); + + ArchRule rule = noClasses().should().dependOnClassesThat().resideInAnyPackage("com.codahale.."); + + rule.check(importedClasses); + } + +} diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/ZKTestCase.java b/zookeeper-server/src/test/java/org/apache/zookeeper/ZKTestCase.java index db24797180b..7b2b8e72a5e 100644 --- a/zookeeper-server/src/test/java/org/apache/zookeeper/ZKTestCase.java +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/ZKTestCase.java @@ -24,6 +24,8 @@ import java.time.Instant; import java.util.concurrent.CompletableFuture; import org.apache.zookeeper.metrics.MetricsUtils; +import org.apache.zookeeper.metrics.impl.DefaultMetricsProvider; +import org.apache.zookeeper.server.ServerMetrics; import org.apache.zookeeper.util.ServiceUtils; import org.hamcrest.CustomMatcher; import org.hamcrest.Description; @@ -93,6 +95,8 @@ public static void before() { @BeforeEach public void starting(TestInfo testInfo) { + ServerMetrics.metricsProviderInitialized(new DefaultMetricsProvider()); + // By default, disable starting a JettyAdminServer in tests to avoid // accidentally attempting to start multiple admin servers on the // same port. diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/LearnerHandlerMetricsTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/LearnerHandlerMetricsTest.java index 3b15e0cca35..b084bf2fe9b 100644 --- a/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/LearnerHandlerMetricsTest.java +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/LearnerHandlerMetricsTest.java @@ -35,6 +35,7 @@ import java.util.concurrent.TimeUnit; import org.apache.jute.BinaryOutputArchive; import org.apache.zookeeper.metrics.MetricsUtils; +import org.apache.zookeeper.metrics.impl.DefaultMetricsProvider; import org.apache.zookeeper.server.ServerMetrics; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -55,6 +56,8 @@ class MockLearnerHandler extends LearnerHandler { @BeforeEach public void setup() throws IOException { + ServerMetrics.metricsProviderInitialized(new DefaultMetricsProvider()); + Leader leader = mock(Leader.class); when(leader.getQuorumAuthServer()).thenReturn(null); diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/SyncRequestProcessorMetricTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/SyncRequestProcessorMetricTest.java index 8ca33f798c7..e3f05fd18a9 100644 --- a/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/SyncRequestProcessorMetricTest.java +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/SyncRequestProcessorMetricTest.java @@ -31,9 +31,11 @@ import java.util.concurrent.TimeUnit; import org.apache.zookeeper.ZooDefs; import org.apache.zookeeper.metrics.MetricsUtils; +import org.apache.zookeeper.metrics.impl.DefaultMetricsProvider; import org.apache.zookeeper.server.Request; import org.apache.zookeeper.server.RequestProcessor; import org.apache.zookeeper.server.RequestRecord; +import org.apache.zookeeper.server.ServerMetrics; import org.apache.zookeeper.server.SyncRequestProcessor; import org.apache.zookeeper.server.ZKDatabase; import org.apache.zookeeper.server.ZooKeeperServer; @@ -48,6 +50,8 @@ public class SyncRequestProcessorMetricTest { @BeforeEach public void setup() throws Exception { + ServerMetrics.metricsProviderInitialized(new DefaultMetricsProvider()); + ZKDatabase db = mock(ZKDatabase.class); when(db.append(any(Request.class))).thenReturn(true); doAnswer(invocation -> {