This is an automated email from the ASF dual-hosted git repository.
mlbiscoc pushed a commit to branch feature/SOLR-17458
in repository https://gitbox.apache.org/repos/asf/solr.git
The following commit(s) were added to refs/heads/feature/SOLR-17458 by this
push:
new 90722410717 SOLR-17806: Recreate OTEL metric registries on core
rename/swapping (#3458)
90722410717 is described below
commit 907224107174b8ae1bd013d02851d0d51f6d5368
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 | 10 +-
.../solr/common/cloud/NodesSysPropsCacher.java | 3 +
.../solr/client/solrj/SolrJMetricTestUtils.java | 3 +-
.../solr/client/solrj/request/TestCoreAdmin.java | 68 ++++++++----
14 files changed, 320 insertions(+), 235 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 829e9875983..dbb96bf88a1 100644
--- a/solr/core/src/java/org/apache/solr/core/CoreContainer.java
+++ b/solr/core/src/java/org/apache/solr/core/CoreContainer.java
@@ -137,7 +137,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;
@@ -2329,18 +2328,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 77badcbf7b8..3a13af1c07b 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 3578ab5d318..3a235d93ca1 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
*
@@ -756,68 +756,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
@@ -1238,6 +1185,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 3559a665f91..c0e29f30a24 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;
@@ -35,6 +37,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;
@@ -169,6 +173,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 10704c19fe2..51ddf4c4735 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.HttpSolrClient;
@@ -35,48 +34,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 {
@@ -97,36 +67,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
@@ -134,12 +75,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
@@ -165,6 +101,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");
@@ -230,6 +167,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 b64f200e2df..c2ab65fed91 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
@@ -114,11 +114,15 @@ Users who that don't need to vary JAR access on a
per-core basis have several op
* The `addHttpRequestToContext` option in `solrconfig.xml` has been removed;
it's obsolete.
Nowadays, the HTTP request is available via internal APIs:
`SolrQueryRequest.getHttpSolrCall().getReq()`.
-* 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.
=== 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/.
\ No newline at end of file
+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
\ No newline at end of file
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 575c9edcf2a..79aae427fe9 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
