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 bee4a36db5b5b8f3bbae53d67eb1c991bede01dc Author: Matthew Biscocho <[email protected]> AuthorDate: Wed Aug 6 10:31:39 2025 -0400 SOLR-17806: Migrate ADMIN node registry metrics to OTEL (#3444) * Migrate admin node metrics to OTEL * Cleanup * Fix up some tests * Move some things around * Bad apple test mbeans handler * mapToDouble * Skip whitespace trimming and skipping --- .../java/org/apache/solr/core/CoreContainer.java | 40 +++--- .../apache/solr/handler/RequestHandlerBase.java | 141 +++++++++------------ .../solr/handler/component/SearchHandler.java | 17 +-- .../apache/solr/jersey/RequestMetricHandling.java | 12 +- .../apache/solr/metrics/SolrCoreMetricManager.java | 3 +- .../apache/solr/metrics/SolrMetricProducer.java | 1 + .../solr/cloud/TestRandomRequestDistribution.java | 59 +++++---- .../org/apache/solr/core/RequestHandlersTest.java | 31 +++-- .../solr/handler/RequestHandlerBaseTest.java | 5 +- .../solr/handler/RequestHandlerMetricsTest.java | 3 + .../solr/handler/admin/MBeansHandlerTest.java | 4 + .../solr/handler/admin/MetricsHandlerTest.java | 4 + .../TestPrometheusResponseWriterCloud.java | 42 ++---- .../solr/client/solrj/SolrJMetricTestUtils.java | 117 +++++++++++++++++ .../solrj/impl/CloudHttp2SolrClientRetryTest.java | 30 +---- .../solrj/impl/CloudHttp2SolrClientTest.java | 105 ++++----------- .../solrj/impl/CloudSolrClientRetryTest.java | 32 +---- .../client/solrj/impl/CloudSolrClientTest.java | 98 ++++---------- .../solr/client/solrj/request/TestCoreAdmin.java | 2 + 19 files changed, 360 insertions(+), 386 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 02b63fe0c16..59e5fe8c561 100644 --- a/solr/core/src/java/org/apache/solr/core/CoreContainer.java +++ b/solr/core/src/java/org/apache/solr/core/CoreContainer.java @@ -26,6 +26,7 @@ import static org.apache.solr.common.params.CommonParams.INFO_HANDLER_PATH; import static org.apache.solr.common.params.CommonParams.METRICS_PATH; import static org.apache.solr.common.params.CommonParams.ZK_PATH; import static org.apache.solr.common.params.CommonParams.ZK_STATUS_PATH; +import static org.apache.solr.metrics.SolrMetricProducer.HANDLER_ATTR; import static org.apache.solr.search.SolrIndexSearcher.EXECUTOR_MAX_CPU_THREADS; import static org.apache.solr.security.AuthenticationPlugin.AUTHENTICATION_PLUGIN_PROP; @@ -515,9 +516,10 @@ public class CoreContainer { newVersion, getResourceLoader().newInstance(klas, AuditLoggerPlugin.class)); newAuditloggerPlugin.plugin.init(auditConf); - // TODO SOLR-17458: Add Otel newAuditloggerPlugin.plugin.initializeMetrics( - solrMetricsContext, Attributes.empty(), "/auditlogging"); + solrMetricsContext, + Attributes.builder().put(HANDLER_ATTR, "/auditlogging").build(), + "/auditlogging"); } else { log.debug("Security conf doesn't exist. Skipping setup for audit logging module."); } @@ -582,9 +584,10 @@ public class CoreContainer { if (authenticationPlugin != null) { authenticationPlugin.plugin.init(authenticationConfig); setupHttpClientForAuthPlugin(authenticationPlugin.plugin); - // TODO SOLR-17458: Add Otel authenticationPlugin.plugin.initializeMetrics( - solrMetricsContext, Attributes.empty(), "/authentication"); + solrMetricsContext, + Attributes.builder().put(HANDLER_ATTR, "/authentication").build(), + "/authentication"); } this.authenticationPlugin = authenticationPlugin; try { @@ -779,14 +782,14 @@ public class CoreContainer { shardHandlerFactory = ShardHandlerFactory.newInstance(cfg.getShardHandlerFactoryPluginInfo(), loader); if (shardHandlerFactory instanceof SolrMetricProducer metricProducer) { - // TODO SOLR-17458: Add Otel + // NOCOMMIT SOLR-17458: Add Otel metricProducer.initializeMetrics(solrMetricsContext, Attributes.empty(), "httpShardHandler"); } updateShardHandler = new UpdateShardHandler(cfg.getUpdateShardHandlerConfig()); solrClientProvider = new HttpSolrClientProvider(cfg.getUpdateShardHandlerConfig(), solrMetricsContext); - // TODO SOLR-17458: Add Otel + // NOCOMMIT SOLR-17458: Add Otel updateShardHandler.initializeMetrics( solrMetricsContext, Attributes.empty(), "updateShardHandler"); solrClientCache = new SolrClientCache(solrClientProvider.getSolrClient()); @@ -799,7 +802,7 @@ public class CoreContainer { for (Map.Entry<String, CacheConfig> e : cachesConfig.entrySet()) { SolrCache<?, ?> c = e.getValue().newInstance(); String cacheName = e.getKey(); - // TODO SOLR-17458: Add Otel + // NOCOMMIT SOLR-17458: Add Otel c.initializeMetrics(solrMetricsContext, Attributes.empty(), "nodeLevelCache/" + cacheName); m.put(cacheName, c); } @@ -814,7 +817,7 @@ public class CoreContainer { if (isZooKeeperAware()) { solrClientCache.setDefaultZKHost(getZkController().getZkServerAddress()); // initialize ZkClient metrics - // TODO SOLR-17458: Add Otel + // NOCOMMIT SOLR-17458: Add Otel zkSys .getZkMetricsProducer() .initializeMetrics(solrMetricsContext, Attributes.empty(), "zkClient"); @@ -823,9 +826,11 @@ public class CoreContainer { this, zkSys.getZkController().getNodeName(), (PublicKeyHandler) containerHandlers.get(PublicKeyHandler.PATH)); - // TODO SOLR-17458: Add Otel + // NOCOMMIT SOLR-17458: AuthenticationPlugin.java pkiAuthenticationSecurityBuilder.initializeMetrics( - solrMetricsContext, Attributes.empty(), "/authentication/pki"); + solrMetricsContext, + Attributes.builder().put(HANDLER_ATTR, "/authentication/pki").build(), + "/authentication/pki"); fileStore = new DistribFileStore(this); registerV2ApiIfEnabled(ClusterFileStore.class); @@ -890,11 +895,15 @@ public class CoreContainer { metricsHandler = new MetricsHandler(this); containerHandlers.put(METRICS_PATH, metricsHandler); // TODO SOLR-17458: Add Otel - metricsHandler.initializeMetrics(solrMetricsContext, Attributes.empty(), METRICS_PATH); + metricsHandler.initializeMetrics( + solrMetricsContext, + Attributes.builder().put(HANDLER_ATTR, METRICS_PATH).build(), + METRICS_PATH); containerHandlers.put(AUTHZ_PATH, securityConfHandler); // TODO SOLR-17458: Add Otel - securityConfHandler.initializeMetrics(solrMetricsContext, Attributes.empty(), AUTHZ_PATH); + securityConfHandler.initializeMetrics( + solrMetricsContext, Attributes.builder().put(HANDLER_ATTR, AUTHZ_PATH).build(), AUTHZ_PATH); containerHandlers.put(AUTHC_PATH, securityConfHandler); PluginInfo[] metricReporters = cfg.getMetricsConfig().getMetricReporters(); @@ -1016,7 +1025,7 @@ public class CoreContainer { "version"); SolrFieldCacheBean fieldCacheBean = new SolrFieldCacheBean(); - // TODO SOLR-17458: Otel migration + // NOCOMMIT SOLR-17458: Otel migration fieldCacheBean.initializeMetrics(solrMetricsContext, Attributes.empty(), ""); if (isZooKeeperAware()) { @@ -2344,8 +2353,9 @@ public class CoreContainer { } if (handler instanceof SolrMetricProducer) { ((SolrMetricProducer) handler) - // TODO SOLR-17458: Add Otel - .initializeMetrics(solrMetricsContext, Attributes.empty(), path); + // NOCOMMIT SOLR-17458: Add Otel + .initializeMetrics( + solrMetricsContext, Attributes.builder().put(HANDLER_ATTR, path).build(), path); } return handler; } diff --git a/solr/core/src/java/org/apache/solr/handler/RequestHandlerBase.java b/solr/core/src/java/org/apache/solr/handler/RequestHandlerBase.java index 8d51929f7cd..6a70abe9441 100644 --- a/solr/core/src/java/org/apache/solr/handler/RequestHandlerBase.java +++ b/solr/core/src/java/org/apache/solr/handler/RequestHandlerBase.java @@ -19,12 +19,11 @@ package org.apache.solr.handler; import static org.apache.solr.core.RequestParams.USEPARAM; import static org.apache.solr.response.SolrQueryResponse.haveCompleteResults; -import com.codahale.metrics.Counter; -import com.codahale.metrics.Meter; -import com.codahale.metrics.Timer; import java.io.IOException; import io.opentelemetry.api.common.AttributeKey; import io.opentelemetry.api.common.Attributes; +import io.opentelemetry.api.metrics.LongCounter; +import io.opentelemetry.api.metrics.LongHistogram; import java.lang.invoke.MethodHandles; import java.util.Collection; import java.util.Collections; @@ -179,7 +178,10 @@ public abstract class RequestHandlerBase this.solrMetricsContext = parentContext.getChildContext(this); } - metrics = new HandlerMetrics(solrMetricsContext, attributes, getCategory().toString(), scope); + metrics = + new HandlerMetrics( + solrMetricsContext, + attributes.toBuilder().put(CATEGORY_ATTR, getCategory().toString()).build()); // NOCOMMIT: I don't see value in this metric solrMetricsContext.gauge( @@ -197,74 +199,65 @@ public abstract class RequestHandlerBase "NO_OP"), Attributes.empty()); - public final Meter numErrors; - public final Meter numServerErrors; - public final Meter numClientErrors; - public final Meter numTimeouts; - public final Counter requests; - public final Timer requestTimes; - public final Counter totalTime; - - public AttributedLongCounter otelRequests; - public AttributedLongCounter otelNumServerErrors; - public AttributedLongCounter otelNumClientErrors; - public AttributedLongCounter otelNumTimeouts; - public AttributedLongTimer otelRequestTimes; - - public HandlerMetrics( - SolrMetricsContext solrMetricsContext, Attributes attributes, String... metricPath) { - - // NOCOMMIT SOLR-17458: To be removed - numErrors = solrMetricsContext.meter("errors", metricPath); - numServerErrors = solrMetricsContext.meter("serverErrors", metricPath); - numClientErrors = solrMetricsContext.meter("clientErrors", metricPath); - numTimeouts = solrMetricsContext.meter("timeouts", metricPath); - requests = solrMetricsContext.counter("requests", metricPath); - requestTimes = solrMetricsContext.timer("requestTimes", metricPath); - totalTime = solrMetricsContext.counter("totalTime", metricPath); - - var baseRequestMetric = - solrMetricsContext.longCounter("solr_metrics_core_requests", "HTTP Solr request counts"); - - var baseErrorRequestMetric = - solrMetricsContext.longCounter( - "solr_metrics_core_requests_errors", "HTTP Solr request error counts"); - - var baseRequestTimeMetric = - solrMetricsContext.longHistogram( - "solr_metrics_core_requests_times", "HTTP Solr request times", "ms"); - - otelRequests = - new AttributedLongCounter( - baseRequestMetric, Attributes.builder().putAll(attributes).build()); + public AttributedLongCounter requests; + public AttributedLongCounter numServerErrors; + public AttributedLongCounter numClientErrors; + public AttributedLongCounter numTimeouts; + public AttributedLongTimer requestTimes; + + public HandlerMetrics(SolrMetricsContext solrMetricsContext, Attributes attributes) { + + LongCounter requestMetric; + LongCounter errorRequestMetric; + LongCounter timeoutRequestMetric; + LongHistogram requestTimeMetric; + + if (solrMetricsContext.getRegistryName().equals("solr.node")) { + requestMetric = + solrMetricsContext.longCounter("solr_node_requests", "Http Solr node requests"); + errorRequestMetric = + solrMetricsContext.longCounter( + "solr_node_requests_errors", "HTTP Solr node request errors"); + timeoutRequestMetric = + solrMetricsContext.longCounter( + "solr_node_requests_timeout", "HTTP Solr node request timeouts"); + requestTimeMetric = + solrMetricsContext.longHistogram( + "solr_node_requests_times", "HTTP Solr node request times", "ms"); + } else { + requestMetric = + solrMetricsContext.longCounter("solr_core_requests", "HTTP Solr core requests"); + errorRequestMetric = + solrMetricsContext.longCounter( + "solr_core_requests_errors", "HTTP Solr core request errors"); + timeoutRequestMetric = + solrMetricsContext.longCounter( + "solr_core_requests_timeout", "HTTP Solr core request timeouts"); + requestTimeMetric = + solrMetricsContext.longHistogram( + "solr_core_requests_times", "HTTP Solr core request times", "ms"); + } - otelNumServerErrors = - new AttributedLongCounter( - baseErrorRequestMetric, - Attributes.builder() - .putAll(attributes) - .put(AttributeKey.stringKey("source"), "server") - .build()); + requests = new AttributedLongCounter(requestMetric, attributes); - otelNumClientErrors = + numServerErrors = new AttributedLongCounter( - baseErrorRequestMetric, - Attributes.builder() - .putAll(attributes) - .put(AttributeKey.stringKey("source"), "client") - .build()); + errorRequestMetric, + attributes.toBuilder().put(AttributeKey.stringKey("source"), "server").build()); - otelNumTimeouts = + numClientErrors = new AttributedLongCounter( - baseErrorRequestMetric, - Attributes.builder().putAll(attributes).put(TYPE_ATTR, "timeouts").build()); + errorRequestMetric, + attributes.toBuilder().put(AttributeKey.stringKey("source"), "client").build()); + + numTimeouts = new AttributedLongCounter(timeoutRequestMetric, attributes); - otelRequestTimes = new AttributedLongTimer(baseRequestTimeMetric, attributes); + requestTimes = new AttributedLongTimer(requestTimeMetric, attributes); // NOCOMMIT: Temporary to see metrics - otelRequests.add(0L); - otelNumTimeouts.add(0L); - otelNumClientErrors.add(0L); - otelNumServerErrors.add(0L); + requests.add(0L); + numTimeouts.add(0L); + numClientErrors.add(0L); + numServerErrors.add(0L); } } @@ -291,10 +284,8 @@ public abstract class RequestHandlerBase HandlerMetrics metrics = getMetricsForThisRequest(req); metrics.requests.inc(); - metrics.otelRequests.inc(); - Timer.Context timer = metrics.requestTimes.time(); - AttributedLongTimer.MetricTimer otelTimer = metrics.otelRequestTimes.start(); + AttributedLongTimer.MetricTimer timer = metrics.requestTimes.start(); try { TestInjection.injectLeaderTragedy(req.getCore()); if (pluginInfo != null && pluginInfo.attributes.containsKey(USEPARAM)) @@ -306,8 +297,7 @@ public abstract class RequestHandlerBase // count timeouts if (!haveCompleteResults(rsp.getResponseHeader())) { - metrics.numTimeouts.mark(); - metrics.otelNumTimeouts.inc(); + metrics.numTimeouts.inc(); rsp.setHttpCaching(false); } } catch (QueryLimitsExceededException e) { @@ -318,9 +308,7 @@ public abstract class RequestHandlerBase rsp.setException(normalized); } finally { try { - long elapsed = timer.stop(); - metrics.totalTime.inc(elapsed); - otelTimer.stop(); + timer.stop(); if (publishCpuTime) { Optional<Long> cpuTime = ThreadCpuTimer.readMSandReset(REQUEST_CPU_TIMER_CONTEXT); @@ -362,15 +350,12 @@ public abstract class RequestHandlerBase } } - metrics.numErrors.mark(); if (isClientError) { log.error("Client exception", e); - metrics.numClientErrors.mark(); - metrics.otelNumClientErrors.inc(); + metrics.numClientErrors.inc(); } else { log.error("Server exception", e); - metrics.numServerErrors.mark(); - metrics.otelNumServerErrors.inc(); + metrics.numServerErrors.inc(); } } diff --git a/solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java b/solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java index fdbf1a37f3b..b42e62d187b 100644 --- a/solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java +++ b/solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java @@ -92,6 +92,8 @@ import org.slf4j.MDC; /** Refer SOLR-281 */ public class SearchHandler extends RequestHandlerBase implements SolrCoreAware, PluginInfoInitialized, PermissionNameProvider { + + public static final AttributeKey<Boolean> INTERNAL_ATTR = AttributeKey.booleanKey("internal"); static final String INIT_COMPONENTS = "components"; static final String INIT_FIRST_COMPONENTS = "first-components"; static final String INIT_LAST_COMPONENTS = "last-components"; @@ -155,28 +157,21 @@ public class SearchHandler extends RequestHandlerBase } } - // TODO SOLR-17458: Fix metric Attributes @Override public void initializeMetrics( SolrMetricsContext parentContext, Attributes attributes, String scope) { super.initializeMetrics( parentContext, - Attributes.builder() - .putAll(attributes) - .put(AttributeKey.stringKey("category"), getCategory().toString()) - .put(AttributeKey.stringKey("internal"), "false") - .build(), + Attributes.builder().putAll(attributes).put(INTERNAL_ATTR, false).build(), scope); metricsShard = new HandlerMetrics( // will register various metrics in the context solrMetricsContext, Attributes.builder() .putAll(attributes) - .put(AttributeKey.stringKey("category"), getCategory().toString()) - .put(AttributeKey.stringKey("internal"), "true") - .build(), - getCategory().toString(), - scope + SHARD_HANDLER_SUFFIX); + .put(CATEGORY_ATTR, getCategory().toString()) + .put(INTERNAL_ATTR, true) + .build()); } @Override diff --git a/solr/core/src/java/org/apache/solr/jersey/RequestMetricHandling.java b/solr/core/src/java/org/apache/solr/jersey/RequestMetricHandling.java index 6cea50d03d2..2ee9c079610 100644 --- a/solr/core/src/java/org/apache/solr/jersey/RequestMetricHandling.java +++ b/solr/core/src/java/org/apache/solr/jersey/RequestMetricHandling.java @@ -21,7 +21,6 @@ import static org.apache.solr.jersey.RequestContextKeys.HANDLER_METRICS; import static org.apache.solr.jersey.RequestContextKeys.SOLR_QUERY_REQUEST; import static org.apache.solr.jersey.RequestContextKeys.TIMER; -import com.codahale.metrics.Timer; import jakarta.ws.rs.container.ContainerRequestContext; import jakarta.ws.rs.container.ContainerRequestFilter; import jakarta.ws.rs.container.ContainerResponseContext; @@ -90,11 +89,8 @@ public class RequestMetricHandling { final RequestHandlerBase.HandlerMetrics metrics = handlerBase.getMetricsForThisRequest(solrQueryRequest); - // TODO SOLR-17458: Recording both Otel and dropwizard for now. Will remove Dropwizard after - // removal metrics.requests.inc(); - requestContext.setProperty(TIMER, metrics.otelRequestTimes.start()); - requestContext.setProperty("dropwizardTimer", metrics.requestTimes.time()); + requestContext.setProperty(TIMER, metrics.requestTimes.start()); requestContext.setProperty(HANDLER_METRICS, metrics); } } @@ -120,15 +116,11 @@ public class RequestMetricHandling { && SolrJerseyResponse.class.isInstance(responseContext.getEntity())) { final SolrJerseyResponse response = (SolrJerseyResponse) responseContext.getEntity(); if (Boolean.TRUE.equals(response.responseHeader.partialResults)) { - metrics.numTimeouts.mark(); - metrics.otelNumTimeouts.inc(); + metrics.numTimeouts.inc(); } } else { log.debug("Skipping partialResults check because entity was not SolrJerseyResponse"); } - final var dropwizardTimer = (Timer.Context) requestContext.getProperty("dropwizardTimer"); - metrics.totalTime.inc(dropwizardTimer.stop()); - final var timer = (AttributedLongTimer.MetricTimer) requestContext.getProperty(TIMER); timer.stop(); } 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 a01183da9ad..01122a779e0 100644 --- a/solr/core/src/java/org/apache/solr/metrics/SolrCoreMetricManager.java +++ b/solr/core/src/java/org/apache/solr/metrics/SolrCoreMetricManager.java @@ -16,6 +16,8 @@ */ package org.apache.solr.metrics; +import static org.apache.solr.metrics.SolrMetricProducer.HANDLER_ATTR; + import com.codahale.metrics.MetricRegistry; import io.opentelemetry.api.common.AttributeKey; import io.opentelemetry.api.common.Attributes; @@ -39,7 +41,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> HANDLER_ATTR = AttributeKey.stringKey("handler"); public static final AttributeKey<String> SCOPE_ATTR = AttributeKey.stringKey("scope"); private final SolrCore core; diff --git a/solr/core/src/java/org/apache/solr/metrics/SolrMetricProducer.java b/solr/core/src/java/org/apache/solr/metrics/SolrMetricProducer.java index e2b433108e6..94b740540af 100644 --- a/solr/core/src/java/org/apache/solr/metrics/SolrMetricProducer.java +++ b/solr/core/src/java/org/apache/solr/metrics/SolrMetricProducer.java @@ -25,6 +25,7 @@ public interface SolrMetricProducer extends AutoCloseable { public static final AttributeKey<String> TYPE_ATTR = AttributeKey.stringKey("type"); public static final AttributeKey<String> CATEGORY_ATTR = AttributeKey.stringKey("category"); + public static final AttributeKey<String> HANDLER_ATTR = AttributeKey.stringKey("handler"); public static final AttributeKey<String> OPERATION_ATTR = AttributeKey.stringKey("ops"); /** diff --git a/solr/core/src/test/org/apache/solr/cloud/TestRandomRequestDistribution.java b/solr/core/src/test/org/apache/solr/cloud/TestRandomRequestDistribution.java index 9cc1ccace2e..a83da0d2e7b 100644 --- a/solr/core/src/test/org/apache/solr/cloud/TestRandomRequestDistribution.java +++ b/solr/core/src/test/org/apache/solr/cloud/TestRandomRequestDistribution.java @@ -16,7 +16,6 @@ */ package org.apache.solr.cloud; -import com.codahale.metrics.Counter; import java.lang.invoke.MethodHandles; import java.util.ArrayList; import java.util.Collection; @@ -42,7 +41,7 @@ import org.apache.solr.common.cloud.ZkStateReader; import org.apache.solr.core.CoreContainer; import org.apache.solr.core.SolrCore; import org.apache.solr.embedded.JettySolrRunner; -import org.apache.solr.metrics.SolrMetricManager; +import org.apache.solr.metrics.SolrMetricTestUtils; import org.junit.Test; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -84,23 +83,23 @@ public class TestRandomRequestDistribution extends AbstractFullDistribZkTestBase ZkStateReader.from(cloudClient).forceUpdateCollection("b1x1"); - // get direct access to the metrics counters for each core/replica we're interested to monitor - // them - final Map<String, Counter> counters = new LinkedHashMap<>(); + // get direct access to the SolrCore objects for each core/replica we're interested to monitor + final Map<String, SolrCore> cores = new LinkedHashMap<>(); for (JettySolrRunner runner : jettys) { CoreContainer container = runner.getCoreContainer(); - SolrMetricManager metricManager = container.getMetricManager(); for (SolrCore core : container.getCores()) { if ("a1x2".equals(core.getCoreDescriptor().getCollectionName())) { - String registry = core.getCoreMetricManager().getRegistryName(); - Counter cnt = metricManager.counter(null, registry, "requests", "QUERY./select"); - // sanity check - assertEquals(core.getName() + " has already received some requests?", 0, cnt.getCount()); - counters.put(core.getName(), cnt); + cores.put(core.getName(), core); } } } - assertEquals("Sanity Check: we know there should be 2 replicas", 2, counters.size()); + assertEquals("Sanity Check: we know there should be 2 replicas", 2, cores.size()); + + // Sanity check - all cores should start with 0 requests + for (Map.Entry<String, SolrCore> entry : cores.entrySet()) { + double initialCount = getSelectRequestCount(entry.getValue()); + assertEquals(entry.getKey() + " has already received some requests?", 0L, initialCount, 0.0); + } // send queries to the node that doesn't host any core/replica and see where it routes them ClusterState clusterState = cloudClient.getClusterState(); @@ -120,22 +119,23 @@ public class TestRandomRequestDistribution extends AbstractFullDistribZkTestBase Set<String> uniqueCoreNames = new LinkedHashSet<>(); log.info("Making requests to {} a1x2", baseUrl); - while (uniqueCoreNames.size() < counters.keySet().size() && expectedTotalRequests < 1000L) { + while (uniqueCoreNames.size() < cores.size() && expectedTotalRequests < 1000L) { expectedTotalRequests++; client.query(new SolrQuery("*:*")); - long actualTotalRequests = 0; - for (Map.Entry<String, Counter> e : counters.entrySet()) { - final long coreCount = e.getValue().getCount(); + double actualTotalRequests = 0; + for (Map.Entry<String, SolrCore> entry : cores.entrySet()) { + final double coreCount = getSelectRequestCount(entry.getValue()); actualTotalRequests += coreCount; if (0 < coreCount) { - uniqueCoreNames.add(e.getKey()); + uniqueCoreNames.add(entry.getKey()); } } assertEquals( "Sanity Check: Num Queries So Far Doesn't Match Total????", expectedTotalRequests, - actualTotalRequests); + actualTotalRequests, + 0.0); } log.info("Total requests: {}", expectedTotalRequests); assertEquals( @@ -144,7 +144,7 @@ public class TestRandomRequestDistribution extends AbstractFullDistribZkTestBase + expectedTotalRequests + " requests", uniqueCoreNames.size(), - counters.size()); + cores.size()); } } @@ -240,10 +240,6 @@ public class TestRandomRequestDistribution extends AbstractFullDistribZkTestBase } assertNotNull(leaderCore); - SolrMetricManager leaderMetricManager = leaderCore.getCoreContainer().getMetricManager(); - String leaderRegistry = leaderCore.getCoreMetricManager().getRegistryName(); - Counter cnt = leaderMetricManager.counter(null, leaderRegistry, "requests", "QUERY./select"); - // All queries should be served by the active replica to make sure that's true we keep // querying the down replica. If queries are getting processed by the down replica then the // cluster state hasn't updated for that replica locally. So we keep trying till it has @@ -253,7 +249,7 @@ public class TestRandomRequestDistribution extends AbstractFullDistribZkTestBase count++; client.query(new SolrQuery("*:*")); - long c = cnt.getCount(); + double c = getSelectRequestCount(leaderCore); if (c == 1) { break; // cluster state has got update locally @@ -274,10 +270,21 @@ public class TestRandomRequestDistribution extends AbstractFullDistribZkTestBase client.query(new SolrQuery("*:*")); count++; - long c = cnt.getCount(); + double c = getSelectRequestCount(leaderCore); - assertEquals("Query wasn't served by leader", count, c); + assertEquals("Query wasn't served by leader", count, c, 0.0); } } } + + private Double getSelectRequestCount(SolrCore core) { + var labels = + SolrMetricTestUtils.getCloudLabelsBase(core) + .label("category", "QUERY") + .label("handler", "/select") + .label("internal", "false") + .build(); + var datapoint = SolrMetricTestUtils.getCounterDatapoint(core, "solr_core_requests", labels); + return datapoint != null ? datapoint.getValue() : 0.0; + } } diff --git a/solr/core/src/test/org/apache/solr/core/RequestHandlersTest.java b/solr/core/src/test/org/apache/solr/core/RequestHandlersTest.java index 7ac9d6a107e..083337dfb97 100644 --- a/solr/core/src/test/org/apache/solr/core/RequestHandlersTest.java +++ b/solr/core/src/test/org/apache/solr/core/RequestHandlersTest.java @@ -17,9 +17,9 @@ package org.apache.solr.core; import com.codahale.metrics.Gauge; -import java.util.Map; import org.apache.solr.SolrTestCaseJ4; import org.apache.solr.metrics.SolrMetricManager; +import org.apache.solr.metrics.SolrMetricTestUtils; import org.apache.solr.request.SolrRequestHandler; import org.junit.BeforeClass; import org.junit.Test; @@ -114,8 +114,6 @@ public class RequestHandlersTest extends SolrTestCaseJ4 { @Test public void testStatistics() { SolrCore core = h.getCore(); - SolrRequestHandler updateHandler = core.getRequestHandler("/update"); - SolrRequestHandler termHandler = core.getRequestHandler("/terms"); assertU( adoc( @@ -125,12 +123,25 @@ public class RequestHandlersTest extends SolrTestCaseJ4 { "line up and fly directly at the enemy death cannons, clogging them with wreckage!")); assertU(commit()); - Map<String, Object> updateStats = updateHandler.getSolrMetricsContext().getMetricsSnapshot(); - Map<String, Object> termStats = termHandler.getSolrMetricsContext().getMetricsSnapshot(); - - Long updateTime = (Long) updateStats.get("UPDATE./update.totalTime"); - Long termTime = (Long) termStats.get("QUERY./terms.totalTime"); - - assertNotEquals("RequestHandlers should not share statistics!", updateTime, termTime); + var updateDp = + SolrMetricTestUtils.getHistogramDatapoint( + core, + "solr_core_requests_times_milliseconds", + SolrMetricTestUtils.newStandaloneLabelsBuilder(core) + .label("category", "UPDATE") + .label("handler", "/update") + .build()); + var termDp = + SolrMetricTestUtils.getHistogramDatapoint( + core, + "solr_core_requests_times_milliseconds", + SolrMetricTestUtils.newStandaloneLabelsBuilder(core) + .label("category", "QUERY") + .label("handler", "/terms") + .build()); + + // RequestHandlers should not share statistics + assertTrue(updateDp.getSum() > 0); + assertNull("/terms should not have any time value", termDp); } } diff --git a/solr/core/src/test/org/apache/solr/handler/RequestHandlerBaseTest.java b/solr/core/src/test/org/apache/solr/handler/RequestHandlerBaseTest.java index 1ea000903cc..74d3d6b2510 100644 --- a/solr/core/src/test/org/apache/solr/handler/RequestHandlerBaseTest.java +++ b/solr/core/src/test/org/apache/solr/handler/RequestHandlerBaseTest.java @@ -187,6 +187,7 @@ public class RequestHandlerBaseTest extends SolrTestCaseJ4 { private RequestHandlerBase.HandlerMetrics createHandlerMetrics() { final SolrMetricsContext metricsContext = mock(SolrMetricsContext.class); + when(metricsContext.getRegistryName()).thenReturn("solr.core"); when(metricsContext.timer(any(), any())).thenReturn(mock(Timer.class)); when(metricsContext.meter(any(), any())).then(invocation -> mock(Meter.class)); when(metricsContext.counter(any(), any())).thenReturn(mock(Counter.class)); @@ -195,8 +196,6 @@ public class RequestHandlerBaseTest extends SolrTestCaseJ4 { when(metricsContext.longHistogram(any(), any())).thenReturn(mockLongHistogram); return new RequestHandlerBase.HandlerMetrics( - metricsContext, - Attributes.of(AttributeKey.stringKey("scope"), "someBaseMetricPath"), - "someBaseMetricPath"); + metricsContext, Attributes.of(AttributeKey.stringKey("/handler"), "/someBaseMetricPath")); } } diff --git a/solr/core/src/test/org/apache/solr/handler/RequestHandlerMetricsTest.java b/solr/core/src/test/org/apache/solr/handler/RequestHandlerMetricsTest.java index 96a038f7bc4..7a23fd88094 100644 --- a/solr/core/src/test/org/apache/solr/handler/RequestHandlerMetricsTest.java +++ b/solr/core/src/test/org/apache/solr/handler/RequestHandlerMetricsTest.java @@ -54,7 +54,10 @@ public class RequestHandlerMetricsTest extends SolrCloudTestCase { System.clearProperty("metricsEnabled"); } + // NOCOMMIT: Need to fix aggregateNodeLevelMetricsEnabled for OTEL. Delegate registries currently + // do not play nice with OTEL instrumentation @Test + @BadApple(bugUrl = "https://issues.apache.org/jira/browse/SOLR-17458";) @SuppressWarnings({"unchecked"}) public void testAggregateNodeLevelMetrics() throws SolrServerException, IOException { String collection1 = "testRequestHandlerMetrics1"; diff --git a/solr/core/src/test/org/apache/solr/handler/admin/MBeansHandlerTest.java b/solr/core/src/test/org/apache/solr/handler/admin/MBeansHandlerTest.java index e7395188bb0..1dab6cf5d42 100644 --- a/solr/core/src/test/org/apache/solr/handler/admin/MBeansHandlerTest.java +++ b/solr/core/src/test/org/apache/solr/handler/admin/MBeansHandlerTest.java @@ -25,6 +25,7 @@ import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; import java.util.regex.Matcher; import java.util.regex.Pattern; +import org.apache.lucene.tests.util.LuceneTestCase; import org.apache.solr.SolrTestCaseJ4; import org.apache.solr.common.params.CommonParams; import org.apache.solr.common.util.ContentStream; @@ -38,6 +39,9 @@ import org.junit.Test; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +// NOCOMMIT: Test fails because of the /admin/mbeans endpoint. Need to create a bridge or shim after +// migrating all metrics to support this. [email protected](bugUrl = "https://issues.apache.org/jira/browse/SOLR-17458";) public class MBeansHandlerTest extends SolrTestCaseJ4 { private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); diff --git a/solr/core/src/test/org/apache/solr/handler/admin/MetricsHandlerTest.java b/solr/core/src/test/org/apache/solr/handler/admin/MetricsHandlerTest.java index af27e7d1a6e..0878bf2c8cd 100644 --- a/solr/core/src/test/org/apache/solr/handler/admin/MetricsHandlerTest.java +++ b/solr/core/src/test/org/apache/solr/handler/admin/MetricsHandlerTest.java @@ -429,7 +429,9 @@ public class MetricsHandlerTest extends SolrTestCaseJ4 { handler.close(); } + // NOCOMMIT: Have not implemented any kind of filtering for OTEL yet @Test + @BadApple(bugUrl = "https://issues.apache.org/jira/browse/SOLR-17458";) public void testKeyMetrics() throws Exception { MetricsHandler handler = new MetricsHandler(h.getCoreContainer()); @@ -577,7 +579,9 @@ public class MetricsHandlerTest extends SolrTestCaseJ4 { handler.close(); } + // NOCOMMIT: Have not implemented any kind of filtering for OTEL yet @Test + @BadApple(bugUrl = "https://issues.apache.org/jira/browse/SOLR-17458";) @SuppressWarnings("unchecked") public void testExprMetrics() throws Exception { MetricsHandler handler = new MetricsHandler(h.getCoreContainer()); diff --git a/solr/core/src/test/org/apache/solr/response/TestPrometheusResponseWriterCloud.java b/solr/core/src/test/org/apache/solr/response/TestPrometheusResponseWriterCloud.java index 0c12b80ce32..003745fe383 100644 --- a/solr/core/src/test/org/apache/solr/response/TestPrometheusResponseWriterCloud.java +++ b/solr/core/src/test/org/apache/solr/response/TestPrometheusResponseWriterCloud.java @@ -26,7 +26,7 @@ import org.apache.solr.client.solrj.impl.InputStreamResponseParser; import org.apache.solr.client.solrj.request.CollectionAdminRequest; import org.apache.solr.client.solrj.request.GenericSolrRequest; import org.apache.solr.cloud.SolrCloudTestCase; -import org.apache.solr.common.params.ModifiableSolrParams; +import org.apache.solr.common.params.SolrParams; import org.apache.solr.common.util.NamedList; import org.junit.Before; import org.junit.BeforeClass; @@ -65,16 +65,13 @@ public class TestPrometheusResponseWriterCloud extends SolrCloudTestCase { public void testPrometheusCloudLabels() throws Exception { var solrClient = cluster.getSolrClient(); - // Increment solr_metrics_core_requests metric for /select + // Increment solr_core_requests metric for /select SolrQuery query = new SolrQuery("*:*"); solrClient.query("collection1", query); var req = new GenericSolrRequest( - METHOD.GET, - "/admin/metrics", - SolrRequestType.ADMIN, - new ModifiableSolrParams().set("wt", "prometheus")); + METHOD.GET, "/admin/metrics", SolrRequestType.ADMIN, SolrParams.of("wt", "prometheus")); req.setResponseParser(new InputStreamResponseParser("prometheus")); NamedList<Object> resp = solrClient.request(req); @@ -86,7 +83,7 @@ public class TestPrometheusResponseWriterCloud extends SolrCloudTestCase { .lines() .anyMatch( line -> - line.startsWith("solr_metrics_core_requests_total") + line.startsWith("solr_core_requests_total") && line.contains("handler=\"/select\"") && line.contains("collection=\"collection1\"") && line.contains("core=\"collection1_shard1_replica_n1\"") @@ -99,17 +96,14 @@ public class TestPrometheusResponseWriterCloud extends SolrCloudTestCase { public void testCollectionDeletePrometheusOutput() throws Exception { var solrClient = cluster.getSolrClient(); - // Increment solr_metrics_core_requests metric for /select and assert it exists + // Increment solr_core_requests metric for /select and assert it exists SolrQuery query = new SolrQuery("*:*"); solrClient.query("collection1", query); solrClient.query("collection2", query); var req = new GenericSolrRequest( - METHOD.GET, - "/admin/metrics", - SolrRequestType.ADMIN, - new ModifiableSolrParams().set("wt", "prometheus")); + METHOD.GET, "/admin/metrics", SolrRequestType.ADMIN, SolrParams.of("wt", "prometheus")); req.setResponseParser(new InputStreamResponseParser("prometheus")); NamedList<Object> resp = solrClient.request(req); @@ -118,21 +112,17 @@ public class TestPrometheusResponseWriterCloud extends SolrCloudTestCase { String output = new String(in.readAllBytes(), StandardCharsets.UTF_8); assertTrue( - "Prometheus output should contains solr_metrics_core_requests for collection1", + "Prometheus output should contains solr_core_requests for collection1", output .lines() .anyMatch( - line -> - line.startsWith("solr_metrics_core_requests") - && line.contains("collection1"))); + line -> line.startsWith("solr_core_requests") && line.contains("collection1"))); assertTrue( - "Prometheus output should contains solr_metrics_core_requests for collection2", + "Prometheus output should contains solr_core_requests for collection2", output .lines() .anyMatch( - line -> - line.startsWith("solr_metrics_core_requests") - && line.contains("collection2"))); + line -> line.startsWith("solr_core_requests") && line.contains("collection2"))); } // Delete collection and assert metrics have been removed @@ -143,21 +133,17 @@ public class TestPrometheusResponseWriterCloud extends SolrCloudTestCase { try (InputStream in = (InputStream) resp.get("stream")) { String output = new String(in.readAllBytes(), StandardCharsets.UTF_8); assertFalse( - "Prometheus output should not contain solr_metrics_core_requests after collection was deleted", + "Prometheus output should not contain solr_core_requests after collection was deleted", output .lines() .anyMatch( - line -> - line.startsWith("solr_metrics_core_requests") - && line.contains("collection1"))); + line -> line.startsWith("solr_core_requests") && line.contains("collection1"))); assertTrue( - "Prometheus output should contains solr_metrics_core_requests for collection2", + "Prometheus output should contains solr_core_requests for collection2", output .lines() .anyMatch( - line -> - line.startsWith("solr_metrics_core_requests") - && line.contains("collection2"))); + line -> line.startsWith("solr_core_requests") && line.contains("collection2"))); } } } 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 new file mode 100644 index 00000000000..755be090eac --- /dev/null +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/SolrJMetricTestUtils.java @@ -0,0 +1,117 @@ +/* + * 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.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; +import org.apache.solr.common.params.SolrParams; +import org.apache.solr.common.util.NamedList; + +public final class SolrJMetricTestUtils { + + public static double getPrometheusMetricValue(CloudSolrClient solrClient, String metricName) + throws SolrServerException, IOException { + var req = + new GenericSolrRequest( + SolrRequest.METHOD.GET, + "/admin/metrics", + SolrRequest.SolrRequestType.ADMIN, + SolrParams.of("wt", "prometheus")); + req.setResponseParser(new InputStreamResponseParser("prometheus")); + + NamedList<Object> resp = solrClient.request(req); + try (InputStream in = (InputStream) resp.get("stream")) { + String output = new String(in.readAllBytes(), StandardCharsets.UTF_8); + return output + .lines() + .filter(l -> l.startsWith(metricName)) + .mapToDouble(s -> Double.parseDouble(s.substring(s.lastIndexOf(" ")))) + .sum(); + } + } + + public static Double getNumCoreRequests( + String baseUrl, String collectionName, String category, String handler) + throws SolrServerException, IOException { + + try (Http2SolrClient client = new Http2SolrClient.Builder(baseUrl).build()) { + var req = + new GenericSolrRequest( + SolrRequest.METHOD.GET, + "/admin/metrics", + SolrRequest.SolrRequestType.ADMIN, + SolrParams.of("wt", "prometheus")); + req.setResponseParser(new InputStreamResponseParser("prometheus")); + + NamedList<Object> resp = client.request(req); + try (InputStream in = (InputStream) resp.get("stream")) { + String output = new String(in.readAllBytes(), StandardCharsets.UTF_8); + String metricName; + + metricName = "solr_core_requests_total"; + + return output + .lines() + .filter( + l -> + l.contains(metricName) + && l.contains(String.format("category=\"%s\"", category)) + && l.contains(String.format("collection=\"%s\"", collectionName)) + && l.contains(String.format("handler=\"%s\"", handler))) + .mapToDouble(s -> Double.parseDouble(s.substring(s.lastIndexOf(" ")))) + .sum(); + } + } + } + + public static Double getNumNodeRequestErrors(String baseUrl, String category, String handler) + throws SolrServerException, IOException { + + try (Http2SolrClient client = new Http2SolrClient.Builder(baseUrl).build()) { + var req = + new GenericSolrRequest( + SolrRequest.METHOD.GET, + "/admin/metrics", + SolrRequest.SolrRequestType.ADMIN, + SolrParams.of("wt", "prometheus")); + req.setResponseParser(new InputStreamResponseParser("prometheus")); + + NamedList<Object> resp = client.request(req); + try (InputStream in = (InputStream) resp.get("stream")) { + String output = new String(in.readAllBytes(), StandardCharsets.UTF_8); + String metricName; + metricName = "solr_node_requests_errors_total"; + // Sum both client and server errors + return output + .lines() + .filter( + l -> + l.contains(metricName) + && l.contains(String.format("category=\"%s\"", category)) + && l.contains(String.format("handler=\"%s\"", handler))) + .mapToDouble(s -> Double.parseDouble(s.substring(s.lastIndexOf(" ")))) + .sum(); + } + } + } +} diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientRetryTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientRetryTest.java index 39f10b2c324..7b5a9b8a0f5 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientRetryTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientRetryTest.java @@ -17,17 +17,13 @@ package org.apache.solr.client.solrj.impl; +import static org.apache.solr.client.solrj.SolrJMetricTestUtils.getPrometheusMetricValue; + import java.util.Collections; import java.util.Optional; -import org.apache.solr.client.solrj.SolrRequest.METHOD; -import org.apache.solr.client.solrj.SolrRequest.SolrRequestType; import org.apache.solr.client.solrj.request.CollectionAdminRequest; -import org.apache.solr.client.solrj.request.GenericSolrRequest; import org.apache.solr.cloud.SolrCloudTestCase; import org.apache.solr.common.SolrInputDocument; -import org.apache.solr.common.params.CommonParams; -import org.apache.solr.common.params.ModifiableSolrParams; -import org.apache.solr.common.util.NamedList; import org.apache.solr.util.TestInjection; import org.junit.BeforeClass; import org.junit.Test; @@ -52,6 +48,8 @@ public class CloudHttp2SolrClientRetryTest extends SolrCloudTestCase { @Test public void testRetry() throws Exception { String collectionName = "testRetry"; + String prometheusMetric = + "solr_core_requests_total{category=\"UPDATE\",collection=\"testRetry\",core=\"testRetry_shard1_replica_n1\",handler=\"/update\",otel_scope_name=\"org.apache.solr\",replica=\"replica_n1\",shard=\"shard1\"}"; try (CloudSolrClient solrClient = new CloudHttp2SolrClient.Builder( Collections.singletonList(cluster.getZkServer().getZkAddress()), Optional.empty()) @@ -60,20 +58,7 @@ public class CloudHttp2SolrClientRetryTest extends SolrCloudTestCase { solrClient.add(collectionName, new SolrInputDocument("id", "1")); - ModifiableSolrParams params = new ModifiableSolrParams(); - String updateRequestCountKey = - "solr.core.testRetry.shard1.replica_n1:UPDATE./update.requestTimes:count"; - params.set("key", updateRequestCountKey); - params.set("indent", "true"); - params.set(CommonParams.WT, "xml"); - - var metricsRequest = - new GenericSolrRequest(METHOD.GET, "/admin/metrics", SolrRequestType.ADMIN, params); - - NamedList<Object> namedList = solrClient.request(metricsRequest); - System.out.println(namedList); - NamedList<?> metrics = (NamedList<?>) namedList.get("metrics"); - assertEquals(1L, metrics.get(updateRequestCountKey)); + assertEquals(1.0, getPrometheusMetricValue(solrClient, prometheusMetric), 0.0); TestInjection.failUpdateRequests = "true:100"; try { @@ -87,10 +72,7 @@ public class CloudHttp2SolrClientRetryTest extends SolrCloudTestCase { TestInjection.reset(); } - namedList = solrClient.request(metricsRequest); - System.out.println(namedList); - metrics = (NamedList<?>) namedList.get("metrics"); - assertEquals(2L, metrics.get(updateRequestCountKey)); + assertEquals(2.0, getPrometheusMetricValue(solrClient, prometheusMetric), 0.0); } } } diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientTest.java index a6d9c13e129..76ce6a9f90e 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientTest.java @@ -33,15 +33,13 @@ import java.util.Locale; import java.util.Map; import java.util.Optional; import java.util.Set; -import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; import org.apache.http.impl.client.CloseableHttpClient; import org.apache.lucene.tests.util.TestUtil; import org.apache.solr.client.solrj.SolrClient; +import org.apache.solr.client.solrj.SolrJMetricTestUtils; import org.apache.solr.client.solrj.SolrQuery; import org.apache.solr.client.solrj.SolrRequest; -import org.apache.solr.client.solrj.SolrRequest.METHOD; -import org.apache.solr.client.solrj.SolrRequest.SolrRequestType; import org.apache.solr.client.solrj.SolrServerException; import org.apache.solr.client.solrj.request.AbstractUpdateRequest; import org.apache.solr.client.solrj.request.CollectionAdminRequest; @@ -402,7 +400,7 @@ public class CloudHttp2SolrClientTest extends SolrCloudTestCase { // Track request counts on each node before query calls ClusterState clusterState = cluster.getSolrClient().getClusterState(); DocCollection col = clusterState.getCollection("routing_collection"); - Map<String, Long> requestCountsMap = new HashMap<>(); + Map<String, Double> requestCountsMap = new HashMap<>(); for (Slice slice : col.getSlices()) { for (Replica replica : slice.getReplicas()) { String baseURL = replica.getBaseUrl(); @@ -463,17 +461,17 @@ public class CloudHttp2SolrClientTest extends SolrCloudTestCase { // Request counts increase from expected nodes should aggregate to 1000, while there should be // no increase in unexpected nodes. - long increaseFromExpectedUrls = 0; - long increaseFromUnexpectedUrls = 0; - Map<String, Long> numRequestsToUnexpectedUrls = new HashMap<>(); + double increaseFromExpectedUrls = 0.0; + double increaseFromUnexpectedUrls = 0.0; + Map<String, Double> numRequestsToUnexpectedUrls = new HashMap<>(); for (Slice slice : col.getSlices()) { for (Replica replica : slice.getReplicas()) { String baseURL = replica.getBaseUrl(); - Long prevNumRequests = requestCountsMap.get(baseURL); - Long curNumRequests = getNumRequests(baseURL, "routing_collection"); + Double prevNumRequests = requestCountsMap.get(baseURL); + Double curNumRequests = getNumRequests(baseURL, "routing_collection"); - long delta = curNumRequests - prevNumRequests; + Double delta = curNumRequests - prevNumRequests; if (expectedBaseURLs.contains(baseURL)) { increaseFromExpectedUrls += delta; } else { @@ -483,11 +481,13 @@ public class CloudHttp2SolrClientTest extends SolrCloudTestCase { } } - assertEquals("Unexpected number of requests to expected URLs", n, increaseFromExpectedUrls); + assertEquals( + "Unexpected number of requests to expected URLs", n, increaseFromExpectedUrls, 0.0); assertEquals( "Unexpected number of requests to unexpected URLs: " + numRequestsToUnexpectedUrls, 0, - increaseFromUnexpectedUrls); + increaseFromUnexpectedUrls, + 0.0); } /** @@ -643,57 +643,6 @@ public class CloudHttp2SolrClientTest extends SolrCloudTestCase { replicaTypeToReplicas.get(shardAddresses.get(0)).toUpperCase(Locale.ROOT)); } - private Long getNumRequests(String baseUrl, String collectionName) - throws SolrServerException, IOException { - return getNumRequests(baseUrl, collectionName, "QUERY", "/select", null, false); - } - - private Long getNumRequests( - String baseUrl, - String collectionName, - String category, - String key, - String scope, - boolean returnNumErrors) - throws SolrServerException, IOException { - - NamedList<Object> resp; - try (SolrClient client = - new HttpSolrClient.Builder(baseUrl) - .withDefaultCollection(collectionName) - .withConnectionTimeout(15000, TimeUnit.MILLISECONDS) - .withSocketTimeout(60000, TimeUnit.MILLISECONDS) - .build()) { - ModifiableSolrParams params = new ModifiableSolrParams(); - params.set("stats", "true"); - params.set("key", key); - params.set("cat", category); - // use generic request to avoid extra processing of queries - var req = - new GenericSolrRequest( - SolrRequest.METHOD.GET, "/admin/mbeans", SolrRequestType.ADMIN, params) - .setRequiresCollection(true); - resp = client.request(req); - } - String name; - if (returnNumErrors) { - name = category + "." + (scope != null ? scope : key) + ".errors"; - } else { - name = category + "." + (scope != null ? scope : key) + ".requests"; - } - @SuppressWarnings({"unchecked"}) - Map<String, Object> map = - (Map<String, Object>) resp._get(List.of("solr-mbeans", category, key, "stats"), null); - if (map == null) { - return null; - } - if (scope != null) { // admin handler uses a meter instead of counter here - return (Long) map.get(name + ".count"); - } else { - return (Long) map.get(name); - } - } - @Test public void testNonRetryableRequests() throws Exception { @@ -720,14 +669,11 @@ public class CloudHttp2SolrClientTest extends SolrCloudTestCase { for (String adminPath : adminPathToMbean.keySet()) { long errorsBefore = 0; for (JettySolrRunner runner : cluster.getJettySolrRunners()) { - Long numRequests = - getNumRequests( + Double numRequests = + SolrJMetricTestUtils.getNumNodeRequestErrors( runner.getBaseUrl().toString(), - "foo", - "ADMIN", - adminPathToMbean.get(adminPath), - adminPath, - true); + SolrRequest.SolrRequestType.ADMIN.name(), + adminPath); errorsBefore += numRequests; if (log.isInfoEnabled()) { log.info( @@ -739,7 +685,8 @@ public class CloudHttp2SolrClientTest extends SolrCloudTestCase { params.set("action", "foobar"); // this should cause an error var request = - new GenericSolrRequest(METHOD.GET, adminPath, SolrRequestType.ADMIN, params); + new GenericSolrRequest( + SolrRequest.METHOD.GET, adminPath, SolrRequest.SolrRequestType.ADMIN, params); try { NamedList<Object> resp = client.request(request); fail("call to foo for admin path " + adminPath + " should have failed"); @@ -748,14 +695,11 @@ public class CloudHttp2SolrClientTest extends SolrCloudTestCase { } long errorsAfter = 0; for (JettySolrRunner runner : cluster.getJettySolrRunners()) { - Long numRequests = - getNumRequests( + Double numRequests = + SolrJMetricTestUtils.getNumNodeRequestErrors( runner.getBaseUrl().toString(), - "foo", - "ADMIN", - adminPathToMbean.get(adminPath), - adminPath, - true); + SolrRequest.SolrRequestType.ADMIN.name(), + adminPath); errorsAfter += numRequests; if (log.isInfoEnabled()) { log.info( @@ -770,6 +714,11 @@ public class CloudHttp2SolrClientTest extends SolrCloudTestCase { } } + private Double getNumRequests(String baseUrl, String collectionName) + throws SolrServerException, IOException { + return SolrJMetricTestUtils.getNumCoreRequests(baseUrl, collectionName, "QUERY", "/select"); + } + @Test public void checkCollectionParameters() throws Exception { try (CloudSolrClient client = diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientRetryTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientRetryTest.java index 92be2220c3d..1bf8c9135cf 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientRetryTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientRetryTest.java @@ -17,15 +17,11 @@ package org.apache.solr.client.solrj.impl; -import org.apache.solr.client.solrj.SolrRequest.METHOD; -import org.apache.solr.client.solrj.SolrRequest.SolrRequestType; +import static org.apache.solr.client.solrj.SolrJMetricTestUtils.getPrometheusMetricValue; + import org.apache.solr.client.solrj.request.CollectionAdminRequest; -import org.apache.solr.client.solrj.request.GenericSolrRequest; import org.apache.solr.cloud.SolrCloudTestCase; import org.apache.solr.common.SolrInputDocument; -import org.apache.solr.common.params.CommonParams; -import org.apache.solr.common.params.ModifiableSolrParams; -import org.apache.solr.common.util.NamedList; import org.apache.solr.util.TestInjection; import org.junit.BeforeClass; import org.junit.Test; @@ -52,24 +48,11 @@ public class CloudSolrClientRetryTest extends SolrCloudTestCase { String collectionName = "testRetry"; CloudSolrClient solrClient = cluster.getSolrClient(); CollectionAdminRequest.createCollection(collectionName, 1, 1).process(solrClient); - + String prometheusMetric = + "solr_core_requests_total{category=\"UPDATE\",collection=\"testRetry\",core=\"testRetry_shard1_replica_n1\",handler=\"/update\",otel_scope_name=\"org.apache.solr\",replica=\"replica_n1\",shard=\"shard1\"}"; solrClient.add(collectionName, new SolrInputDocument("id", "1")); - ModifiableSolrParams params = new ModifiableSolrParams(); - String updateRequestCountKey = - "solr.core.testRetry.shard1.replica_n1:UPDATE./update.requestTimes:count"; - params.set("key", updateRequestCountKey); - params.set("indent", "true"); - params.set(CommonParams.WT, "xml"); - - var metricsRequest = - new GenericSolrRequest(METHOD.GET, "/admin/metrics", SolrRequestType.ADMIN, params); - - NamedList<Object> namedList = solrClient.request(metricsRequest); - System.out.println(namedList); - @SuppressWarnings({"rawtypes"}) - NamedList metrics = (NamedList) namedList.get("metrics"); - assertEquals(1L, metrics.get(updateRequestCountKey)); + assertEquals(1.0, getPrometheusMetricValue(solrClient, prometheusMetric), 0.0); TestInjection.failUpdateRequests = "true:100"; try { @@ -83,9 +66,6 @@ public class CloudSolrClientRetryTest extends SolrCloudTestCase { TestInjection.reset(); } - namedList = solrClient.request(metricsRequest); - System.out.println(namedList); - metrics = (NamedList) namedList.get("metrics"); - assertEquals(2L, metrics.get(updateRequestCountKey)); + assertEquals(2.0, getPrometheusMetricValue(solrClient, prometheusMetric), 0.0); } } diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientTest.java index a43da7a8c1d..aa4c8ae5b61 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientTest.java @@ -33,11 +33,11 @@ import java.util.Locale; import java.util.Map; import java.util.Optional; import java.util.Set; -import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; import org.apache.http.impl.client.CloseableHttpClient; import org.apache.lucene.tests.util.TestUtil; import org.apache.solr.client.solrj.SolrClient; +import org.apache.solr.client.solrj.SolrJMetricTestUtils; import org.apache.solr.client.solrj.SolrQuery; import org.apache.solr.client.solrj.SolrRequest; import org.apache.solr.client.solrj.SolrRequest.METHOD; @@ -339,11 +339,11 @@ public class CloudSolrClientTest extends SolrCloudTestCase { // Track request counts on each node before query calls ClusterState clusterState = cluster.getSolrClient().getClusterState(); DocCollection col = clusterState.getCollection("routing_collection"); - Map<String, Long> requestCountsMap = new HashMap<>(); + Map<String, Double> requestCountsMap = new HashMap<>(); for (Slice slice : col.getSlices()) { for (Replica replica : slice.getReplicas()) { String baseURL = replica.getBaseUrl(); - requestCountsMap.put(baseURL, getNumRequests(baseURL, "routing_collection")); + requestCountsMap.put(baseURL, getNumSelectRequests(baseURL)); } } @@ -399,17 +399,17 @@ public class CloudSolrClientTest extends SolrCloudTestCase { // Request counts increase from expected nodes should aggregate to 1000, while there should be // no increase in unexpected nodes. - long increaseFromExpectedUrls = 0; - long increaseFromUnexpectedUrls = 0; - Map<String, Long> numRequestsToUnexpectedUrls = new HashMap<>(); + Double increaseFromExpectedUrls = 0.0; + Double increaseFromUnexpectedUrls = 0.0; + Map<String, Double> numRequestsToUnexpectedUrls = new HashMap<>(); for (Slice slice : col.getSlices()) { for (Replica replica : slice.getReplicas()) { String baseURL = replica.getBaseUrl(); - Long prevNumRequests = requestCountsMap.get(baseURL); - Long curNumRequests = getNumRequests(baseURL, "routing_collection"); + Double prevNumRequests = requestCountsMap.get(baseURL); + Double curNumRequests = getNumSelectRequests(baseURL); - long delta = curNumRequests - prevNumRequests; + double delta = curNumRequests - prevNumRequests; if (expectedBaseURLs.contains(baseURL)) { increaseFromExpectedUrls += delta; } else { @@ -419,11 +419,13 @@ public class CloudSolrClientTest extends SolrCloudTestCase { } } - assertEquals("Unexpected number of requests to expected URLs", n, increaseFromExpectedUrls); + assertEquals( + "Unexpected number of requests to expected URLs", n, increaseFromExpectedUrls, 0.0); assertEquals( "Unexpected number of requests to unexpected URLs: " + numRequestsToUnexpectedUrls, 0, - increaseFromUnexpectedUrls); + increaseFromUnexpectedUrls, + 0.0); } /** @@ -578,55 +580,9 @@ public class CloudSolrClientTest extends SolrCloudTestCase { replicaTypeToReplicas.get(shardAddresses.get(0)).toUpperCase(Locale.ROOT)); } - private Long getNumRequests(String baseUrl, String collectionName) - throws SolrServerException, IOException { - return getNumRequests(baseUrl, collectionName, "QUERY", "/select", null, false); - } - - private Long getNumRequests( - String baseUrl, - String collectionName, - String category, - String key, - String scope, - boolean returnNumErrors) - throws SolrServerException, IOException { - - NamedList<Object> resp; - try (SolrClient client = - new HttpSolrClient.Builder(baseUrl) - .withDefaultCollection(collectionName) - .withConnectionTimeout(15000, TimeUnit.MILLISECONDS) - .withSocketTimeout(60000, TimeUnit.MILLISECONDS) - .build()) { - ModifiableSolrParams params = new ModifiableSolrParams(); - params.set("stats", "true"); - params.set("key", key); - params.set("cat", category); - // use generic request to avoid extra processing of queries - var req = - new GenericSolrRequest( - SolrRequest.METHOD.GET, "/admin/mbeans", SolrRequestType.ADMIN, params) - .setRequiresCollection(true); - resp = client.request(req); - } - String name; - if (returnNumErrors) { - name = category + "." + (scope != null ? scope : key) + ".errors"; - } else { - name = category + "." + (scope != null ? scope : key) + ".requests"; - } - @SuppressWarnings({"unchecked"}) - Map<String, Object> map = - (Map<String, Object>) resp._get(List.of("solr-mbeans", category, key, "stats"), null); - if (map == null) { - return null; - } - if (scope != null) { // admin handler uses a meter instead of counter here - return (Long) map.get(name + ".count"); - } else { - return (Long) map.get(name); - } + private Double getNumSelectRequests(String baseUrl) throws SolrServerException, IOException { + return SolrJMetricTestUtils.getNumCoreRequests( + baseUrl, "routing_collection", "QUERY", "/select"); } @Test @@ -656,14 +612,9 @@ public class CloudSolrClientTest extends SolrCloudTestCase { for (String adminPath : adminPathToMbean.keySet()) { long errorsBefore = 0; for (JettySolrRunner runner : cluster.getJettySolrRunners()) { - Long numRequests = - getNumRequests( - runner.getBaseUrl().toString(), - collection, - "ADMIN", - adminPathToMbean.get(adminPath), - adminPath, - true); + Double numRequests = + SolrJMetricTestUtils.getNumNodeRequestErrors( + runner.getBaseUrl().toString(), SolrRequestType.ADMIN.name(), adminPath); errorsBefore += numRequests; if (log.isInfoEnabled()) { log.info( @@ -684,14 +635,9 @@ public class CloudSolrClientTest extends SolrCloudTestCase { } long errorsAfter = 0; for (JettySolrRunner runner : cluster.getJettySolrRunners()) { - Long numRequests = - getNumRequests( - runner.getBaseUrl().toString(), - collection, - "ADMIN", - adminPathToMbean.get(adminPath), - adminPath, - true); + Double numRequests = + SolrJMetricTestUtils.getNumNodeRequestErrors( + runner.getBaseUrl().toString(), SolrRequestType.ADMIN.name(), adminPath); errorsAfter += numRequests; if (log.isInfoEnabled()) { log.info( 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 d0a52e9322b..980a326a32f 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 @@ -168,7 +168,9 @@ 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();
