SOLR-18455: actually disable metrics on <metrics enabled="false"/>

changelog/unreleased/SOLR-18455-metrics-disabled.yml

@@ -0,0 +1,12 @@
+title: >
+  `<metrics enabled="false"/>` in `solr.xml` now actually disables the metrics facility: no metric
+  exporter or JVM runtime metrics are set up, and instruments are created from a no-op OpenTelemetry
+  meter, so registries no longer accumulate metrics (and the memory they retain) for a disabled node.
+type: fixed
+authors:
+  - name: Mikhail Khludnev
+    nick: mkhludnev
+    url: https://home.apache.org/phonebook.html?uid=mkhl
+links:
+  - name: SOLR-18455
+    url: https://issues.apache.org/jira/browse/SOLR-18455

solr/core/src/java/org/apache/solr/core/CoreContainer.java

@@ -407,7 +407,7 @@ public CoreContainer(NodeConfig config, CoresLocator locator, boolean asyncSolrC
     this.solrCores = SolrCores.newSolrCores(this);
     this.nodeKeyPair = new SolrNodeKeyPair(cfg.getCloudConfig());
     OpenTelemetryConfigurator.initializeOpenTelemetrySdk(cfg, loader);
-    this.metricManager = new SolrMetricManager(loader);
+    this.metricManager = new SolrMetricManager(loader, cfg.getMetricsConfig().isEnabled());
     this.tracer = TraceUtils.getGlobalTracer();
 
     containerHandlers.put(PublicKeyHandler.PATH, new PublicKeyHandler(nodeKeyPair));

solr/core/src/java/org/apache/solr/metrics/SolrMetricManager.java

@@ -36,6 +36,7 @@
 import io.opentelemetry.api.metrics.LongHistogramBuilder;
 import io.opentelemetry.api.metrics.LongUpDownCounter;
 import io.opentelemetry.api.metrics.LongUpDownCounterBuilder;
+import io.opentelemetry.api.metrics.MeterProvider;
 import io.opentelemetry.api.metrics.ObservableDoubleCounter;
 import io.opentelemetry.api.metrics.ObservableDoubleGauge;
 import io.opentelemetry.api.metrics.ObservableDoubleMeasurement;
@@ -123,6 +124,7 @@ public class SolrMetricManager {
       new ConcurrentHashMap<>();
 
   private final MetricExporter metricExporter;
+  private final boolean enabled;
   private OtelRuntimeJvmMetrics otelRuntimeJvmMetrics;
 
   private static final List<Double> SOLR_NANOSECOND_HISTOGRAM_BOUNDARIES =
@@ -143,12 +145,24 @@ public class SolrMetricManager {
           1_000_000_000.0);
 
   public SolrMetricManager(MetricExporter exporter) {
+    this(exporter, true);
+  }
+
+  public SolrMetricManager(MetricExporter exporter, boolean enabled) {
     metricExporter = exporter;
+    this.enabled = enabled;
   }
 
   public SolrMetricManager(SolrResourceLoader loader) {
-    this.metricExporter = loadMetricExporter(loader);
-    this.otelRuntimeJvmMetrics = new OtelRuntimeJvmMetrics().initialize(this, JVM_REGISTRY);
+    this(loader, true);
+  }
+
+  public SolrMetricManager(SolrResourceLoader loader, boolean enabled) {
+    this.enabled = enabled;
+    this.metricExporter = enabled ? loadMetricExporter(loader) : null;
+    if (enabled) {
+      this.otelRuntimeJvmMetrics = new OtelRuntimeJvmMetrics().initialize(this, JVM_REGISTRY);
+    }
   }
 
   public LongCounter longCounter(
@@ -429,9 +443,12 @@ public boolean hasRegistry(String name) {
    * Get (or create if not present) a named {@link SdkMeterProvider}.
    *
    * @param providerName name of the meter provider and prometheus metric reader
-   * @return existing or newly created meter provider
+   * @return existing or newly created meter provider, or a no-op one when metrics are disabled
    */
-  public SdkMeterProvider meterProvider(String providerName) {
+  public MeterProvider meterProvider(String providerName) {
+    if (!enabled) {
+      return MeterProvider.noop();
+    }
     providerName = enforcePrefix(providerName);
     return meterProviderAndReaders
         .computeIfAbsent(

solr/core/src/test/org/apache/solr/cloud/CloudExitableDirectoryReaderTest.java

@@ -72,6 +72,8 @@ public class CloudExitableDirectoryReaderTest extends SolrCloudTestCase {
 
   @BeforeClass
   public static void setupCluster() throws Exception {
+    // this test inspects node level request metrics
+    System.setProperty("metricsEnabled", "true");
     // create one more node than shard, so that we also test the case of proxied requests.
     MiniSolrCloudCluster.Builder clusterBuilder =
         configureCluster(3)

solr/core/src/test/org/apache/solr/cloud/TestPullReplica.java

@@ -90,6 +90,8 @@ private String suggestedCollectionName() {
 
   @BeforeClass
   public static void createTestCluster() throws Exception {
+    // this test inspects core level update metrics
+    System.setProperty("metricsEnabled", "true");
     System.setProperty("solr.solrj.cloud.max.stale.retries", "1");
     System.setProperty("zkReaderGetLeaderRetryTimeoutMs", "1000");
 

solr/core/src/test/org/apache/solr/cloud/TestPullReplicaWithAuth.java

@@ -53,6 +53,8 @@ public class TestPullReplicaWithAuth extends SolrCloudTestCase {
 
   @BeforeClass
   public static void setupClusterWithSecurityEnabled() throws Exception {
+    // this test inspects core level update metrics
+    System.setProperty("metricsEnabled", "true");
     configureCluster(2)
         .addConfig("conf", configset("cloud-minimal"))
         .withSecurityJson(SecurityJson.SIMPLE)

solr/core/src/test/org/apache/solr/metrics/SolrMetricsDisabledIntegrationTest.java

@@ -0,0 +1,84 @@
+/*
+ * 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.solr.metrics;
+
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.core.NodeConfig;
+import org.apache.solr.core.SolrCore;
+import org.apache.solr.core.SolrXmlConfig;
+import org.apache.solr.util.TestHarness;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+public class SolrMetricsDisabledIntegrationTest extends SolrTestCaseJ4 {
+  private CoreContainer cc;
+  private SolrMetricManager metricManager;
+
+  @Before
+  public void beforeTest() throws Exception {
+    Path home = TEST_PATH();
+    // SolrTestCaseJ4 installs SystemPropertiesRestoreRule as a method @Rule, so this is
+    // reverted after each test method; no manual save/restore needed.
+    System.setProperty("metricsEnabled", "false");
+
+    String solrXml = Files.readString(home.resolve("solr.xml"), StandardCharsets.UTF_8);
+    NodeConfig cfg = SolrXmlConfig.fromString(home, solrXml);
+    cc =
+        createCoreContainer(
+            cfg,
+            new TestHarness.TestCoresLocator(
+                DEFAULT_TEST_CORENAME,
+                initAndGetDataDir().toString(),
+                "solrconfig.xml",
+                "schema.xml"));
+    h.coreName = DEFAULT_TEST_CORENAME;
+    metricManager = cc.getMetricManager();
+  }
+
+  @After
+  public void afterTest() {
+    if (metricManager != null) {
+      deleteCore();
+    }
+  }
+
+  @Test
+  public void testMetricsDisabledPreventsNodeAndCoreRegistries() throws Exception {
+    assertFalse(cc.getConfig().getMetricsConfig().isEnabled());
+    assertTrue(metricManager.registryNames().isEmpty());
+    assertNull(metricManager.getPrometheusMetricReader("solr.node"));
+
+    try (SolrCore core = cc.getCore(DEFAULT_TEST_CORENAME)) {
+      assertNotNull(core);
+      assertNull(
+          metricManager.getPrometheusMetricReader(core.getCoreMetricManager().getRegistryName()));
+    }
+
+    assertQ(req("q", "*:*"), "//result[@numFound='0']");
+    assertU(adoc("id", "1"));
+    assertU(commit());
+
+    assertTrue(metricManager.registryNames().isEmpty());
+    assertNull(metricManager.getPrometheusMetricReader("solr.node"));
+  }
+}