diff --git a/changelog/unreleased/SOLR-18455-metrics-disabled.yml b/changelog/unreleased/SOLR-18455-metrics-disabled.yml new file mode 100644 index 00000000000..142738b4768 --- /dev/null +++ b/changelog/unreleased/SOLR-18455-metrics-disabled.yml @@ -0,0 +1,12 @@ +title: > + `` 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 diff --git a/solr/core/src/java/org/apache/solr/core/CoreContainer.java b/solr/core/src/java/org/apache/solr/core/CoreContainer.java index 64916561070..ef6852a14db 100644 --- a/solr/core/src/java/org/apache/solr/core/CoreContainer.java +++ b/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)); diff --git a/solr/core/src/java/org/apache/solr/metrics/SolrMetricManager.java b/solr/core/src/java/org/apache/solr/metrics/SolrMetricManager.java index 336610c536d..f68ee18b2e6 100644 --- a/solr/core/src/java/org/apache/solr/metrics/SolrMetricManager.java +++ b/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 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( diff --git a/solr/core/src/test/org/apache/solr/cloud/CloudExitableDirectoryReaderTest.java b/solr/core/src/test/org/apache/solr/cloud/CloudExitableDirectoryReaderTest.java index 28bd7dc9d4b..4cad3210323 100644 --- a/solr/core/src/test/org/apache/solr/cloud/CloudExitableDirectoryReaderTest.java +++ b/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) diff --git a/solr/core/src/test/org/apache/solr/cloud/TestPullReplica.java b/solr/core/src/test/org/apache/solr/cloud/TestPullReplica.java index d3e4c120b13..55254077e46 100644 --- a/solr/core/src/test/org/apache/solr/cloud/TestPullReplica.java +++ b/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"); diff --git a/solr/core/src/test/org/apache/solr/cloud/TestPullReplicaWithAuth.java b/solr/core/src/test/org/apache/solr/cloud/TestPullReplicaWithAuth.java index 8d3fea023d3..f79599103ce 100644 --- a/solr/core/src/test/org/apache/solr/cloud/TestPullReplicaWithAuth.java +++ b/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) diff --git a/solr/core/src/test/org/apache/solr/metrics/SolrMetricsDisabledIntegrationTest.java b/solr/core/src/test/org/apache/solr/metrics/SolrMetricsDisabledIntegrationTest.java new file mode 100644 index 00000000000..13e8d943503 --- /dev/null +++ b/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")); + } +}