This is an automated email from the ASF dual-hosted git repository. mlbiscoc pushed a commit to branch feature/SOLR-17458-rebased in repository https://gitbox.apache.org/repos/asf/solr.git
commit 09125e74c3641c0b0a4ade332d0351dc4efa20a8 Author: Matthew Biscocho <[email protected]> AuthorDate: Wed Aug 27 12:28:26 2025 -0400 SOLR-17806: Recreate OTEL metric registries on core rename/swapping (#3458) * Migrate core swapping and renaming * Broken tests * Cleanup from comments * Changes from review * Cleanup * Add comments --- .../java/org/apache/solr/core/CoreContainer.java | 9 +- .../src/java/org/apache/solr/core/SolrCore.java | 1 - .../src/java/org/apache/solr/core/SolrCores.java | 8 +- .../apache/solr/handler/admin/MetricsHandler.java | 2 +- .../apache/solr/metrics/SolrCoreMetricManager.java | 82 ++++++++++----- .../org/apache/solr/metrics/SolrMetricManager.java | 66 ++---------- .../solr/metrics/SolrCoreMetricManagerTest.java | 48 +++++++++ .../apache/solr/metrics/SolrMetricManagerTest.java | 42 -------- .../apache/solr/metrics/SolrMetricTestUtils.java | 114 +++++++++++++++++++++ .../solr/metrics/SolrMetricsIntegrationTest.java | 99 ++++++------------ .../pages/major-changes-in-solr-10.adoc | 8 +- .../solr/common/cloud/NodesSysPropsCacher.java | 3 + .../solr/client/solrj/SolrJMetricTestUtils.java | 3 +- .../solr/client/solrj/request/TestCoreAdmin.java | 68 ++++++++---- 14 files changed, 319 insertions(+), 234 deletions(-) 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 cd6bf740b36..808ee59f6bb 100644 --- a/solr/core/src/java/org/apache/solr/core/CoreContainer.java +++ b/solr/core/src/java/org/apache/solr/core/CoreContainer.java @@ -126,7 +126,6 @@ import org.apache.solr.jersey.InjectionFactories; import org.apache.solr.jersey.JerseyAppHandlerCache; import org.apache.solr.logging.LogWatcher; import org.apache.solr.logging.MDCLoggingContext; -import org.apache.solr.metrics.SolrCoreMetricManager; import org.apache.solr.metrics.SolrMetricManager; import org.apache.solr.metrics.SolrMetricProducer; import org.apache.solr.metrics.SolrMetricsContext; @@ -2224,18 +2223,18 @@ public class CoreContainer { SolrIdentifierValidator.validateCoreName(toName); try (SolrCore core = getCore(name)) { if (core != null) { - String oldRegistryName = core.getCoreMetricManager().getRegistryName(); - String newRegistryName = SolrCoreMetricManager.createRegistryName(core, toName); - metricManager.swapRegistries(oldRegistryName, newRegistryName); // The old coreDescriptor is obsolete, so remove it. registerCore will put it back. CoreDescriptor cd = core.getCoreDescriptor(); solrCores.removeCoreDescriptor(cd); cd.setProperty("name", toName); solrCores.addCoreDescriptor(cd); + // Before setting the new name, delete the old metrics registry then reregister metrics + // under the new core name + metricManager.removeRegistry(core.getCoreMetricManager().getRegistryName()); core.setName(toName); + core.getCoreMetricManager().reregisterCoreMetrics(); registerCore(cd, core, true, false); SolrCore old = solrCores.remove(name); - coresLocator.rename(this, old.getCoreDescriptor(), core.getCoreDescriptor()); } } diff --git a/solr/core/src/java/org/apache/solr/core/SolrCore.java b/solr/core/src/java/org/apache/solr/core/SolrCore.java index 50c40530937..5f1760673ab 100644 --- a/solr/core/src/java/org/apache/solr/core/SolrCore.java +++ b/solr/core/src/java/org/apache/solr/core/SolrCore.java @@ -554,7 +554,6 @@ public class SolrCore implements SolrInfoBean, Closeable { assert this.name != null; assert coreDescriptor.getCloudDescriptor() == null : "Cores are not renamed in SolrCloud"; this.name = Objects.requireNonNull(v); - coreMetricManager.afterCoreRename(); } /** diff --git a/solr/core/src/java/org/apache/solr/core/SolrCores.java b/solr/core/src/java/org/apache/solr/core/SolrCores.java index 39b1f68b155..8aef23b0555 100644 --- a/solr/core/src/java/org/apache/solr/core/SolrCores.java +++ b/solr/core/src/java/org/apache/solr/core/SolrCores.java @@ -257,12 +257,8 @@ public class SolrCores { cores.put(n1, c0); c0.setName(n1); c1.setName(n0); - - container - .getMetricManager() - .swapRegistries( - c0.getCoreMetricManager().getRegistryName(), - c1.getCoreMetricManager().getRegistryName()); + c0.getCoreMetricManager().reregisterCoreMetrics(); + c1.getCoreMetricManager().reregisterCoreMetrics(); } } diff --git a/solr/core/src/java/org/apache/solr/handler/admin/MetricsHandler.java b/solr/core/src/java/org/apache/solr/handler/admin/MetricsHandler.java index f55c0c4b177..633ccc7bb00 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/MetricsHandler.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/MetricsHandler.java @@ -294,7 +294,7 @@ public class MetricsHandler extends RequestHandlerBase implements PermissionName final String registryName = unescape(parts[0]); final String metricName = unescape(parts[1]); final String propertyName = parts.length > 2 ? unescape(parts[2]) : null; - if (!metricManager.hasRegistry(registryName)) { + if (!metricManager.hasDropwizardRegistry(registryName)) { errors.add(key, "registry '" + registryName + "' not found"); continue; } diff --git a/solr/core/src/java/org/apache/solr/metrics/SolrCoreMetricManager.java b/solr/core/src/java/org/apache/solr/metrics/SolrCoreMetricManager.java index 01122a779e0..6af631d6ebf 100644 --- a/solr/core/src/java/org/apache/solr/metrics/SolrCoreMetricManager.java +++ b/solr/core/src/java/org/apache/solr/metrics/SolrCoreMetricManager.java @@ -23,6 +23,8 @@ import io.opentelemetry.api.common.AttributeKey; import io.opentelemetry.api.common.Attributes; import java.io.Closeable; import java.io.IOException; +import java.util.ArrayList; +import java.util.List; import org.apache.solr.cloud.CloudDescriptor; import org.apache.solr.common.util.Utils; import org.apache.solr.core.CoreContainer; @@ -41,7 +43,6 @@ public class SolrCoreMetricManager implements Closeable { public static final AttributeKey<String> CORE_ATTR = AttributeKey.stringKey("core"); public static final AttributeKey<String> SHARD_ATTR = AttributeKey.stringKey("shard"); public static final AttributeKey<String> REPLICA_ATTR = AttributeKey.stringKey("replica"); - public static final AttributeKey<String> SCOPE_ATTR = AttributeKey.stringKey("scope"); private final SolrCore core; private SolrMetricsContext solrMetricsContext; @@ -52,6 +53,12 @@ public class SolrCoreMetricManager implements Closeable { private String leaderRegistryName; private boolean cloudMode; + // Track all metric producers registered for this core so we can re-initialize them during core + // rename + private final List<MetricProducerInfo> registeredProducers = new ArrayList<>(); + + private record MetricProducerInfo(SolrMetricProducer producer, String scope) {} + /** * Constructs a metric manager. * @@ -105,34 +112,46 @@ public class SolrCoreMetricManager implements Closeable { } /** - * Make sure that metrics already collected that correspond to the old core name are carried over - * and will be used under the new core name. This method also reloads reporters so that they use - * the new core name. + * Re-register all metric producers associated with this core. This recreates the metric registry + * resetting its state and recreating its attributes for all tracked registered producers. */ - // NOCOMMIT SOLR-17458: Update for core renaming - public void afterCoreRename() { - assert core.getCoreDescriptor().getCloudDescriptor() == null; - String oldRegistryName = solrMetricsContext.getRegistryName(); - String oldLeaderRegistryName = leaderRegistryName; - String newRegistryName = - createRegistryName(cloudMode, collectionName, shardName, replicaName, core.getName()); - leaderRegistryName = createLeaderRegistryName(cloudMode, collectionName, shardName); - if (oldRegistryName.equals(newRegistryName)) { - return; - } - // close old reporters - metricManager.closeReporters(oldRegistryName, solrMetricsContext.getTag()); - if (oldLeaderRegistryName != null) { - metricManager.closeReporters(oldLeaderRegistryName, solrMetricsContext.getTag()); - } - solrMetricsContext = - new SolrMetricsContext(metricManager, newRegistryName, solrMetricsContext.getTag()); - // load reporters again, using the new core name - loadReporters(); + public void reregisterCoreMetrics() { + this.solrMetricsContext = + new SolrMetricsContext( + metricManager, + createRegistryName(cloudMode, collectionName, shardName, replicaName, core.getName()), + solrMetricsContext.getTag()); + metricManager.removeRegistry(solrMetricsContext.getRegistryName()); + if (leaderRegistryName != null) metricManager.removeRegistry(leaderRegistryName); + + // TODO: We are going to recreate the attributes and re-initialize/reregister metrics from + // tracked producers. + // There is some possible improvement that can be done here to not have to duplicate code in + // registerMetricProducer + var attributes = + Attributes.builder() + .put(CORE_ATTR, core.getName()) + .put(COLLECTION_ATTR, collectionName) + .put(SHARD_ATTR, shardName) + .put(REPLICA_ATTR, replicaName) + .build(); + + core.initializeMetrics(solrMetricsContext, attributes, core.getName()); + + registeredProducers.forEach( + metricProducer -> { + var producerAttributes = attributes.toBuilder(); + if (metricProducer.scope().startsWith("/")) + producerAttributes.put(HANDLER_ATTR, metricProducer.scope); + metricProducer.producer.initializeMetrics( + solrMetricsContext, producerAttributes.build(), metricProducer.scope); + }); } /** - * Registers a mapping of name/metric's with the manager's metric registry. + * Registers a mapping of name/metric's with the manager's metric registry and creates the base + * set of attributes for core level metrics. All metric producers are tracked for re-registering + * in the case of core swapping/renaming * * @param scope the scope of the metrics to be registered (e.g. `/admin/ping`) * @param producer producer of metrics to be registered @@ -147,11 +166,16 @@ public class SolrCoreMetricManager implements Closeable { + producer); } - // NOCOMMIT SOLR-17458: These attributes may not work for standalone mode and maybe make the - // scope attribute optional + // Track this producer for potential re-initialization during core rename + registeredProducers.add(new MetricProducerInfo(producer, scope)); + + // TODO: We initialize metrics with attributes of the core. This happens again in + // reregisterCoreMetrics + // There is some possible improvement that can be done here to not have to duplicate code in + // reregisterCoreMetrics var attributesBuilder = Attributes.builder() - .put(CORE_ATTR, core.getCoreDescriptor().getName()) + .put(CORE_ATTR, core.getName()) .put(COLLECTION_ATTR, collectionName) .put(SHARD_ATTR, shardName) .put(REPLICA_ATTR, replicaName); @@ -179,6 +203,8 @@ public class SolrCoreMetricManager implements Closeable { } metricManager.unregisterGauges( solrMetricsContext.getRegistryName(), solrMetricsContext.getTag()); + + registeredProducers.clear(); } public SolrMetricsContext getSolrMetricsContext() { 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 367bc9da886..a950e1520fb 100644 --- a/solr/core/src/java/org/apache/solr/metrics/SolrMetricManager.java +++ b/solr/core/src/java/org/apache/solr/metrics/SolrMetricManager.java @@ -630,17 +630,17 @@ public class SolrMetricManager { * @param name registry name * @return true if this name points to a registry that already exists, false otherwise */ - // TODO SOLR-17458: We may not need public boolean hasRegistry(String name) { + return meterProviderAndReaders.containsKey(enforcePrefix(name)); + } + + // NOCOMMIT: Used in filtering. Will remove later + public boolean hasDropwizardRegistry(String name) { Set<String> names = registryNames(); name = enforcePrefix(name); return names.contains(name); } - public boolean hasMeterProvider(String name) { - return meterProviderAndReaders.containsKey(enforcePrefix(name)); - } - /** * Return set of existing registry names that match a regex pattern * @@ -772,68 +772,15 @@ public class SolrMetricManager { * * @param registry name of the registry to remove */ - // TODO SOLR-17458: You can't delete OTEL meters public void removeRegistry(String registry) { - // NOCOMMIT Remove all closing Dropwizard registries - // close any reporters for this registry first - closeReporters(registry, null); - // make sure we use a name with prefix - registry = enforcePrefix(registry); - if (isSharedRegistry(registry)) { - SharedMetricRegistries.remove(registry); - } else { - swapLock.lock(); - try { - registries.remove(registry); - } finally { - swapLock.unlock(); - } - } meterProviderAndReaders.computeIfPresent( - registry, + enforcePrefix(registry), (key, meterAndReader) -> { meterAndReader.sdkMeterProvider().close(); return null; }); } - /** - * Swap registries. This is useful eg. during {@link org.apache.solr.core.SolrCore} rename or swap - * operations. NOTE: this operation is not supported for shared registries. - * - * @param registry1 source registry - * @param registry2 target registry. Note: when used after core rename the target registry doesn't - * exist, so the swap operation will only rename the existing registry without creating an - * empty one under the previous name. - */ - // NOCOMMIT SOLR-17458: Don't think we need - public void swapRegistries(String registry1, String registry2) { - registry1 = enforcePrefix(registry1); - registry2 = enforcePrefix(registry2); - if (isSharedRegistry(registry1) || isSharedRegistry(registry2)) { - throw new UnsupportedOperationException( - "Cannot swap shared registry: " + registry1 + ", " + registry2); - } - swapLock.lock(); - try { - MetricRegistry from = registries.get(registry1); - MetricRegistry to = registries.get(registry2); - if (from == to) { - return; - } - MetricRegistry reg1 = registries.remove(registry1); - MetricRegistry reg2 = registries.remove(registry2); - if (reg2 != null) { - registries.put(registry1, reg2); - } - if (reg1 != null) { - registries.put(registry2, reg1); - } - } finally { - swapLock.unlock(); - } - } - /** * Potential conflict resolution strategies when attempting to register a new metric that already * exists @@ -1254,6 +1201,7 @@ public class SolrMetricManager { * @param group selected group, not null * @param registryNames optional child registry name elements */ + // NOCOMMIT: Come back and cleanup and remove all reporters code public void loadReporters( PluginInfo[] pluginInfos, SolrResourceLoader loader, diff --git a/solr/core/src/test/org/apache/solr/metrics/SolrCoreMetricManagerTest.java b/solr/core/src/test/org/apache/solr/metrics/SolrCoreMetricManagerTest.java index 365a997339f..93626397dc9 100644 --- a/solr/core/src/test/org/apache/solr/metrics/SolrCoreMetricManagerTest.java +++ b/solr/core/src/test/org/apache/solr/metrics/SolrCoreMetricManagerTest.java @@ -19,6 +19,8 @@ package org.apache.solr.metrics; import com.codahale.metrics.Counter; import com.codahale.metrics.Metric; import com.codahale.metrics.MetricRegistry; +import io.opentelemetry.api.common.AttributeKey; +import io.opentelemetry.api.common.Attributes; import java.io.IOException; import java.util.HashMap; import java.util.Map; @@ -43,6 +45,8 @@ import org.junit.After; import org.junit.Before; import org.junit.Test; +// NOCOMMIT: Need to fix up these tests to use the new SolrMetricTestUtils once we move off of +// Dropwizard public class SolrCoreMetricManagerTest extends SolrTestCaseJ4 { private static final int MAX_ITERATIONS = 100; @@ -177,6 +181,50 @@ public class SolrCoreMetricManagerTest extends SolrTestCaseJ4 { } } + @Test + public void testReregisterMetrics() { + Random random = random(); + + Map<String, Long> initialMetrics = + SolrMetricTestUtils.getRandomPrometheusMetricsWithReplacements(random, new HashMap<>()); + var initialProducer = new SolrMetricTestUtils.TestSolrMetricProducer(coreName, initialMetrics); + coreMetricManager.registerMetricProducer( + SolrMetricTestUtils.getRandomScope(random, true), initialProducer); + + var labels = SolrMetricTestUtils.newStandaloneLabelsBuilder(h.getCore()).build(); + + String randomMetricName = initialMetrics.entrySet().iterator().next().getKey(); + + long actualValue = + (long) + SolrMetricTestUtils.getCounterDatapoint(h.getCore(), randomMetricName, labels) + .getValue(); + long expectedValue = initialMetrics.get(randomMetricName); + + assertEquals(expectedValue, actualValue); + + // Change the metric value in OTEL + initialProducer + .getCounters() + .get(randomMetricName) + .add(10L, Attributes.of(AttributeKey.stringKey("core"), coreName)); + + long newActualValue = + (long) + SolrMetricTestUtils.getCounterDatapoint(h.getCore(), randomMetricName, labels) + .getValue(); + assertEquals(expectedValue + 10L, newActualValue); + + // Reregister the core metrics which should reset the metric value back to the initial value + coreMetricManager.reregisterCoreMetrics(); + + long reregisteredValue = + (long) + SolrMetricTestUtils.getCounterDatapoint(h.getCore(), randomMetricName, labels) + .getValue(); + assertEquals(expectedValue, reregisteredValue); + } + @Test public void testNonCloudRegistryName() { String registryName = h.getCore().getCoreMetricManager().getRegistryName(); diff --git a/solr/core/src/test/org/apache/solr/metrics/SolrMetricManagerTest.java b/solr/core/src/test/org/apache/solr/metrics/SolrMetricManagerTest.java index d5701d15111..1d324ab7de4 100644 --- a/solr/core/src/test/org/apache/solr/metrics/SolrMetricManagerTest.java +++ b/solr/core/src/test/org/apache/solr/metrics/SolrMetricManagerTest.java @@ -64,48 +64,6 @@ public class SolrMetricManagerTest extends SolrTestCaseJ4 { this.reader = metricManager.getPrometheusMetricReader(METER_PROVIDER_NAME); } - // NOCOMMIT: We might not be supported core swapping in 10. Maybe remove this test - @Test - public void testSwapRegistries() { - Random r = random(); - - SolrMetricManager metricManager = new SolrMetricManager(); - - Map<String, Counter> metrics1 = SolrMetricTestUtils.getRandomMetrics(r, true); - Map<String, Counter> metrics2 = SolrMetricTestUtils.getRandomMetrics(r, true); - String fromName = "from-" + TestUtil.randomSimpleString(r, 1, 10); - String toName = "to-" + TestUtil.randomSimpleString(r, 1, 10); - // register test metrics - for (Map.Entry<String, Counter> entry : metrics1.entrySet()) { - metricManager.registerMetric( - null, fromName, entry.getValue(), false, entry.getKey(), "metrics1"); - } - for (Map.Entry<String, Counter> entry : metrics2.entrySet()) { - metricManager.registerMetric( - null, toName, entry.getValue(), false, entry.getKey(), "metrics2"); - } - assertEquals(metrics1.size(), metricManager.registry(fromName).getMetrics().size()); - assertEquals(metrics2.size(), metricManager.registry(toName).getMetrics().size()); - - // swap - metricManager.swapRegistries(fromName, toName); - // check metrics - Map<String, Metric> fromMetrics = metricManager.registry(fromName).getMetrics(); - assertEquals(metrics2.size(), fromMetrics.size()); - for (Map.Entry<String, Counter> entry : metrics2.entrySet()) { - Object value = fromMetrics.get(SolrMetricManager.mkName(entry.getKey(), "metrics2")); - assertNotNull(value); - assertEquals(entry.getValue(), value); - } - Map<String, Metric> toMetrics = metricManager.registry(toName).getMetrics(); - assertEquals(metrics1.size(), toMetrics.size()); - for (Map.Entry<String, Counter> entry : metrics1.entrySet()) { - Object value = toMetrics.get(SolrMetricManager.mkName(entry.getKey(), "metrics1")); - assertNotNull(value); - assertEquals(entry.getValue(), value); - } - } - // NOCOMMIT: Migration of this to OTEL isn't possible. You can't register instruments to a // meterprovider that the provider itself didn't create @Test diff --git a/solr/core/src/test/org/apache/solr/metrics/SolrMetricTestUtils.java b/solr/core/src/test/org/apache/solr/metrics/SolrMetricTestUtils.java index f7878f19c8e..425aa45468e 100644 --- a/solr/core/src/test/org/apache/solr/metrics/SolrMetricTestUtils.java +++ b/solr/core/src/test/org/apache/solr/metrics/SolrMetricTestUtils.java @@ -17,6 +17,7 @@ package org.apache.solr.metrics; import com.codahale.metrics.Counter; +import io.opentelemetry.api.common.AttributeKey; import io.opentelemetry.api.common.Attributes; import io.opentelemetry.exporter.prometheus.PrometheusMetricReader; import io.prometheus.metrics.model.snapshots.CounterSnapshot; @@ -26,6 +27,7 @@ import io.prometheus.metrics.model.snapshots.HistogramSnapshot; import io.prometheus.metrics.model.snapshots.Labels; import java.util.ArrayList; import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.Random; import org.apache.lucene.tests.util.TestUtil; @@ -68,8 +70,40 @@ public final class SolrMetricTestUtils { return shouldDefineMetrics ? getRandomMetricsWithReplacements(random, new HashMap<>()) : null; } + /** + * Generate random OpenTelemetry metric names for testing Prometheus metrics. Returns a map of + * metric names to their expected increment values. + */ + public static Map<String, Long> getRandomPrometheusMetrics(Random random) { + return random.nextBoolean() + ? getRandomPrometheusMetricsWithReplacements(random, new HashMap<>()) + : null; + } + + public static Map<String, Long> getRandomPrometheusMetricsWithReplacements( + Random random, Map<String, Long> existing) { + HashMap<String, Long> metrics = new HashMap<>(); + List<String> existingKeys = List.copyOf(existing.keySet()); + + int numMetrics = TestUtil.nextInt(random, 1, MAX_ITERATIONS); + for (int i = 0; i < numMetrics; ++i) { + boolean shouldReplaceMetric = !existing.isEmpty() && random.nextBoolean(); + String name = + shouldReplaceMetric + ? existingKeys.get(TestUtil.nextInt(random, 0, existingKeys.size() - 1)) + : TestUtil.randomSimpleString(random, 5, 10) + + SUFFIX; // must be simple string for JMX publishing + + Long incrementValue = TestUtil.nextLong(random, 1L, 1000L); + metrics.put(name, incrementValue); + } + + return metrics; + } + public static final String SUFFIX = "_testing"; + // NOCOMMIT: This will get replaced by getRandomPrometheusMetricsWithReplacements public static Map<String, Counter> getRandomMetricsWithReplacements( Random random, Map<String, Counter> existing) { HashMap<String, Counter> metrics = new HashMap<>(); @@ -193,4 +227,84 @@ public final class SolrMetricTestUtils { return getDatapoint( core, metricName, labels, HistogramSnapshot.HistogramDataPointSnapshot.class); } + + public static CounterSnapshot.CounterDataPointSnapshot newStandaloneSelectRequestsDatapoint( + SolrCore core) { + return SolrMetricTestUtils.getCounterDatapoint( + core, + "solr_core_requests", + SolrMetricTestUtils.newStandaloneLabelsBuilder(core) + .label("handler", "/select") + .label("category", "QUERY") + .label("internal", "false") + .build()); + } + + public static CounterSnapshot.CounterDataPointSnapshot newCloudSelectRequestsDatapoint( + SolrCore core) { + return SolrMetricTestUtils.getCounterDatapoint( + core, + "solr_core_requests", + SolrMetricTestUtils.newCloudLabelsBuilder(core) + .label("handler", "/select") + .label("category", "QUERY") + .label("internal", "false") + .build()); + } + + public static CounterSnapshot.CounterDataPointSnapshot newStandaloneUpdateRequestsDatapoint( + SolrCore core) { + return SolrMetricTestUtils.getCounterDatapoint( + core, + "solr_core_requests", + SolrMetricTestUtils.newStandaloneLabelsBuilder(core) + .label("handler", "/update") + .label("category", "UPDATE") + .build()); + } + + public static CounterSnapshot.CounterDataPointSnapshot newCloudUpdateRequestsDatapoint( + SolrCore core) { + return SolrMetricTestUtils.getCounterDatapoint( + core, + "solr_core_requests", + SolrMetricTestUtils.newCloudLabelsBuilder(core) + .label("handler", "/update") + .label("category", "UPDATE") + .build()); + } + + public static class TestSolrMetricProducer implements SolrMetricProducer { + SolrMetricsContext solrMetricsContext; + private final Map<String, io.opentelemetry.api.metrics.LongCounter> counters = new HashMap<>(); + private final String coreName; + private final Map<String, Long> metrics; + + public TestSolrMetricProducer(String coreName, Map<String, Long> metrics) { + this.coreName = coreName; + this.metrics = metrics; + } + + @Override + public void initializeMetrics( + SolrMetricsContext parentContext, Attributes attributes, String scope) { + this.solrMetricsContext = parentContext.getChildContext(this); + for (Map.Entry<String, Long> entry : metrics.entrySet()) { + String metricName = entry.getKey(); + Long incrementValue = entry.getValue(); + var counter = solrMetricsContext.longCounter(metricName, "testing"); + counters.put(metricName, counter); + counter.add(incrementValue, Attributes.of(AttributeKey.stringKey("core"), coreName)); + } + } + + @Override + public SolrMetricsContext getSolrMetricsContext() { + return solrMetricsContext; + } + + public Map<String, io.opentelemetry.api.metrics.LongCounter> getCounters() { + return counters; + } + } } diff --git a/solr/core/src/test/org/apache/solr/metrics/SolrMetricsIntegrationTest.java b/solr/core/src/test/org/apache/solr/metrics/SolrMetricsIntegrationTest.java index b62f56745a8..117f02936c4 100644 --- a/solr/core/src/test/org/apache/solr/metrics/SolrMetricsIntegrationTest.java +++ b/solr/core/src/test/org/apache/solr/metrics/SolrMetricsIntegrationTest.java @@ -27,7 +27,6 @@ import java.util.List; import java.util.Map; import java.util.Set; import org.apache.http.client.HttpClient; -import org.apache.lucene.tests.util.LuceneTestCase; import org.apache.solr.SolrTestCaseJ4; import org.apache.solr.client.solrj.SolrClient; import org.apache.solr.client.solrj.impl.HttpClientUtil; @@ -36,48 +35,19 @@ import org.apache.solr.cloud.MiniSolrCloudCluster; import org.apache.solr.common.util.Utils; import org.apache.solr.core.CoreContainer; import org.apache.solr.core.NodeConfig; -import org.apache.solr.core.PluginInfo; +import org.apache.solr.core.SolrCore; import org.apache.solr.core.SolrInfoBean; import org.apache.solr.core.SolrXmlConfig; import org.apache.solr.embedded.JettySolrRunner; -import org.apache.solr.util.JmxUtil; import org.apache.solr.util.TestHarness; import org.hamcrest.number.OrderingComparison; import org.junit.After; import org.junit.Before; import org.junit.Test; -// NOCOMMIT: Test fails because of the @After assert on index path. Was going to migrate to just -// check the registry for the core is deleted but this test does a rename operation which otel has -// not addressed yet. Need to migrate the rename operation to otel first. [email protected](bugUrl = "https://issues.apache.org/jira/browse/SOLR-17458";) public class SolrMetricsIntegrationTest extends SolrTestCaseJ4 { - private static final int MAX_ITERATIONS = 20; - private static final String CORE_NAME = "metrics_integration"; - private static final String METRIC_NAME = "requestTimes"; - private static final String HANDLER_NAME = "/select"; - private static final String[] REPORTER_NAMES = {"reporter1", "reporter2"}; - private static final String UNIVERSAL = "universal"; - private static final String SPECIFIC = "specific"; - private static final String MULTIGROUP = "multigroup"; - private static final String MULTIREGISTRY = "multiregistry"; - private static final String[] INITIAL_REPORTERS = { - REPORTER_NAMES[0], REPORTER_NAMES[1], UNIVERSAL, SPECIFIC, MULTIGROUP, MULTIREGISTRY - }; - private static final String[] RENAMED_REPORTERS = { - REPORTER_NAMES[0], REPORTER_NAMES[1], UNIVERSAL, MULTIGROUP - }; - private static final SolrInfoBean.Category HANDLER_CATEGORY = SolrInfoBean.Category.QUERY; - private CoreContainer cc; private SolrMetricManager metricManager; - private String tag; - private int jmxReporter; - - private void assertTagged(Map<String, SolrMetricReporter> reporters, String name) { - assertTrue( - "Reporter '" + name + "' missing in " + reporters, reporters.containsKey(name + "@" + tag)); - } @Before public void beforeTest() throws Exception { @@ -98,36 +68,7 @@ public class SolrMetricsIntegrationTest extends SolrTestCaseJ4 { "schema.xml")); h.coreName = DEFAULT_TEST_CORENAME; - jmxReporter = JmxUtil.findFirstMBeanServer() != null ? 1 : 0; - metricManager = cc.getMetricManager(); - tag = h.getCore().getCoreMetricManager().getTag(); - // initially there are more reporters, because two of them are added via a matching collection - // name - Map<String, SolrMetricReporter> reporters = - metricManager.getReporters("solr.core." + DEFAULT_TEST_CORENAME); - assertEquals(INITIAL_REPORTERS.length + jmxReporter, reporters.size()); - for (String r : INITIAL_REPORTERS) { - assertTagged(reporters, r); - } - // test rename operation - cc.rename(DEFAULT_TEST_CORENAME, CORE_NAME); - h.coreName = CORE_NAME; - cfg = cc.getConfig(); - PluginInfo[] plugins = cfg.getMetricsConfig().getMetricReporters(); - assertNotNull(plugins); - assertEquals(10 + jmxReporter, plugins.length); - reporters = metricManager.getReporters("solr.node"); - assertEquals(4 + jmxReporter, reporters.size()); - assertTrue( - "Reporter '" + REPORTER_NAMES[0] + "' missing in solr.node", - reporters.containsKey(REPORTER_NAMES[0])); - assertTrue( - "Reporter '" + UNIVERSAL + "' missing in solr.node", reporters.containsKey(UNIVERSAL)); - assertTrue( - "Reporter '" + MULTIGROUP + "' missing in solr.node", reporters.containsKey(MULTIGROUP)); - assertTrue( - "Reporter '" + MULTIREGISTRY + "' missing in solr.node", - reporters.containsKey(MULTIREGISTRY)); + metricManager = h.getCore().getCoreContainer().getMetricManager(); } @After @@ -135,12 +76,7 @@ public class SolrMetricsIntegrationTest extends SolrTestCaseJ4 { if (null == metricManager) { return; // test failed to init, nothing to clean up } - - SolrCoreMetricManager coreMetricManager = h.getCore().getCoreMetricManager(); - Gauge<?> gauge = (Gauge<?>) coreMetricManager.getRegistry().getMetrics().get("CORE.indexDir"); - assertNotNull(gauge.getValue()); deleteCore(); // closes TestHarness which closes CoreContainer which closes SolrCore - assertEquals(metricManager.nullString(), gauge.getValue()); } @Test @@ -166,6 +102,7 @@ public class SolrMetricsIntegrationTest extends SolrTestCaseJ4 { assertEquals(g.getValue(), cc.getSolrHome().toString()); } + // NOCOMMIT: Comeback and fix this test after merging the SolrZKClient metrics migration @Test public void testZkMetrics() throws Exception { System.setProperty("metricsEnabled", "true"); @@ -231,6 +168,36 @@ public class SolrMetricsIntegrationTest extends SolrTestCaseJ4 { } } + @Test + public void testCoreRename() { + String newCoreName = "renamed_core"; + String originalRegistryName; + + try (SolrCore core = cc.getCore(DEFAULT_TEST_CORENAME)) { + originalRegistryName = core.getCoreMetricManager().getRegistryName(); + assertTrue("Original registry should exist", metricManager.hasRegistry(originalRegistryName)); + assertQ(req("q", "*:*"), "//result[@numFound='0']"); + assertEquals( + 1.0, SolrMetricTestUtils.newStandaloneSelectRequestsDatapoint(core).getValue(), 0.0); + } + + cc.rename(DEFAULT_TEST_CORENAME, newCoreName); + h.coreName = newCoreName; + + try (SolrCore core = cc.getCore(newCoreName)) { + assertFalse( + "Original registry should not exist", metricManager.hasRegistry(originalRegistryName)); + assertTrue( + "Renamed registry should exist", + metricManager.hasRegistry(core.getCoreMetricManager().getRegistryName())); + assertQ(req("q", "*:*"), "//result[@numFound='0']"); + assertEquals( + 1.0, + SolrMetricTestUtils.newStandaloneSelectRequestsDatapoint(h.getCore()).getValue(), + 0.0); + } + } + private long findDelta(Map<String, Object> m1, Map<String, Object> m2, String k) { return ((Number) m2.get(k)).longValue() - ((Number) m1.get(k)).longValue(); } diff --git a/solr/solr-ref-guide/modules/upgrade-notes/pages/major-changes-in-solr-10.adoc b/solr/solr-ref-guide/modules/upgrade-notes/pages/major-changes-in-solr-10.adoc index 5b3b74e53df..004539aa1aa 100644 --- a/solr/solr-ref-guide/modules/upgrade-notes/pages/major-changes-in-solr-10.adoc +++ b/solr/solr-ref-guide/modules/upgrade-notes/pages/major-changes-in-solr-10.adoc @@ -168,8 +168,6 @@ Nowadays, the HTTP request is available via internal APIs: `SolrQueryRequest.get * BlobHandler and the `.system` collection have been removed in favour of FileStore API. (SOLR-17851). -* JMX, SLF4J and Graphite metric reporters have been removed. Users should migrate to using OTLP or the /admin/metrics endpoint with external tools to get metrics to their preferred backend. - === Security * There is no longer a distinction between trusted and untrusted configSets; all configSets are now considered trusted. To ensure security, Solr should be properly protected using authentication and authorization mechanisms, allowing only authorized users with administrative privileges to publish them. @@ -177,6 +175,12 @@ Nowadays, the HTTP request is available via internal APIs: `SolrQueryRequest.get === Upgrade to Jetty 12.x Solr upgraded to Jetty 12.x from 10.x as Jetty 10 and 11 have reached end-of-life support. Jetty 12.x requires Java 17 or newer and is fully compatible with Solr's new minimum requirement of Java 21. This upgrade brings support for modern HTTP protocols and adopts the Jakarta EE 10 namespace. For more details, see https://webtide.com/jetty-12-has-arrived/. +=== Open Telemetry + +* JMX, SLF4J and Graphite metric reporters have been removed. Users should migrate to using OTLP or the /admin/metrics endpoint with external tools to get metrics to their preferred backend. + +* Core renaming and swapping will reset the state of all the corresponding cores metrics + === Docker The OS version of the official Docker image and provided Dockerfile has been upgraded to Ubuntu 24 (noble) from Ubuntu 22 (jammy). diff --git a/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/NodesSysPropsCacher.java b/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/NodesSysPropsCacher.java index 35d6829f7cb..4e79f905972 100644 --- a/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/NodesSysPropsCacher.java +++ b/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/NodesSysPropsCacher.java @@ -74,6 +74,9 @@ public class NodesSysPropsCacher implements NodesSysProps, AutoCloseable { return result; } + // NOCOMMIT: These properties were fetched from the /admin/metrics endpoint. These properties were + // stored as strings instead of numeric values. This is not possible in OTEL metrics. + // Use /admin/info/properties to fetch system properties. private Map<String, Object> fetchProps(String nodeName, Collection<String> tags) { ModifiableSolrParams msp = new ModifiableSolrParams(); msp.add(CommonParams.OMIT_HEADER, "true"); diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/SolrJMetricTestUtils.java b/solr/solrj/src/test/org/apache/solr/client/solrj/SolrJMetricTestUtils.java index 755be090eac..d48f79cc55a 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/SolrJMetricTestUtils.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/SolrJMetricTestUtils.java @@ -20,7 +20,6 @@ package org.apache.solr.client.solrj; import java.io.IOException; import java.io.InputStream; import java.nio.charset.StandardCharsets; -import org.apache.solr.client.solrj.impl.CloudSolrClient; import org.apache.solr.client.solrj.impl.Http2SolrClient; import org.apache.solr.client.solrj.impl.InputStreamResponseParser; import org.apache.solr.client.solrj.request.GenericSolrRequest; @@ -29,7 +28,7 @@ import org.apache.solr.common.util.NamedList; public final class SolrJMetricTestUtils { - public static double getPrometheusMetricValue(CloudSolrClient solrClient, String metricName) + public static double getPrometheusMetricValue(SolrClient solrClient, String metricName) throws SolrServerException, IOException { var req = new GenericSolrRequest( diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/request/TestCoreAdmin.java b/solr/solrj/src/test/org/apache/solr/client/solrj/request/TestCoreAdmin.java index 980a326a32f..7cb86c03949 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/request/TestCoreAdmin.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/request/TestCoreAdmin.java @@ -16,10 +16,10 @@ */ package org.apache.solr.client.solrj.request; +import static org.apache.solr.client.solrj.SolrJMetricTestUtils.getPrometheusMetricValue; import static org.hamcrest.CoreMatchers.notNullValue; import static org.hamcrest.core.Is.is; -import com.codahale.metrics.MetricRegistry; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; @@ -40,8 +40,6 @@ import org.apache.solr.common.SolrInputDocument; import org.apache.solr.common.params.ModifiableSolrParams; import org.apache.solr.core.CoreContainer; import org.apache.solr.core.SolrCore; -import org.apache.solr.metrics.SolrCoreMetricManager; -import org.apache.solr.metrics.SolrMetricManager; import org.junit.Test; public class TestCoreAdmin extends AbstractEmbeddedSolrServerTestCase { @@ -168,9 +166,7 @@ public class TestCoreAdmin extends AbstractEmbeddedSolrServerTestCase { assertEquals(names.size(), cores.getNumAllCores()); } - // NOCOMMIT: We have not yet implemented core swapping or renaming for OTEL yet @Test - @BadApple(bugUrl = "https://issues.apache.org/jira/browse/SOLR-17458";) public void testCoreSwap() throws Exception { // index marker docs to core0 SolrClient cli0 = getSolrCore0(); @@ -203,19 +199,20 @@ public class TestCoreAdmin extends AbstractEmbeddedSolrServerTestCase { assertTrue(doc.toString(), doc.getFieldValue("id").toString().startsWith("core1-")); }); - // assert initial metrics - SolrMetricManager metricManager = cores.getMetricManager(); - String core0RegistryName = - SolrCoreMetricManager.createRegistryName(false, null, null, null, "core0"); - String core1RegistryName = - SolrCoreMetricManager.createRegistryName(false, null, null, null, "core1"); - MetricRegistry core0Registry = metricManager.registry(core0RegistryName); - MetricRegistry core1Registry = metricManager.registry(core1RegistryName); - - // 2 docs + 1 commit - assertEquals(3, core0Registry.counter("UPDATE./update.requests").getCount()); - // 1 doc + 1 commit - assertEquals(2, core1Registry.counter("UPDATE./update.requests").getCount()); + // Check initial request counts + Double core0InitialRequests = + getPrometheusMetricValue( + getSolrAdmin(), + "solr_core_requests_total{category=\"UPDATE\",core=\"core0\",handler=\"/update\",otel_scope_name=\"org.apache.solr\"}"); + Double core1InitialRequests = + getPrometheusMetricValue( + getSolrAdmin(), + "solr_core_requests_total{category=\"UPDATE\",core=\"core1\",handler=\"/update\",otel_scope_name=\"org.apache.solr\"}"); + + // 2 docs + 1 commit = 3 requests for core0 + assertEquals("Initial core0 update requests", 3.0, core0InitialRequests, 0.1); + // 1 doc + 1 commit = 2 requests for core1 + assertEquals("Initial core1 update requests", 2.0, core1InitialRequests, 0.1); // swap CoreAdminRequest.swapCore("core0", "core1", getSolrAdmin()); @@ -240,11 +237,38 @@ public class TestCoreAdmin extends AbstractEmbeddedSolrServerTestCase { assertTrue(doc.toString(), doc.getFieldValue("id").toString().startsWith("core0-")); }); - core0Registry = metricManager.registry(core0RegistryName); - core1Registry = metricManager.registry(core1RegistryName); + Double core0PostSwapRequests = + getPrometheusMetricValue( + getSolrAdmin(), + "solr_core_requests_total{category=\"UPDATE\",core=\"core0\",handler=\"/update\",otel_scope_name=\"org.apache.solr\"}"); + Double core1PostSwapRequests = + getPrometheusMetricValue( + getSolrAdmin(), + "solr_core_requests_total{category=\"UPDATE\",core=\"core1\",handler=\"/update\",otel_scope_name=\"org.apache.solr\"}"); + + // Both cores should have reset metrics after swap (registries recreated) + assertEquals("core0 requests should be reset after swap", 0.0, core0PostSwapRequests, 0.1); + assertEquals("core1 requests should be reset after swap", 0.0, core1PostSwapRequests, 0.1); + + // Index documents after swap and assert request counts again + cli0.add(d); + d = new SolrInputDocument("id", "core0-1"); + cli0.add(d); + cli0.commit(); + d = new SolrInputDocument("id", "core1-0"); + cli1.add(d); + cli1.commit(); - assertEquals(2, core0Registry.counter("UPDATE./update.requests").getCount()); - assertEquals(3, core1Registry.counter("UPDATE./update.requests").getCount()); + core0PostSwapRequests = + getPrometheusMetricValue( + getSolrAdmin(), + "solr_core_requests_total{category=\"UPDATE\",core=\"core0\",handler=\"/update\",otel_scope_name=\"org.apache.solr\"}"); + core1PostSwapRequests = + getPrometheusMetricValue( + getSolrAdmin(), + "solr_core_requests_total{category=\"UPDATE\",core=\"core1\",handler=\"/update\",otel_scope_name=\"org.apache.solr\"}"); + assertEquals("Initial core0 update requests", 3.0, core0PostSwapRequests, 0.1); + assertEquals("Initial core1 update requests", 2.0, core1PostSwapRequests, 0.1); } @Test
