From 835cf9c60f46fdb5a33e9b87cd575f22d48afead Mon Sep 17 00:00:00 2001 From: Matthew Biscocho Date: Fri, 15 Aug 2025 12:03:48 -0400 Subject: [PATCH 1/8] Support node level metric rollups --- .../solr/handler/RequestHandlerBase.java | 159 ++++++++++++------ .../solr/handler/component/SearchHandler.java | 5 +- .../instruments/AttributedDoubleCounter.java | 2 +- .../AttributedDoubleUpDownCounter.java | 4 +- .../instruments/AttributedLongCounter.java | 2 +- .../AttributedLongUpDownCounter.java | 4 +- .../DualRegistryAttributedLongCounter.java | 50 ++++++ .../DualRegistryAttributedLongTimer.java | 44 +++++ .../solr/handler/RequestHandlerBaseTest.java | 4 +- .../handler/RequestHandlerMetricsTest.java | 154 ++++++----------- 10 files changed, 270 insertions(+), 158 deletions(-) create mode 100644 solr/core/src/java/org/apache/solr/metrics/otel/instruments/DualRegistryAttributedLongCounter.java create mode 100644 solr/core/src/java/org/apache/solr/metrics/otel/instruments/DualRegistryAttributedLongTimer.java 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 4ac39cda833..d11b3b8e7c0 100644 --- a/solr/core/src/java/org/apache/solr/handler/RequestHandlerBase.java +++ b/solr/core/src/java/org/apache/solr/handler/RequestHandlerBase.java @@ -41,13 +41,13 @@ import org.apache.solr.core.PluginInfo; import org.apache.solr.core.SolrCore; import org.apache.solr.core.SolrInfoBean; -import org.apache.solr.metrics.SolrDelegateRegistryMetricsContext; import org.apache.solr.metrics.SolrMetricManager; -import org.apache.solr.metrics.SolrMetricProducer; import org.apache.solr.metrics.SolrMetricsContext; import org.apache.solr.metrics.otel.OtelUnit; import org.apache.solr.metrics.otel.instruments.AttributedLongCounter; import org.apache.solr.metrics.otel.instruments.AttributedLongTimer; +import org.apache.solr.metrics.otel.instruments.DualRegistryAttributedLongCounter; +import org.apache.solr.metrics.otel.instruments.DualRegistryAttributedLongTimer; import org.apache.solr.request.SolrQueryRequest; import org.apache.solr.request.SolrRequestHandler; import org.apache.solr.response.SolrQueryResponse; @@ -167,21 +167,13 @@ public SolrMetricsContext getSolrMetricsContext() { @Override public void initializeMetrics( SolrMetricsContext parentContext, Attributes attributes, String scope) { - if (aggregateNodeLevelMetricsEnabled) { - this.solrMetricsContext = - new SolrDelegateRegistryMetricsContext( - parentContext.getMetricManager(), - parentContext.getRegistryName(), - SolrMetricProducer.getUniqueMetricTag(this, parentContext.getTag()), - SolrMetricManager.getRegistryName(SolrInfoBean.Group.node)); - } else { - this.solrMetricsContext = parentContext.getChildContext(this); - } + this.solrMetricsContext = parentContext.getChildContext(this); metrics = new HandlerMetrics( solrMetricsContext, - attributes.toBuilder().put(CATEGORY_ATTR, getCategory().toString()).build()); + attributes.toBuilder().put(CATEGORY_ATTR, getCategory().toString()).build(), + aggregateNodeLevelMetricsEnabled); // NOCOMMIT: I don't see value in this metric solrMetricsContext.gauge( @@ -201,53 +193,124 @@ public static class HandlerMetrics { 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", OtelUnit.MILLISECONDS); + public HandlerMetrics( + SolrMetricsContext solrMetricsContext, + Attributes coreAttributes, + boolean aggregateNodeLevelMetricsEnabled) { + + if (aggregateNodeLevelMetricsEnabled) { + createDualRegistryMetrics(solrMetricsContext, coreAttributes); } 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", OtelUnit.MILLISECONDS); + createSingleRegistryMetrics(solrMetricsContext, coreAttributes); } + } - requests = new AttributedLongCounter(requestMetric, attributes); + private void createDualRegistryMetrics( + SolrMetricsContext solrCoreMetricsContext, Attributes coreAttributes) { + SolrMetricsContext solrNodeMetricsContext = + new SolrMetricsContext( + solrCoreMetricsContext.getMetricManager(), + SolrMetricManager.getRegistryName(SolrInfoBean.Group.node), + null); + + Attributes nodeAttributes = + Attributes.builder() + .put(CATEGORY_ATTR, coreAttributes.get(CATEGORY_ATTR)) + .put(HANDLER_ATTR, coreAttributes.get(HANDLER_ATTR)) + .build(); + + requests = + new DualRegistryAttributedLongCounter( + requestCounter(solrCoreMetricsContext, false), coreAttributes, + requestCounter(solrNodeMetricsContext, true), nodeAttributes); + numServerErrors = + new DualRegistryAttributedLongCounter( + requestErrorCounter(solrCoreMetricsContext, false), + coreAttributes.toBuilder().put(AttributeKey.stringKey("source"), "server").build(), + requestErrorCounter(solrNodeMetricsContext, true), + nodeAttributes.toBuilder().put(AttributeKey.stringKey("source"), "server").build()); + + numClientErrors = + new DualRegistryAttributedLongCounter( + requestErrorCounter(solrCoreMetricsContext, false), + coreAttributes.toBuilder().put(AttributeKey.stringKey("source"), "client").build(), + requestErrorCounter(solrNodeMetricsContext, true), + nodeAttributes.toBuilder().put(AttributeKey.stringKey("source"), "client").build()); + numTimeouts = + new DualRegistryAttributedLongCounter( + requestTimeoutCounter(solrCoreMetricsContext, false), coreAttributes, + requestTimeoutCounter(solrNodeMetricsContext, true), nodeAttributes); + requestTimes = + new DualRegistryAttributedLongTimer( + requestTimeHistogram(solrCoreMetricsContext, false), coreAttributes, + requestTimeHistogram(solrNodeMetricsContext, true), nodeAttributes); + } + private void createSingleRegistryMetrics( + SolrMetricsContext solrMetricsContext, Attributes coreAttributes) { + boolean isNodeRegistry = + solrMetricsContext + .getRegistryName() + .equals(SolrMetricManager.getRegistryName(SolrInfoBean.Group.node)); + + Attributes attributes = + isNodeRegistry + ? Attributes.builder() + .put(CATEGORY_ATTR, coreAttributes.get(CATEGORY_ATTR)) + .put(HANDLER_ATTR, coreAttributes.get(HANDLER_ATTR)) + .build() + : coreAttributes; + + requests = + new AttributedLongCounter(requestCounter(solrMetricsContext, isNodeRegistry), attributes); numServerErrors = new AttributedLongCounter( - errorRequestMetric, + requestErrorCounter(solrMetricsContext, isNodeRegistry), attributes.toBuilder().put(AttributeKey.stringKey("source"), "server").build()); - numClientErrors = new AttributedLongCounter( - errorRequestMetric, + requestErrorCounter(solrMetricsContext, isNodeRegistry), attributes.toBuilder().put(AttributeKey.stringKey("source"), "client").build()); + numTimeouts = + new AttributedLongCounter( + requestTimeoutCounter(solrMetricsContext, isNodeRegistry), attributes); + requestTimes = + new AttributedLongTimer( + requestTimeHistogram(solrMetricsContext, isNodeRegistry), attributes); + } + + private LongCounter requestCounter( + SolrMetricsContext solrMetricsContext, boolean isNodeRegistry) { + return (isNodeRegistry) + ? solrMetricsContext.longCounter("solr_node_requests", "HTTP Solr node requests", null) + : solrMetricsContext.longCounter("solr_core_requests", "HTTP Solr core requests"); + } + + private LongCounter requestErrorCounter( + SolrMetricsContext solrMetricsContext, boolean isNodeRegistry) { + return (isNodeRegistry) + ? solrMetricsContext.longCounter( + "solr_node_requests_errors", "HTTP Solr node request errors", null) + : solrMetricsContext.longCounter( + "solr_core_requests_errors", "HTTP Solr core request errors"); + } - numTimeouts = new AttributedLongCounter(timeoutRequestMetric, attributes); + private LongCounter requestTimeoutCounter( + SolrMetricsContext solrMetricsContext, boolean isNodeRegistry) { + return (isNodeRegistry) + ? solrMetricsContext.longCounter( + "solr_node_requests_timeout", "HTTP Solr node request timeouts", null) + : solrMetricsContext.longCounter( + "solr_core_requests_timeout", "HTTP Solr core request timeouts"); + } - requestTimes = new AttributedLongTimer(requestTimeMetric, attributes); + private LongHistogram requestTimeHistogram( + SolrMetricsContext solrMetricsContext, boolean isNodeRegistry) { + return (isNodeRegistry) + ? solrMetricsContext.longHistogram( + "solr_node_requests_times", "HTTP Solr node request times", OtelUnit.MILLISECONDS) + : solrMetricsContext.longHistogram( + "solr_core_requests_times", "HTTP Solr core request times", OtelUnit.MILLISECONDS); } } 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 b42e62d187b..cc7ad157651 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 @@ -98,8 +98,6 @@ public class SearchHandler extends RequestHandlerBase static final String INIT_FIRST_COMPONENTS = "first-components"; static final String INIT_LAST_COMPONENTS = "last-components"; - protected static final String SHARD_HANDLER_SUFFIX = "[shard]"; - private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); /** @@ -171,7 +169,8 @@ public void initializeMetrics( .putAll(attributes) .put(CATEGORY_ATTR, getCategory().toString()) .put(INTERNAL_ATTR, true) - .build()); + .build(), + false); } @Override diff --git a/solr/core/src/java/org/apache/solr/metrics/otel/instruments/AttributedDoubleCounter.java b/solr/core/src/java/org/apache/solr/metrics/otel/instruments/AttributedDoubleCounter.java index 43adf8ad087..bf345bb69df 100644 --- a/solr/core/src/java/org/apache/solr/metrics/otel/instruments/AttributedDoubleCounter.java +++ b/solr/core/src/java/org/apache/solr/metrics/otel/instruments/AttributedDoubleCounter.java @@ -30,7 +30,7 @@ public AttributedDoubleCounter(DoubleCounter counter, Attributes attributes) { } public void inc() { - add(1.0); + counter.add(1.0, attributes); } public void add(Double value) { diff --git a/solr/core/src/java/org/apache/solr/metrics/otel/instruments/AttributedDoubleUpDownCounter.java b/solr/core/src/java/org/apache/solr/metrics/otel/instruments/AttributedDoubleUpDownCounter.java index 23c0d3b9a53..02456e6c8fd 100644 --- a/solr/core/src/java/org/apache/solr/metrics/otel/instruments/AttributedDoubleUpDownCounter.java +++ b/solr/core/src/java/org/apache/solr/metrics/otel/instruments/AttributedDoubleUpDownCounter.java @@ -30,11 +30,11 @@ public AttributedDoubleUpDownCounter(DoubleUpDownCounter upDownCounter, Attribut } public void inc() { - add(1.0); + upDownCounter.add(1.0, attributes); } public void dec() { - add(-1.0); + upDownCounter.add(-1.0, attributes); } public void add(Double value) { diff --git a/solr/core/src/java/org/apache/solr/metrics/otel/instruments/AttributedLongCounter.java b/solr/core/src/java/org/apache/solr/metrics/otel/instruments/AttributedLongCounter.java index f33b4784c53..82d0ba39d46 100644 --- a/solr/core/src/java/org/apache/solr/metrics/otel/instruments/AttributedLongCounter.java +++ b/solr/core/src/java/org/apache/solr/metrics/otel/instruments/AttributedLongCounter.java @@ -30,7 +30,7 @@ public AttributedLongCounter(LongCounter baseCounter, Attributes attributes) { } public void inc() { - add(1L); + baseCounter.add(1L, attributes); } public void add(Long value) { diff --git a/solr/core/src/java/org/apache/solr/metrics/otel/instruments/AttributedLongUpDownCounter.java b/solr/core/src/java/org/apache/solr/metrics/otel/instruments/AttributedLongUpDownCounter.java index 91487c9e677..daf69abfb9d 100644 --- a/solr/core/src/java/org/apache/solr/metrics/otel/instruments/AttributedLongUpDownCounter.java +++ b/solr/core/src/java/org/apache/solr/metrics/otel/instruments/AttributedLongUpDownCounter.java @@ -30,11 +30,11 @@ public AttributedLongUpDownCounter(LongUpDownCounter upDownCounter, Attributes a } public void inc() { - add(1L); + upDownCounter.add(1L, attributes); } public void dec() { - add(-1L); + upDownCounter.add(-1L, attributes); } public void add(Long value) { diff --git a/solr/core/src/java/org/apache/solr/metrics/otel/instruments/DualRegistryAttributedLongCounter.java b/solr/core/src/java/org/apache/solr/metrics/otel/instruments/DualRegistryAttributedLongCounter.java new file mode 100644 index 00000000000..b7cc1379479 --- /dev/null +++ b/solr/core/src/java/org/apache/solr/metrics/otel/instruments/DualRegistryAttributedLongCounter.java @@ -0,0 +1,50 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.solr.metrics.otel.instruments; + +import io.opentelemetry.api.common.Attributes; +import io.opentelemetry.api.metrics.LongCounter; + +/** + * An AttributedLongCounter that writes to both core and node registries with corresponding + * attributes. + */ +public class DualRegistryAttributedLongCounter extends AttributedLongCounter { + + private final AttributedLongCounter nodeCounter; + + public DualRegistryAttributedLongCounter( + LongCounter coreCounter, + Attributes coreAttributes, + LongCounter nodeCounter, + Attributes nodeAttributes) { + super(coreCounter, coreAttributes); + this.nodeCounter = new AttributedLongCounter(nodeCounter, nodeAttributes); + } + + @Override + public void inc() { + super.inc(); + nodeCounter.inc(); + } + + @Override + public void add(Long value) { + super.add(value); + nodeCounter.add(value); + } +} diff --git a/solr/core/src/java/org/apache/solr/metrics/otel/instruments/DualRegistryAttributedLongTimer.java b/solr/core/src/java/org/apache/solr/metrics/otel/instruments/DualRegistryAttributedLongTimer.java new file mode 100644 index 00000000000..73f45e3d533 --- /dev/null +++ b/solr/core/src/java/org/apache/solr/metrics/otel/instruments/DualRegistryAttributedLongTimer.java @@ -0,0 +1,44 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.solr.metrics.otel.instruments; + +import io.opentelemetry.api.common.Attributes; +import io.opentelemetry.api.metrics.LongHistogram; + +/** + * An AttributedLongTimer that records to both core and node registries with corresponding + * attributes. + */ +public class DualRegistryAttributedLongTimer extends AttributedLongTimer { + + private final AttributedLongTimer nodeTimer; + + public DualRegistryAttributedLongTimer( + LongHistogram coreHistogram, + Attributes coreAttributes, + LongHistogram nodeHistogram, + Attributes nodeAttributes) { + super(coreHistogram, coreAttributes); + this.nodeTimer = new AttributedLongTimer(nodeHistogram, nodeAttributes); + } + + @Override + public void record(Long value) { + super.record(value); + nodeTimer.record(value); + } +} 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 74d3d6b2510..951c7f69385 100644 --- a/solr/core/src/test/org/apache/solr/handler/RequestHandlerBaseTest.java +++ b/solr/core/src/test/org/apache/solr/handler/RequestHandlerBaseTest.java @@ -196,6 +196,8 @@ private RequestHandlerBase.HandlerMetrics createHandlerMetrics() { when(metricsContext.longHistogram(any(), any())).thenReturn(mockLongHistogram); return new RequestHandlerBase.HandlerMetrics( - metricsContext, Attributes.of(AttributeKey.stringKey("/handler"), "/someBaseMetricPath")); + metricsContext, + Attributes.of(AttributeKey.stringKey("/handler"), "/someBaseMetricPath"), + false); } } 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 467094ccd62..b840605395c 100644 --- a/solr/core/src/test/org/apache/solr/handler/RequestHandlerMetricsTest.java +++ b/solr/core/src/test/org/apache/solr/handler/RequestHandlerMetricsTest.java @@ -16,19 +16,17 @@ */ package org.apache.solr.handler; +import io.prometheus.metrics.model.snapshots.CounterSnapshot; +import io.prometheus.metrics.model.snapshots.Labels; import java.io.IOException; -import java.util.HashSet; -import java.util.Map; -import java.util.Set; import org.apache.solr.client.solrj.SolrQuery; -import org.apache.solr.client.solrj.SolrRequest; import org.apache.solr.client.solrj.SolrServerException; import org.apache.solr.client.solrj.impl.CloudSolrClient; 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.util.NamedList; +import org.apache.solr.core.SolrCore; +import org.apache.solr.metrics.SolrMetricTestUtils; import org.junit.AfterClass; import org.junit.Before; import org.junit.BeforeClass; @@ -54,11 +52,8 @@ public static void afterClass() { System.clearProperty("metricsEnabled"); } - // NOCOMMIT: Need to fix aggregateNodeLevelMetricsEnabled for OTEL. Delegate registries currently - // do not play nice with OTEL instrumentation + // Test for dual registry metrics (core + node level aggregation) @Test - @AwaitsFix(bugUrl = "https://issues.apache.org/jira/browse/SOLR-17458") - @SuppressWarnings({"unchecked"}) public void testAggregateNodeLevelMetrics() throws SolrServerException, IOException { String collection1 = "testRequestHandlerMetrics1"; String collection2 = "testRequestHandlerMetrics2"; @@ -83,97 +78,56 @@ public void testAggregateNodeLevelMetrics() throws SolrServerException, IOExcept cloudClient.query(collection1, solrQuery); cloudClient.query(collection2, solrQuery); - NamedList response = - cloudClient.request( - new GenericSolrRequest( - SolrRequest.METHOD.GET, "/admin/metrics", SolrRequest.SolrRequestType.ADMIN)); - - NamedList metrics = (NamedList) response.get("metrics"); - - final double[] minQueryTime = {Double.MAX_VALUE}; - final double[] maxQueryTime = {-1.0}; - final double[] minUpdateTime = {Double.MAX_VALUE}; - final double[] maxUpdateTime = {-1.0}; - Set> coreMetrics = new HashSet<>(); - metrics.forEach( - (key, coreMetric) -> { - if (key.startsWith("solr.core.testRequestHandlerMetrics")) { - coreMetrics.add((NamedList) coreMetric); - } - }); - assertEquals(2, coreMetrics.size()); - coreMetrics.forEach( - metric -> { - assertEquals( - 1L, - ((Map) metric.get("QUERY./select.requestTimes")) - .get("count") - .longValue()); - minQueryTime[0] = - Math.min( - minQueryTime[0], - ((Map) metric.get("QUERY./select.requestTimes")) - .get("min_ms") - .doubleValue()); - maxQueryTime[0] = - Math.max( - maxQueryTime[0], - ((Map) metric.get("QUERY./select.requestTimes")) - .get("max_ms") - .doubleValue()); - assertEquals( - 1L, - ((Map) metric.get("UPDATE./update.requestTimes")) - .get("count") - .longValue()); - minUpdateTime[0] = - Math.min( - minUpdateTime[0], - ((Map) metric.get("UPDATE./update.requestTimes")) - .get("min_ms") - .doubleValue()); - maxUpdateTime[0] = - Math.max( - maxUpdateTime[0], - ((Map) metric.get("UPDATE./update.requestTimes")) - .get("max_ms") - .doubleValue()); - }); - - NamedList nodeMetrics = (NamedList) metrics.get("solr.node"); - assertEquals( - 2L, - ((Map) nodeMetrics.get("QUERY./select.requestTimes")) - .get("count") - .longValue()); - assertEquals( - minQueryTime[0], - ((Map) nodeMetrics.get("QUERY./select.requestTimes")) - .get("min_ms") - .doubleValue(), - 0.0); - assertEquals( - maxQueryTime[0], - ((Map) nodeMetrics.get("QUERY./select.requestTimes")) - .get("max_ms") - .doubleValue(), - 0.0); - assertEquals( - 2L, - ((Map) nodeMetrics.get("UPDATE./update.requestTimes")) - .get("count") - .longValue()); + var coreContainer = cluster.getJettySolrRunners().get(0).getCoreContainer(); + SolrCore core1 = coreContainer.getCore(coreContainer.getAllCoreNames().get(0)); + SolrCore core2 = coreContainer.getCore(coreContainer.getAllCoreNames().get(1)); + + CounterSnapshot.CounterDataPointSnapshot actualCore1Selects = + SolrMetricTestUtils.newCloudSelectRequestsDatapoint(core1); + CounterSnapshot.CounterDataPointSnapshot actualCore1Updates = + SolrMetricTestUtils.newCloudUpdateRequestsDatapoint(core1); + CounterSnapshot.CounterDataPointSnapshot actualCore2Selects = + SolrMetricTestUtils.newCloudSelectRequestsDatapoint(core2); + CounterSnapshot.CounterDataPointSnapshot actualCore2Updates = + SolrMetricTestUtils.newCloudUpdateRequestsDatapoint(core2); + + assertEquals(1.0, actualCore1Selects.getValue(), 0.0); + assertEquals(1.0, actualCore1Updates.getValue(), 0.0); + assertEquals(1.0, actualCore2Updates.getValue(), 0.0); + assertEquals(1.0, actualCore2Selects.getValue(), 0.0); + + // Get node metrics and the select/update requests should be the sum of both cores requests + var nodeReader = SolrMetricTestUtils.getPrometheusMetricReader(coreContainer, "solr.node"); + + CounterSnapshot.CounterDataPointSnapshot nodeSelectRequests = + (CounterSnapshot.CounterDataPointSnapshot) + SolrMetricTestUtils.getDataPointSnapshot( + nodeReader, + "solr_node_requests", + Labels.builder() + .label("category", "QUERY") + .label("handler", "/select") + .label("otel_scope_name", "org.apache.solr") + .build()); + CounterSnapshot.CounterDataPointSnapshot nodeUpdateRequests = + (CounterSnapshot.CounterDataPointSnapshot) + SolrMetricTestUtils.getDataPointSnapshot( + nodeReader, + "solr_node_requests", + Labels.builder() + .label("category", "UPDATE") + .label("handler", "/update") + .label("otel_scope_name", "org.apache.solr") + .build()); + + assertNotNull("Node select requests should be recorded", nodeSelectRequests); + assertNotNull("Node update requests should be recorded", nodeUpdateRequests); assertEquals( - minUpdateTime[0], - ((Map) nodeMetrics.get("UPDATE./update.requestTimes")) - .get("min_ms") - .doubleValue(), - 0.0); + "Node should have 2 select requests total", 2.0, nodeSelectRequests.getValue(), 0.0); assertEquals( - maxUpdateTime[0], - ((Map) nodeMetrics.get("UPDATE./update.requestTimes")) - .get("max_ms") - .doubleValue(), - 0.0); + "Node should have 2 update requests total", 2.0, nodeUpdateRequests.getValue(), 0.0); + + core1.close(); + core2.close(); } } From 82ce8e943face3a2a94dfd3b22873459f761c636 Mon Sep 17 00:00:00 2001 From: Matthew Biscocho Date: Fri, 15 Aug 2025 12:07:29 -0400 Subject: [PATCH 2/8] Cleanup --- .../java/org/apache/solr/handler/RequestHandlerBase.java | 6 +++--- .../org/apache/solr/handler/RequestHandlerMetricsTest.java | 7 ++----- 2 files changed, 5 insertions(+), 8 deletions(-) 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 d11b3b8e7c0..ce8b04cafec 100644 --- a/solr/core/src/java/org/apache/solr/handler/RequestHandlerBase.java +++ b/solr/core/src/java/org/apache/solr/handler/RequestHandlerBase.java @@ -282,7 +282,7 @@ private void createSingleRegistryMetrics( private LongCounter requestCounter( SolrMetricsContext solrMetricsContext, boolean isNodeRegistry) { return (isNodeRegistry) - ? solrMetricsContext.longCounter("solr_node_requests", "HTTP Solr node requests", null) + ? solrMetricsContext.longCounter("solr_node_requests", "HTTP Solr node requests") : solrMetricsContext.longCounter("solr_core_requests", "HTTP Solr core requests"); } @@ -290,7 +290,7 @@ private LongCounter requestErrorCounter( SolrMetricsContext solrMetricsContext, boolean isNodeRegistry) { return (isNodeRegistry) ? solrMetricsContext.longCounter( - "solr_node_requests_errors", "HTTP Solr node request errors", null) + "solr_node_requests_errors", "HTTP Solr node request errors") : solrMetricsContext.longCounter( "solr_core_requests_errors", "HTTP Solr core request errors"); } @@ -299,7 +299,7 @@ private LongCounter requestTimeoutCounter( SolrMetricsContext solrMetricsContext, boolean isNodeRegistry) { return (isNodeRegistry) ? solrMetricsContext.longCounter( - "solr_node_requests_timeout", "HTTP Solr node request timeouts", null) + "solr_node_requests_timeout", "HTTP Solr node request timeouts") : solrMetricsContext.longCounter( "solr_core_requests_timeout", "HTTP Solr core request timeouts"); } 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 b840605395c..6a85abd322e 100644 --- a/solr/core/src/test/org/apache/solr/handler/RequestHandlerMetricsTest.java +++ b/solr/core/src/test/org/apache/solr/handler/RequestHandlerMetricsTest.java @@ -52,7 +52,6 @@ public static void afterClass() { System.clearProperty("metricsEnabled"); } - // Test for dual registry metrics (core + node level aggregation) @Test public void testAggregateNodeLevelMetrics() throws SolrServerException, IOException { String collection1 = "testRequestHandlerMetrics1"; @@ -122,10 +121,8 @@ public void testAggregateNodeLevelMetrics() throws SolrServerException, IOExcept assertNotNull("Node select requests should be recorded", nodeSelectRequests); assertNotNull("Node update requests should be recorded", nodeUpdateRequests); - assertEquals( - "Node should have 2 select requests total", 2.0, nodeSelectRequests.getValue(), 0.0); - assertEquals( - "Node should have 2 update requests total", 2.0, nodeUpdateRequests.getValue(), 0.0); + assertEquals(2.0, nodeSelectRequests.getValue(), 0.0); + assertEquals(2.0, nodeUpdateRequests.getValue(), 0.0); core1.close(); core2.close(); From 87e81edaa7e8c816f10b4be54912a90fff2612bd Mon Sep 17 00:00:00 2001 From: Matthew Biscocho Date: Sun, 5 Oct 2025 13:40:16 -0400 Subject: [PATCH 3/8] Rebase misses --- .../src/java/org/apache/solr/handler/RequestHandlerBase.java | 3 ++- .../org/apache/solr/handler/RequestHandlerMetricsTest.java | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) 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 ce8b04cafec..09fbbc6c0c7 100644 --- a/solr/core/src/java/org/apache/solr/handler/RequestHandlerBase.java +++ b/solr/core/src/java/org/apache/solr/handler/RequestHandlerBase.java @@ -185,7 +185,8 @@ public static class HandlerMetrics { public static final HandlerMetrics NO_OP = new HandlerMetrics( new SolrMetricsContext(new SolrMetricManager(null), "NO_OP", "NO_OP"), - Attributes.empty()); + Attributes.empty(), + false); public AttributedLongCounter requests; public AttributedLongCounter numServerErrors; 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 6a85abd322e..0c6412e322a 100644 --- a/solr/core/src/test/org/apache/solr/handler/RequestHandlerMetricsTest.java +++ b/solr/core/src/test/org/apache/solr/handler/RequestHandlerMetricsTest.java @@ -26,7 +26,7 @@ import org.apache.solr.cloud.SolrCloudTestCase; import org.apache.solr.common.SolrInputDocument; import org.apache.solr.core.SolrCore; -import org.apache.solr.metrics.SolrMetricTestUtils; +import org.apache.solr.util.SolrMetricTestUtils; import org.junit.AfterClass; import org.junit.Before; import org.junit.BeforeClass; From e035c924aa59a5dc2007900d6f2da587d3db527e Mon Sep 17 00:00:00 2001 From: Matthew Biscocho Date: Thu, 9 Oct 2025 15:28:04 -0400 Subject: [PATCH 4/8] Factory method and DirectUpdateHandler2 --- .../solr/handler/RequestHandlerBase.java | 142 ++++--------- .../instruments/AttributedDoubleCounter.java | 4 +- .../AttributedDoubleUpDownCounter.java | 8 +- .../AttributedInstrumentFactory.java | 184 +++++++++++++++++ .../instruments/AttributedLongCounter.java | 4 +- .../AttributedLongUpDownCounter.java | 8 +- .../DualRegistryAttributedLongCounter.java | 7 +- .../DualRegistryAttributedLongTimer.java | 1 + ...alRegistryAttributedLongUpDownCounter.java | 45 +++++ .../solr/update/DirectUpdateHandler2.java | 191 ++++++++++-------- .../conf/solrconfig.xml | 5 +- .../solr/handler/RequestHandlerBaseTest.java | 5 +- .../handler/RequestHandlerMetricsTest.java | 30 +++ 13 files changed, 417 insertions(+), 217 deletions(-) create mode 100644 solr/core/src/java/org/apache/solr/metrics/otel/instruments/AttributedInstrumentFactory.java create mode 100644 solr/core/src/java/org/apache/solr/metrics/otel/instruments/DualRegistryAttributedLongUpDownCounter.java 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 f3ac368825a..54b836c9a3e 100644 --- a/solr/core/src/java/org/apache/solr/handler/RequestHandlerBase.java +++ b/solr/core/src/java/org/apache/solr/handler/RequestHandlerBase.java @@ -21,8 +21,6 @@ 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.io.IOException; import java.lang.invoke.MethodHandles; import java.util.Collection; @@ -44,10 +42,9 @@ import org.apache.solr.metrics.SolrMetricManager; import org.apache.solr.metrics.SolrMetricsContext; import org.apache.solr.metrics.otel.OtelUnit; +import org.apache.solr.metrics.otel.instruments.AttributedInstrumentFactory; import org.apache.solr.metrics.otel.instruments.AttributedLongCounter; import org.apache.solr.metrics.otel.instruments.AttributedLongTimer; -import org.apache.solr.metrics.otel.instruments.DualRegistryAttributedLongCounter; -import org.apache.solr.metrics.otel.instruments.DualRegistryAttributedLongTimer; import org.apache.solr.request.SolrQueryRequest; import org.apache.solr.request.SolrRequestHandler; import org.apache.solr.response.SolrQueryResponse; @@ -72,6 +69,7 @@ public abstract class RequestHandlerBase PermissionNameProvider { public static final String REQUEST_CPU_TIMER_CONTEXT = "publishCpuTime"; + public static final AttributeKey SOURCE_ATTR = AttributeKey.stringKey("source"); protected NamedList initArgs = null; protected SolrParams defaults; protected SolrParams appends; @@ -195,119 +193,47 @@ public HandlerMetrics( Attributes coreAttributes, boolean aggregateNodeLevelMetricsEnabled) { - if (aggregateNodeLevelMetricsEnabled) { - createDualRegistryMetrics(solrMetricsContext, coreAttributes); - } else { - createSingleRegistryMetrics(solrMetricsContext, coreAttributes); - } - } - - private void createDualRegistryMetrics( - SolrMetricsContext solrCoreMetricsContext, Attributes coreAttributes) { - SolrMetricsContext solrNodeMetricsContext = - new SolrMetricsContext( - solrCoreMetricsContext.getMetricManager(), - SolrMetricManager.getRegistryName(SolrInfoBean.Group.node), - null); + AttributedInstrumentFactory factory = + new AttributedInstrumentFactory( + solrMetricsContext, coreAttributes, aggregateNodeLevelMetricsEnabled); - Attributes nodeAttributes = - Attributes.builder() - .put(CATEGORY_ATTR, coreAttributes.get(CATEGORY_ATTR)) - .put(HANDLER_ATTR, coreAttributes.get(HANDLER_ATTR)) - .build(); + String corePrefix = "solr_core_"; + String nodePrefix = "solr_node_"; requests = - new DualRegistryAttributedLongCounter( - requestCounter(solrCoreMetricsContext, false), coreAttributes, - requestCounter(solrNodeMetricsContext, true), nodeAttributes); + factory.attributedLongCounter( + AttributedInstrumentFactory.standardNameProvider(corePrefix, nodePrefix, "requests"), + "HTTP Solr requests", + Attributes.empty()); + numServerErrors = - new DualRegistryAttributedLongCounter( - requestErrorCounter(solrCoreMetricsContext, false), - coreAttributes.toBuilder().put(AttributeKey.stringKey("source"), "server").build(), - requestErrorCounter(solrNodeMetricsContext, true), - nodeAttributes.toBuilder().put(AttributeKey.stringKey("source"), "server").build()); + factory.attributedLongCounter( + AttributedInstrumentFactory.standardNameProvider( + corePrefix, nodePrefix, "requests_errors"), + "HTTP Solr request errors", + Attributes.of(SOURCE_ATTR, "server")); numClientErrors = - new DualRegistryAttributedLongCounter( - requestErrorCounter(solrCoreMetricsContext, false), - coreAttributes.toBuilder().put(AttributeKey.stringKey("source"), "client").build(), - requestErrorCounter(solrNodeMetricsContext, true), - nodeAttributes.toBuilder().put(AttributeKey.stringKey("source"), "client").build()); - numTimeouts = - new DualRegistryAttributedLongCounter( - requestTimeoutCounter(solrCoreMetricsContext, false), coreAttributes, - requestTimeoutCounter(solrNodeMetricsContext, true), nodeAttributes); - requestTimes = - new DualRegistryAttributedLongTimer( - requestTimeHistogram(solrCoreMetricsContext, false), coreAttributes, - requestTimeHistogram(solrNodeMetricsContext, true), nodeAttributes); - } - - private void createSingleRegistryMetrics( - SolrMetricsContext solrMetricsContext, Attributes coreAttributes) { - boolean isNodeRegistry = - solrMetricsContext - .getRegistryName() - .equals(SolrMetricManager.getRegistryName(SolrInfoBean.Group.node)); - - Attributes attributes = - isNodeRegistry - ? Attributes.builder() - .put(CATEGORY_ATTR, coreAttributes.get(CATEGORY_ATTR)) - .put(HANDLER_ATTR, coreAttributes.get(HANDLER_ATTR)) - .build() - : coreAttributes; + factory.attributedLongCounter( + AttributedInstrumentFactory.standardNameProvider( + corePrefix, nodePrefix, "requests_errors"), + "HTTP Solr request errors", + Attributes.of(SOURCE_ATTR, "client")); - requests = - new AttributedLongCounter(requestCounter(solrMetricsContext, isNodeRegistry), attributes); - numServerErrors = - new AttributedLongCounter( - requestErrorCounter(solrMetricsContext, isNodeRegistry), - attributes.toBuilder().put(AttributeKey.stringKey("source"), "server").build()); - numClientErrors = - new AttributedLongCounter( - requestErrorCounter(solrMetricsContext, isNodeRegistry), - attributes.toBuilder().put(AttributeKey.stringKey("source"), "client").build()); numTimeouts = - new AttributedLongCounter( - requestTimeoutCounter(solrMetricsContext, isNodeRegistry), attributes); - requestTimes = - new AttributedLongTimer( - requestTimeHistogram(solrMetricsContext, isNodeRegistry), attributes); - } - - private LongCounter requestCounter( - SolrMetricsContext solrMetricsContext, boolean isNodeRegistry) { - return (isNodeRegistry) - ? solrMetricsContext.longCounter("solr_node_requests", "HTTP Solr node requests") - : solrMetricsContext.longCounter("solr_core_requests", "HTTP Solr core requests"); - } + factory.attributedLongCounter( + AttributedInstrumentFactory.standardNameProvider( + corePrefix, nodePrefix, "requests_timeout"), + "HTTP Solr request timeouts", + Attributes.empty()); - private LongCounter requestErrorCounter( - SolrMetricsContext solrMetricsContext, boolean isNodeRegistry) { - return (isNodeRegistry) - ? solrMetricsContext.longCounter( - "solr_node_requests_errors", "HTTP Solr node request errors") - : solrMetricsContext.longCounter( - "solr_core_requests_errors", "HTTP Solr core request errors"); - } - - private LongCounter requestTimeoutCounter( - SolrMetricsContext solrMetricsContext, boolean isNodeRegistry) { - return (isNodeRegistry) - ? solrMetricsContext.longCounter( - "solr_node_requests_timeout", "HTTP Solr node request timeouts") - : solrMetricsContext.longCounter( - "solr_core_requests_timeout", "HTTP Solr core request timeouts"); - } - - private LongHistogram requestTimeHistogram( - SolrMetricsContext solrMetricsContext, boolean isNodeRegistry) { - return (isNodeRegistry) - ? solrMetricsContext.longHistogram( - "solr_node_requests_times", "HTTP Solr node request times", OtelUnit.MILLISECONDS) - : solrMetricsContext.longHistogram( - "solr_core_requests_times", "HTTP Solr core request times", OtelUnit.MILLISECONDS); + requestTimes = + factory.attributedLongTimer( + AttributedInstrumentFactory.standardNameProvider( + "solr_core_", nodePrefix, "requests_times"), + "HTTP Solr request times", + OtelUnit.MILLISECONDS, + Attributes.empty()); } } diff --git a/solr/core/src/java/org/apache/solr/metrics/otel/instruments/AttributedDoubleCounter.java b/solr/core/src/java/org/apache/solr/metrics/otel/instruments/AttributedDoubleCounter.java index bf345bb69df..e2f28293e4a 100644 --- a/solr/core/src/java/org/apache/solr/metrics/otel/instruments/AttributedDoubleCounter.java +++ b/solr/core/src/java/org/apache/solr/metrics/otel/instruments/AttributedDoubleCounter.java @@ -29,8 +29,8 @@ public AttributedDoubleCounter(DoubleCounter counter, Attributes attributes) { this.attributes = attributes; } - public void inc() { - counter.add(1.0, attributes); + public final void inc() { + add(1.0); } public void add(Double value) { diff --git a/solr/core/src/java/org/apache/solr/metrics/otel/instruments/AttributedDoubleUpDownCounter.java b/solr/core/src/java/org/apache/solr/metrics/otel/instruments/AttributedDoubleUpDownCounter.java index 02456e6c8fd..ca2808a596f 100644 --- a/solr/core/src/java/org/apache/solr/metrics/otel/instruments/AttributedDoubleUpDownCounter.java +++ b/solr/core/src/java/org/apache/solr/metrics/otel/instruments/AttributedDoubleUpDownCounter.java @@ -29,12 +29,12 @@ public AttributedDoubleUpDownCounter(DoubleUpDownCounter upDownCounter, Attribut this.attributes = attributes; } - public void inc() { - upDownCounter.add(1.0, attributes); + public final void inc() { + add(1.0); } - public void dec() { - upDownCounter.add(-1.0, attributes); + public final void dec() { + add(-1.0); } public void add(Double value) { diff --git a/solr/core/src/java/org/apache/solr/metrics/otel/instruments/AttributedInstrumentFactory.java b/solr/core/src/java/org/apache/solr/metrics/otel/instruments/AttributedInstrumentFactory.java new file mode 100644 index 00000000000..29deeb4a1ae --- /dev/null +++ b/solr/core/src/java/org/apache/solr/metrics/otel/instruments/AttributedInstrumentFactory.java @@ -0,0 +1,184 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.solr.metrics.otel.instruments; + +import static org.apache.solr.metrics.SolrMetricProducer.CATEGORY_ATTR; +import static org.apache.solr.metrics.SolrMetricProducer.HANDLER_ATTR; + +import io.opentelemetry.api.common.Attributes; +import io.opentelemetry.api.metrics.LongCounter; +import io.opentelemetry.api.metrics.LongHistogram; +import io.opentelemetry.api.metrics.LongUpDownCounter; +import org.apache.solr.core.SolrInfoBean; +import org.apache.solr.metrics.SolrMetricManager; +import org.apache.solr.metrics.SolrMetricsContext; +import org.apache.solr.metrics.otel.OtelUnit; + +/** + * Factory for creating metrics instruments that can write to either single or dual registries (core + * and node). + */ +public class AttributedInstrumentFactory { + + private final SolrMetricsContext primaryMetricsContext; + private final Attributes primaryAttributes; + private final boolean aggregateToNodeRegistry; + private final boolean primaryIsNodeRegistry; + private SolrMetricsContext nodeMetricsContext = null; + private Attributes nodeAttributes = null; + + public AttributedInstrumentFactory( + SolrMetricsContext primaryMetricsContext, + Attributes primaryAttributes, + boolean aggregateToNodeRegistry) { + this.primaryMetricsContext = primaryMetricsContext; + this.primaryAttributes = primaryAttributes; + this.aggregateToNodeRegistry = aggregateToNodeRegistry; + + // Primary could be a node + this.primaryIsNodeRegistry = + primaryMetricsContext + .getRegistryName() + .equals(SolrMetricManager.getRegistryName(SolrInfoBean.Group.node)); + + // Only create node registry if we want dual registry mode AND primary is not already a node + // registry + if (aggregateToNodeRegistry && !primaryIsNodeRegistry) { + this.nodeMetricsContext = + new SolrMetricsContext( + primaryMetricsContext.getMetricManager(), + SolrMetricManager.getRegistryName(SolrInfoBean.Group.node), + null); + this.nodeAttributes = createNodeAttributes(primaryAttributes); + } + } + + public AttributedLongCounter attributedLongCounter( + MetricNameProvider metricNameProvider, String description, Attributes additionalAttributes) { + Attributes finalPrimaryAttrs = appendAttributes(primaryAttributes, additionalAttributes); + + if (aggregateToNodeRegistry && nodeMetricsContext != null) { + Attributes finalNodeAttrs = appendAttributes(nodeAttributes, additionalAttributes); + + LongCounter primaryCounter = + primaryMetricsContext.longCounter(metricNameProvider.getPrimaryMetricName(), description); + LongCounter nodeCounter = + nodeMetricsContext.longCounter(metricNameProvider.getNodeMetricName(), description); + return new DualRegistryAttributedLongCounter( + primaryCounter, finalPrimaryAttrs, nodeCounter, finalNodeAttrs); + } else { + String metricName = + primaryIsNodeRegistry + ? metricNameProvider.getNodeMetricName() + : metricNameProvider.getPrimaryMetricName(); + + LongCounter counter = primaryMetricsContext.longCounter(metricName, description); + return new AttributedLongCounter(counter, finalPrimaryAttrs); + } + } + + public AttributedLongUpDownCounter attributedLongUpDownCounter( + MetricNameProvider metricNameProvider, String description, Attributes additionalAttributes) { + Attributes finalPrimaryAttrs = appendAttributes(primaryAttributes, additionalAttributes); + + if (aggregateToNodeRegistry && nodeMetricsContext != null) { + Attributes finalNodeAttrs = appendAttributes(nodeAttributes, additionalAttributes); + + LongUpDownCounter primaryCounter = + primaryMetricsContext.longUpDownCounter( + metricNameProvider.getPrimaryMetricName(), description); + LongUpDownCounter nodeCounter = + nodeMetricsContext.longUpDownCounter(metricNameProvider.getNodeMetricName(), description); + + return new DualRegistryAttributedLongUpDownCounter( + primaryCounter, finalPrimaryAttrs, nodeCounter, finalNodeAttrs); + } else { + String metricName = + primaryIsNodeRegistry + ? metricNameProvider.getNodeMetricName() + : metricNameProvider.getPrimaryMetricName(); + + LongUpDownCounter counter = primaryMetricsContext.longUpDownCounter(metricName, description); + return new AttributedLongUpDownCounter(counter, finalPrimaryAttrs); + } + } + + public AttributedLongTimer attributedLongTimer( + MetricNameProvider metricNameProvider, + String description, + OtelUnit unit, + Attributes additionalAttributes) { + Attributes finalPrimaryAttrs = appendAttributes(primaryAttributes, additionalAttributes); + + if (aggregateToNodeRegistry) { + Attributes finalNodeAttrs = appendAttributes(nodeAttributes, additionalAttributes); + LongHistogram coreHistogram = + primaryMetricsContext.longHistogram( + metricNameProvider.getPrimaryMetricName(), description, unit); + LongHistogram nodeHistogram = + nodeMetricsContext.longHistogram( + metricNameProvider.getNodeMetricName(), description, unit); + return new DualRegistryAttributedLongTimer( + coreHistogram, finalPrimaryAttrs, nodeHistogram, finalNodeAttrs); + } else { + String metricName = + primaryIsNodeRegistry + ? metricNameProvider.getNodeMetricName() + : metricNameProvider.getPrimaryMetricName(); + + LongHistogram histogram = primaryMetricsContext.longHistogram(metricName, description, unit); + return new AttributedLongTimer(histogram, finalPrimaryAttrs); + } + } + + // Filter out core attributes and keep only category and handler if they exist + private Attributes createNodeAttributes(Attributes coreAttributes) { + var builder = Attributes.builder(); + + if (coreAttributes.get(CATEGORY_ATTR) != null) + builder.put(CATEGORY_ATTR, coreAttributes.get(CATEGORY_ATTR)); + if (coreAttributes.get(HANDLER_ATTR) != null) + builder.put(HANDLER_ATTR, coreAttributes.get(HANDLER_ATTR)); + + return builder.build(); + } + + private Attributes appendAttributes(Attributes base, Attributes additional) { + return base.toBuilder().putAll(additional).build(); + } + + public interface MetricNameProvider { + String getPrimaryMetricName(); + + String getNodeMetricName(); + } + + public static MetricNameProvider standardNameProvider( + String corePrefix, String nodePrefix, String metricSuffix) { + return new MetricNameProvider() { + @Override + public String getPrimaryMetricName() { + return corePrefix + metricSuffix; + } + + @Override + public String getNodeMetricName() { + return nodePrefix + metricSuffix; + } + }; + } +} diff --git a/solr/core/src/java/org/apache/solr/metrics/otel/instruments/AttributedLongCounter.java b/solr/core/src/java/org/apache/solr/metrics/otel/instruments/AttributedLongCounter.java index 82d0ba39d46..e2a477eac35 100644 --- a/solr/core/src/java/org/apache/solr/metrics/otel/instruments/AttributedLongCounter.java +++ b/solr/core/src/java/org/apache/solr/metrics/otel/instruments/AttributedLongCounter.java @@ -29,8 +29,8 @@ public AttributedLongCounter(LongCounter baseCounter, Attributes attributes) { this.attributes = attributes; } - public void inc() { - baseCounter.add(1L, attributes); + public final void inc() { + add(1L); } public void add(Long value) { diff --git a/solr/core/src/java/org/apache/solr/metrics/otel/instruments/AttributedLongUpDownCounter.java b/solr/core/src/java/org/apache/solr/metrics/otel/instruments/AttributedLongUpDownCounter.java index daf69abfb9d..e3ae3e98edd 100644 --- a/solr/core/src/java/org/apache/solr/metrics/otel/instruments/AttributedLongUpDownCounter.java +++ b/solr/core/src/java/org/apache/solr/metrics/otel/instruments/AttributedLongUpDownCounter.java @@ -29,12 +29,12 @@ public AttributedLongUpDownCounter(LongUpDownCounter upDownCounter, Attributes a this.attributes = attributes; } - public void inc() { - upDownCounter.add(1L, attributes); + public final void inc() { + add(1L); } - public void dec() { - upDownCounter.add(-1L, attributes); + public final void dec() { + add(-1L); } public void add(Long value) { diff --git a/solr/core/src/java/org/apache/solr/metrics/otel/instruments/DualRegistryAttributedLongCounter.java b/solr/core/src/java/org/apache/solr/metrics/otel/instruments/DualRegistryAttributedLongCounter.java index b7cc1379479..15fc27b85c5 100644 --- a/solr/core/src/java/org/apache/solr/metrics/otel/instruments/DualRegistryAttributedLongCounter.java +++ b/solr/core/src/java/org/apache/solr/metrics/otel/instruments/DualRegistryAttributedLongCounter.java @@ -33,15 +33,10 @@ public DualRegistryAttributedLongCounter( LongCounter nodeCounter, Attributes nodeAttributes) { super(coreCounter, coreAttributes); + assert coreCounter != nodeCounter; this.nodeCounter = new AttributedLongCounter(nodeCounter, nodeAttributes); } - @Override - public void inc() { - super.inc(); - nodeCounter.inc(); - } - @Override public void add(Long value) { super.add(value); diff --git a/solr/core/src/java/org/apache/solr/metrics/otel/instruments/DualRegistryAttributedLongTimer.java b/solr/core/src/java/org/apache/solr/metrics/otel/instruments/DualRegistryAttributedLongTimer.java index 73f45e3d533..b42d869892c 100644 --- a/solr/core/src/java/org/apache/solr/metrics/otel/instruments/DualRegistryAttributedLongTimer.java +++ b/solr/core/src/java/org/apache/solr/metrics/otel/instruments/DualRegistryAttributedLongTimer.java @@ -33,6 +33,7 @@ public DualRegistryAttributedLongTimer( LongHistogram nodeHistogram, Attributes nodeAttributes) { super(coreHistogram, coreAttributes); + assert coreHistogram != nodeHistogram; this.nodeTimer = new AttributedLongTimer(nodeHistogram, nodeAttributes); } diff --git a/solr/core/src/java/org/apache/solr/metrics/otel/instruments/DualRegistryAttributedLongUpDownCounter.java b/solr/core/src/java/org/apache/solr/metrics/otel/instruments/DualRegistryAttributedLongUpDownCounter.java new file mode 100644 index 00000000000..6cf505fb54e --- /dev/null +++ b/solr/core/src/java/org/apache/solr/metrics/otel/instruments/DualRegistryAttributedLongUpDownCounter.java @@ -0,0 +1,45 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.solr.metrics.otel.instruments; + +import io.opentelemetry.api.common.Attributes; +import io.opentelemetry.api.metrics.LongUpDownCounter; + +/** + * An AttributedLongUpDownCounter that writes to both core and node registries with corresponding + * attributes. + */ +public class DualRegistryAttributedLongUpDownCounter extends AttributedLongUpDownCounter { + + private final AttributedLongUpDownCounter nodeUpDownCounter; + + public DualRegistryAttributedLongUpDownCounter( + LongUpDownCounter coreUpDownCounter, + Attributes coreAttributes, + LongUpDownCounter nodeUpDownCounter, + Attributes nodeAttributes) { + super(coreUpDownCounter, coreAttributes); + assert coreUpDownCounter != nodeUpDownCounter; + this.nodeUpDownCounter = new AttributedLongUpDownCounter(nodeUpDownCounter, nodeAttributes); + } + + @Override + public void add(Long value) { + super.add(value); + nodeUpDownCounter.add(value); + } +} diff --git a/solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java b/solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java index 0a027282c46..e53b0c617b4 100644 --- a/solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java +++ b/solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java @@ -53,11 +53,9 @@ import org.apache.solr.common.util.NamedList; import org.apache.solr.core.SolrConfig.UpdateHandlerInfo; import org.apache.solr.core.SolrCore; -import org.apache.solr.core.SolrInfoBean; -import org.apache.solr.metrics.SolrDelegateRegistryMetricsContext; -import org.apache.solr.metrics.SolrMetricManager; import org.apache.solr.metrics.SolrMetricProducer; import org.apache.solr.metrics.SolrMetricsContext; +import org.apache.solr.metrics.otel.instruments.AttributedInstrumentFactory; import org.apache.solr.metrics.otel.instruments.AttributedLongCounter; import org.apache.solr.metrics.otel.instruments.AttributedLongUpDownCounter; import org.apache.solr.request.LocalSolrQueryRequest; @@ -227,84 +225,137 @@ public DirectUpdateHandler2(SolrCore core, UpdateHandler updateHandler) { @Override public void initializeMetrics( SolrMetricsContext parentContext, Attributes attributes, String scope) { - if (core.getSolrConfig().getUpdateHandlerInfo().aggregateNodeLevelMetricsEnabled) { - this.solrMetricsContext = - new SolrDelegateRegistryMetricsContext( - parentContext.getMetricManager(), - parentContext.getRegistryName(), - SolrMetricProducer.getUniqueMetricTag(this, parentContext.getTag()), - SolrMetricManager.getRegistryName(SolrInfoBean.Group.node)); - } else { - this.solrMetricsContext = parentContext.getChildContext(this); - } - final List observables = new ArrayList<>(); + this.solrMetricsContext = parentContext.getChildContext(this); var baseAttributes = attributes.toBuilder() .put(AttributeKey.stringKey("category"), getCategory().toString()) .build(); - var baseCommandsMetric = - solrMetricsContext.longUpDownCounter( - "solr_core_update_cumulative_ops", - "Cumulative number of update commands processed. Cumulative can decrease from rollback command"); + boolean aggregateNodeLevelMetricsEnabled = + core.getSolrConfig().getUpdateHandlerInfo().aggregateNodeLevelMetricsEnabled; + + createMetrics(baseAttributes, aggregateNodeLevelMetricsEnabled); + } + + private void createMetrics(Attributes baseAttributes, boolean aggregateToNodeRegistry) { + final List observables = new ArrayList<>(); + + AttributedInstrumentFactory factory = + new AttributedInstrumentFactory( + solrMetricsContext, baseAttributes, aggregateToNodeRegistry); + + String corePrefix = "solr_core_update"; + String nodePrefix = "solr_node_update_"; addCommandsCumulative = - new AttributedLongUpDownCounter( - baseCommandsMetric, baseAttributes.toBuilder().put(OPERATION_ATTR, "adds").build()); + factory.attributedLongUpDownCounter( + AttributedInstrumentFactory.standardNameProvider( + corePrefix, nodePrefix, "cumulative_ops"), + "Cumulative number of update commands processed. Cumulative can decrease from rollback command", + Attributes.of(OPERATION_ATTR, "adds")); deleteByIdCommandsCumulative = - new AttributedLongUpDownCounter( - baseCommandsMetric, - baseAttributes.toBuilder().put(OPERATION_ATTR, "deletes_by_id").build()); + factory.attributedLongUpDownCounter( + AttributedInstrumentFactory.standardNameProvider( + corePrefix, nodePrefix, "cumulative_ops"), + "Cumulative number of update commands processed. Cumulative can decrease from rollback command", + Attributes.of(OPERATION_ATTR, "deletes_by_id")); deleteByQueryCommandsCumulative = - new AttributedLongUpDownCounter( - baseCommandsMetric, - baseAttributes.toBuilder().put(OPERATION_ATTR, "deletes_by_query").build()); - - var baseCommitOpsMetric = - solrMetricsContext.longCounter( - "solr_core_update_commit_ops", "Total number of commit operations"); + factory.attributedLongUpDownCounter( + AttributedInstrumentFactory.standardNameProvider( + corePrefix, nodePrefix, "cumulative_ops"), + "Cumulative number of update commands processed. Cumulative can decrease from rollback command", + Attributes.of(OPERATION_ATTR, "deletes_by_query")); commitCommands = - new AttributedLongCounter( - baseCommitOpsMetric, baseAttributes.toBuilder().put(OPERATION_ATTR, "commits").build()); + factory.attributedLongCounter( + AttributedInstrumentFactory.standardNameProvider(corePrefix, nodePrefix, "commit_ops"), + "Total number of commit operations", + Attributes.of(OPERATION_ATTR, "commits")); optimizeCommands = - new AttributedLongCounter( - baseCommitOpsMetric, - baseAttributes.toBuilder().put(OPERATION_ATTR, "optimize").build()); + factory.attributedLongCounter( + AttributedInstrumentFactory.standardNameProvider(corePrefix, nodePrefix, "commit_ops"), + "Total number of commit operations", + Attributes.of(OPERATION_ATTR, "optimize")); mergeIndexesCommands = - new AttributedLongCounter( - baseCommitOpsMetric, - baseAttributes.toBuilder().put(OPERATION_ATTR, "merge_indexes").build()); + factory.attributedLongCounter( + AttributedInstrumentFactory.standardNameProvider(corePrefix, nodePrefix, "commit_ops"), + "Total number of commit operations", + Attributes.of(OPERATION_ATTR, "merge_indexes")); expungeDeleteCommands = - new AttributedLongCounter( - baseCommitOpsMetric, - baseAttributes.toBuilder().put(OPERATION_ATTR, "expunge_deletes").build()); - - var baseMaintenanceMetric = - solrMetricsContext.longCounter( - "solr_core_update_maintenance_ops", "Total number of maintenance operations"); + factory.attributedLongCounter( + AttributedInstrumentFactory.standardNameProvider(corePrefix, nodePrefix, "commit_ops"), + "Total number of commit operations", + Attributes.of(OPERATION_ATTR, "expunge_deletes")); rollbackCommands = - new AttributedLongCounter( - baseMaintenanceMetric, - baseAttributes.toBuilder().put(OPERATION_ATTR, "rollback").build()); + factory.attributedLongCounter( + AttributedInstrumentFactory.standardNameProvider( + corePrefix, nodePrefix, "maintenance_ops"), + "Total number of maintenance operations", + Attributes.of(OPERATION_ATTR, "rollback")); splitCommands = - new AttributedLongCounter( - baseMaintenanceMetric, baseAttributes.toBuilder().put(OPERATION_ATTR, "split").build()); - - var baseErrorsMetric = - solrMetricsContext.longCounter("solr_core_update_errors", "Total number of update errors"); + factory.attributedLongCounter( + AttributedInstrumentFactory.standardNameProvider( + corePrefix, nodePrefix, "maintenance_ops"), + "Total number of maintenance operations", + Attributes.of(OPERATION_ATTR, "split")); numErrorsCumulative = - new AttributedLongCounter(baseErrorsMetric, baseAttributes.toBuilder().build()); + factory.attributedLongCounter( + AttributedInstrumentFactory.standardNameProvider(corePrefix, nodePrefix, "errors"), + "Total number of update errors", + Attributes.empty()); + + submittedAdds = + factory.attributedLongCounter( + AttributedInstrumentFactory.standardNameProvider( + corePrefix, nodePrefix, "submitted_ops"), + "Total number of submitted update operations", + Attributes.of(OPERATION_ATTR, "adds")); + + submittedDeleteById = + factory.attributedLongCounter( + AttributedInstrumentFactory.standardNameProvider( + corePrefix, nodePrefix, "submitted_ops"), + "Total number of submitted update operations", + Attributes.of(OPERATION_ATTR, "deletes_by_id")); + + submittedDeleteByQuery = + factory.attributedLongCounter( + AttributedInstrumentFactory.standardNameProvider( + corePrefix, nodePrefix, "submitted_ops"), + "Total number of submitted update operations", + Attributes.of(OPERATION_ATTR, "deletes_by_query")); + + committedAdds = + factory.attributedLongCounter( + AttributedInstrumentFactory.standardNameProvider( + corePrefix, nodePrefix, "committed_ops"), + "Total number of committed update operations", + Attributes.of(OPERATION_ATTR, "adds")); + + committedDeleteById = + factory.attributedLongCounter( + AttributedInstrumentFactory.standardNameProvider( + corePrefix, nodePrefix, "committed_ops"), + "Total number of committed update operations", + Attributes.of(OPERATION_ATTR, "deletes_by_id")); + committedDeleteByQuery = + factory.attributedLongCounter( + AttributedInstrumentFactory.standardNameProvider( + corePrefix, nodePrefix, "committed_ops"), + "Total number of committed update operations", + Attributes.of(OPERATION_ATTR, "deletes_by_query")); + + // Create observable metrics only for core registry observables.add( solrMetricsContext.observableLongCounter( "solr_core_update_auto_commits", @@ -361,38 +412,6 @@ public void initializeMetrics( })); this.toClose = Collections.unmodifiableList(observables); - - var baseSubmittedOpsMetric = - solrMetricsContext.longCounter( - "solr_core_update_submitted_ops", "Total number of submitted update operations"); - - var baseCommittedOpsMetric = - solrMetricsContext.longCounter( - "solr_core_update_committed_ops", "Total number of committed update operations"); - - submittedAdds = - new AttributedLongCounter( - baseSubmittedOpsMetric, baseAttributes.toBuilder().put(OPERATION_ATTR, "adds").build()); - submittedDeleteById = - new AttributedLongCounter( - baseSubmittedOpsMetric, - baseAttributes.toBuilder().put(OPERATION_ATTR, "deletes_by_id").build()); - submittedDeleteByQuery = - new AttributedLongCounter( - baseSubmittedOpsMetric, - baseAttributes.toBuilder().put(OPERATION_ATTR, "deletes_by_query").build()); - - committedAdds = - new AttributedLongCounter( - baseCommittedOpsMetric, baseAttributes.toBuilder().put(OPERATION_ATTR, "adds").build()); - committedDeleteById = - new AttributedLongCounter( - baseCommittedOpsMetric, - baseAttributes.toBuilder().put(OPERATION_ATTR, "deletes_by_id").build()); - committedDeleteByQuery = - new AttributedLongCounter( - baseCommittedOpsMetric, - baseAttributes.toBuilder().put(OPERATION_ATTR, "deletes_by_query").build()); } private void deleteAll() throws IOException { diff --git a/solr/core/src/test-files/solr/configsets/cloud-aggregate-node-metrics/conf/solrconfig.xml b/solr/core/src/test-files/solr/configsets/cloud-aggregate-node-metrics/conf/solrconfig.xml index f23456d060a..2f605d1aaa8 100644 --- a/solr/core/src/test-files/solr/configsets/cloud-aggregate-node-metrics/conf/solrconfig.xml +++ b/solr/core/src/test-files/solr/configsets/cloud-aggregate-node-metrics/conf/solrconfig.xml @@ -29,11 +29,12 @@ ${tests.luceneMatchVersion:LATEST} - + ${solr.commitwithin.softcommit:true} + true @@ -52,4 +53,4 @@ - \ No newline at end of file + 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 951c7f69385..2320b526b08 100644 --- a/solr/core/src/test/org/apache/solr/handler/RequestHandlerBaseTest.java +++ b/solr/core/src/test/org/apache/solr/handler/RequestHandlerBaseTest.java @@ -17,6 +17,7 @@ package org.apache.solr.handler; +import static org.apache.solr.metrics.SolrMetricProducer.HANDLER_ATTR; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.ArgumentMatchers.eq; @@ -196,8 +197,6 @@ private RequestHandlerBase.HandlerMetrics createHandlerMetrics() { when(metricsContext.longHistogram(any(), any())).thenReturn(mockLongHistogram); return new RequestHandlerBase.HandlerMetrics( - metricsContext, - Attributes.of(AttributeKey.stringKey("/handler"), "/someBaseMetricPath"), - false); + metricsContext, Attributes.of(HANDLER_ATTR, "/someBaseMetricPath"), false); } } 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 0c6412e322a..e1f480eaa13 100644 --- a/solr/core/src/test/org/apache/solr/handler/RequestHandlerMetricsTest.java +++ b/solr/core/src/test/org/apache/solr/handler/RequestHandlerMetricsTest.java @@ -89,11 +89,29 @@ public void testAggregateNodeLevelMetrics() throws SolrServerException, IOExcept SolrMetricTestUtils.newCloudSelectRequestsDatapoint(core2); CounterSnapshot.CounterDataPointSnapshot actualCore2Updates = SolrMetricTestUtils.newCloudUpdateRequestsDatapoint(core2); + CounterSnapshot.CounterDataPointSnapshot actualCore1SubmittedOps = + SolrMetricTestUtils.getCounterDatapoint( + core1, + "solr_core_update_submitted_ops", + SolrMetricTestUtils.newCloudLabelsBuilder(core1) + .label("category", "UPDATE") + .label("ops", "adds") + .build()); + CounterSnapshot.CounterDataPointSnapshot actualCore2SubmittedOps = + SolrMetricTestUtils.getCounterDatapoint( + core2, + "solr_core_update_submitted_ops", + SolrMetricTestUtils.newCloudLabelsBuilder(core2) + .label("category", "UPDATE") + .label("ops", "adds") + .build()); assertEquals(1.0, actualCore1Selects.getValue(), 0.0); assertEquals(1.0, actualCore1Updates.getValue(), 0.0); assertEquals(1.0, actualCore2Updates.getValue(), 0.0); assertEquals(1.0, actualCore2Selects.getValue(), 0.0); + assertEquals(1.0, actualCore1SubmittedOps.getValue(), 0.0); + assertEquals(1.0, actualCore2SubmittedOps.getValue(), 0.0); // Get node metrics and the select/update requests should be the sum of both cores requests var nodeReader = SolrMetricTestUtils.getPrometheusMetricReader(coreContainer, "solr.node"); @@ -118,11 +136,23 @@ public void testAggregateNodeLevelMetrics() throws SolrServerException, IOExcept .label("handler", "/update") .label("otel_scope_name", "org.apache.solr") .build()); + CounterSnapshot.CounterDataPointSnapshot nodeSubmittedOps = + (CounterSnapshot.CounterDataPointSnapshot) + SolrMetricTestUtils.getDataPointSnapshot( + nodeReader, + "solr_node_update_submitted_ops", + Labels.builder() + .label("category", "UPDATE") + .label("ops", "adds") + .label("otel_scope_name", "org.apache.solr") + .build()); assertNotNull("Node select requests should be recorded", nodeSelectRequests); assertNotNull("Node update requests should be recorded", nodeUpdateRequests); + assertNotNull("Node submitted update operations should be recorded", nodeUpdateRequests); assertEquals(2.0, nodeSelectRequests.getValue(), 0.0); assertEquals(2.0, nodeUpdateRequests.getValue(), 0.0); + assertEquals(2.0, nodeSubmittedOps.getValue(), 0.0); core1.close(); core2.close(); From 8aa8d2ab333959b7bbc10d4340c507a11b66e8ab Mon Sep 17 00:00:00 2001 From: Matthew Biscocho Date: Thu, 9 Oct 2025 15:31:00 -0400 Subject: [PATCH 5/8] Missing underscore --- .../src/java/org/apache/solr/update/DirectUpdateHandler2.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java b/solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java index e53b0c617b4..9f3883d81e3 100644 --- a/solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java +++ b/solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java @@ -245,7 +245,7 @@ private void createMetrics(Attributes baseAttributes, boolean aggregateToNodeReg new AttributedInstrumentFactory( solrMetricsContext, baseAttributes, aggregateToNodeRegistry); - String corePrefix = "solr_core_update"; + String corePrefix = "solr_core_update_"; String nodePrefix = "solr_node_update_"; addCommandsCumulative = From 22dede56b215f356154f6684de5b01e8d3e63c84 Mon Sep 17 00:00:00 2001 From: Matthew Biscocho Date: Thu, 9 Oct 2025 15:33:19 -0400 Subject: [PATCH 6/8] Remove aggregateNodeLevelMetricsEnabled --- .../configsets/cloud-aggregate-node-metrics/conf/solrconfig.xml | 1 - 1 file changed, 1 deletion(-) diff --git a/solr/core/src/test-files/solr/configsets/cloud-aggregate-node-metrics/conf/solrconfig.xml b/solr/core/src/test-files/solr/configsets/cloud-aggregate-node-metrics/conf/solrconfig.xml index 2f605d1aaa8..2b1586ae253 100644 --- a/solr/core/src/test-files/solr/configsets/cloud-aggregate-node-metrics/conf/solrconfig.xml +++ b/solr/core/src/test-files/solr/configsets/cloud-aggregate-node-metrics/conf/solrconfig.xml @@ -34,7 +34,6 @@ ${solr.commitwithin.softcommit:true} - true From 06fca6dc04c3167d621029ccade0a505c2117181 Mon Sep 17 00:00:00 2001 From: Matthew Biscocho Date: Fri, 10 Oct 2025 12:04:58 -0400 Subject: [PATCH 7/8] PR feedback changes --- .../solr/handler/RequestHandlerBase.java | 21 +-- .../AttributedInstrumentFactory.java | 108 +++++------- .../solr/update/DirectUpdateHandler2.java | 48 ++---- .../handler/RequestHandlerMetricsTest.java | 155 +++++++++--------- 4 files changed, 140 insertions(+), 192 deletions(-) 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 54b836c9a3e..f2407a4273e 100644 --- a/solr/core/src/java/org/apache/solr/handler/RequestHandlerBase.java +++ b/solr/core/src/java/org/apache/solr/handler/RequestHandlerBase.java @@ -197,40 +197,29 @@ public HandlerMetrics( new AttributedInstrumentFactory( solrMetricsContext, coreAttributes, aggregateNodeLevelMetricsEnabled); - String corePrefix = "solr_core_"; - String nodePrefix = "solr_node_"; - requests = factory.attributedLongCounter( - AttributedInstrumentFactory.standardNameProvider(corePrefix, nodePrefix, "requests"), - "HTTP Solr requests", - Attributes.empty()); + "solr_core_requests", "HTTP Solr requests", Attributes.empty()); numServerErrors = factory.attributedLongCounter( - AttributedInstrumentFactory.standardNameProvider( - corePrefix, nodePrefix, "requests_errors"), + "solr_core_requests_errors", "HTTP Solr request errors", Attributes.of(SOURCE_ATTR, "server")); numClientErrors = factory.attributedLongCounter( - AttributedInstrumentFactory.standardNameProvider( - corePrefix, nodePrefix, "requests_errors"), + "solr_core_requests_errors", "HTTP Solr request errors", Attributes.of(SOURCE_ATTR, "client")); numTimeouts = factory.attributedLongCounter( - AttributedInstrumentFactory.standardNameProvider( - corePrefix, nodePrefix, "requests_timeout"), - "HTTP Solr request timeouts", - Attributes.empty()); + "solr_core_requests_timeout", "HTTP Solr request timeouts", Attributes.empty()); requestTimes = factory.attributedLongTimer( - AttributedInstrumentFactory.standardNameProvider( - "solr_core_", nodePrefix, "requests_times"), + "solr_core_requests_times", "HTTP Solr request times", OtelUnit.MILLISECONDS, Attributes.empty()); diff --git a/solr/core/src/java/org/apache/solr/metrics/otel/instruments/AttributedInstrumentFactory.java b/solr/core/src/java/org/apache/solr/metrics/otel/instruments/AttributedInstrumentFactory.java index 29deeb4a1ae..c73edadb6e4 100644 --- a/solr/core/src/java/org/apache/solr/metrics/otel/instruments/AttributedInstrumentFactory.java +++ b/solr/core/src/java/org/apache/solr/metrics/otel/instruments/AttributedInstrumentFactory.java @@ -16,13 +16,18 @@ */ package org.apache.solr.metrics.otel.instruments; -import static org.apache.solr.metrics.SolrMetricProducer.CATEGORY_ATTR; -import static org.apache.solr.metrics.SolrMetricProducer.HANDLER_ATTR; +import static org.apache.solr.handler.component.SearchHandler.INTERNAL_ATTR; +import static org.apache.solr.metrics.SolrCoreMetricManager.COLLECTION_ATTR; +import static org.apache.solr.metrics.SolrCoreMetricManager.CORE_ATTR; +import static org.apache.solr.metrics.SolrCoreMetricManager.REPLICA_TYPE_ATTR; +import static org.apache.solr.metrics.SolrCoreMetricManager.SHARD_ATTR; +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 io.opentelemetry.api.metrics.LongUpDownCounter; +import java.util.Set; import org.apache.solr.core.SolrInfoBean; import org.apache.solr.metrics.SolrMetricManager; import org.apache.solr.metrics.SolrMetricsContext; @@ -34,6 +39,8 @@ */ public class AttributedInstrumentFactory { + private static final Set> FILTER_ATTRS_SET = + Set.of(COLLECTION_ATTR, CORE_ATTR, SHARD_ATTR, REPLICA_TYPE_ATTR, INTERNAL_ATTR); private final SolrMetricsContext primaryMetricsContext; private final Attributes primaryAttributes; private final boolean aggregateToNodeRegistry; @@ -68,91 +75,81 @@ public AttributedInstrumentFactory( } public AttributedLongCounter attributedLongCounter( - MetricNameProvider metricNameProvider, String description, Attributes additionalAttributes) { + String metricName, String description, Attributes additionalAttributes) { Attributes finalPrimaryAttrs = appendAttributes(primaryAttributes, additionalAttributes); if (aggregateToNodeRegistry && nodeMetricsContext != null) { Attributes finalNodeAttrs = appendAttributes(nodeAttributes, additionalAttributes); - LongCounter primaryCounter = - primaryMetricsContext.longCounter(metricNameProvider.getPrimaryMetricName(), description); + LongCounter primaryCounter = primaryMetricsContext.longCounter(metricName, description); LongCounter nodeCounter = - nodeMetricsContext.longCounter(metricNameProvider.getNodeMetricName(), description); + nodeMetricsContext.longCounter(toNodeMetricName(metricName), description); return new DualRegistryAttributedLongCounter( primaryCounter, finalPrimaryAttrs, nodeCounter, finalNodeAttrs); } else { - String metricName = - primaryIsNodeRegistry - ? metricNameProvider.getNodeMetricName() - : metricNameProvider.getPrimaryMetricName(); - - LongCounter counter = primaryMetricsContext.longCounter(metricName, description); + String finalMetricName = primaryIsNodeRegistry ? toNodeMetricName(metricName) : metricName; + LongCounter counter = primaryMetricsContext.longCounter(finalMetricName, description); return new AttributedLongCounter(counter, finalPrimaryAttrs); } } public AttributedLongUpDownCounter attributedLongUpDownCounter( - MetricNameProvider metricNameProvider, String description, Attributes additionalAttributes) { + String metricName, String description, Attributes additionalAttributes) { Attributes finalPrimaryAttrs = appendAttributes(primaryAttributes, additionalAttributes); if (aggregateToNodeRegistry && nodeMetricsContext != null) { Attributes finalNodeAttrs = appendAttributes(nodeAttributes, additionalAttributes); LongUpDownCounter primaryCounter = - primaryMetricsContext.longUpDownCounter( - metricNameProvider.getPrimaryMetricName(), description); + primaryMetricsContext.longUpDownCounter(metricName, description); LongUpDownCounter nodeCounter = - nodeMetricsContext.longUpDownCounter(metricNameProvider.getNodeMetricName(), description); - + nodeMetricsContext.longUpDownCounter(toNodeMetricName(metricName), description); return new DualRegistryAttributedLongUpDownCounter( primaryCounter, finalPrimaryAttrs, nodeCounter, finalNodeAttrs); } else { - String metricName = - primaryIsNodeRegistry - ? metricNameProvider.getNodeMetricName() - : metricNameProvider.getPrimaryMetricName(); - - LongUpDownCounter counter = primaryMetricsContext.longUpDownCounter(metricName, description); + String finalMetricName = primaryIsNodeRegistry ? toNodeMetricName(metricName) : metricName; + LongUpDownCounter counter = + primaryMetricsContext.longUpDownCounter(finalMetricName, description); return new AttributedLongUpDownCounter(counter, finalPrimaryAttrs); } } public AttributedLongTimer attributedLongTimer( - MetricNameProvider metricNameProvider, - String description, - OtelUnit unit, - Attributes additionalAttributes) { + String metricName, String description, OtelUnit unit, Attributes additionalAttributes) { Attributes finalPrimaryAttrs = appendAttributes(primaryAttributes, additionalAttributes); - if (aggregateToNodeRegistry) { + if (aggregateToNodeRegistry && nodeMetricsContext != null) { Attributes finalNodeAttrs = appendAttributes(nodeAttributes, additionalAttributes); - LongHistogram coreHistogram = - primaryMetricsContext.longHistogram( - metricNameProvider.getPrimaryMetricName(), description, unit); + LongHistogram primaryHistogram = + primaryMetricsContext.longHistogram(metricName, description, unit); LongHistogram nodeHistogram = - nodeMetricsContext.longHistogram( - metricNameProvider.getNodeMetricName(), description, unit); + nodeMetricsContext.longHistogram(toNodeMetricName(metricName), description, unit); return new DualRegistryAttributedLongTimer( - coreHistogram, finalPrimaryAttrs, nodeHistogram, finalNodeAttrs); + primaryHistogram, finalPrimaryAttrs, nodeHistogram, finalNodeAttrs); } else { - String metricName = - primaryIsNodeRegistry - ? metricNameProvider.getNodeMetricName() - : metricNameProvider.getPrimaryMetricName(); - - LongHistogram histogram = primaryMetricsContext.longHistogram(metricName, description, unit); + String finalMetricName = primaryIsNodeRegistry ? toNodeMetricName(metricName) : metricName; + LongHistogram histogram = + primaryMetricsContext.longHistogram(finalMetricName, description, unit); return new AttributedLongTimer(histogram, finalPrimaryAttrs); } } - // Filter out core attributes and keep only category and handler if they exist + // Replace core metric name prefix to node prefix + private String toNodeMetricName(String coreMetricName) { + return coreMetricName.replace("solr_core", "solr_node"); + } + + // Filter out core attributes and keep all others for node-level metrics + @SuppressWarnings("unchecked") private Attributes createNodeAttributes(Attributes coreAttributes) { var builder = Attributes.builder(); - if (coreAttributes.get(CATEGORY_ATTR) != null) - builder.put(CATEGORY_ATTR, coreAttributes.get(CATEGORY_ATTR)); - if (coreAttributes.get(HANDLER_ATTR) != null) - builder.put(HANDLER_ATTR, coreAttributes.get(HANDLER_ATTR)); + coreAttributes.forEach( + (key, value) -> { + if (!FILTER_ATTRS_SET.contains(key)) { + builder.put((AttributeKey) key, value); + } + }); return builder.build(); } @@ -160,25 +157,4 @@ private Attributes createNodeAttributes(Attributes coreAttributes) { private Attributes appendAttributes(Attributes base, Attributes additional) { return base.toBuilder().putAll(additional).build(); } - - public interface MetricNameProvider { - String getPrimaryMetricName(); - - String getNodeMetricName(); - } - - public static MetricNameProvider standardNameProvider( - String corePrefix, String nodePrefix, String metricSuffix) { - return new MetricNameProvider() { - @Override - public String getPrimaryMetricName() { - return corePrefix + metricSuffix; - } - - @Override - public String getNodeMetricName() { - return nodePrefix + metricSuffix; - } - }; - } } diff --git a/solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java b/solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java index 9f3883d81e3..dc802afc2b7 100644 --- a/solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java +++ b/solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java @@ -245,113 +245,97 @@ private void createMetrics(Attributes baseAttributes, boolean aggregateToNodeReg new AttributedInstrumentFactory( solrMetricsContext, baseAttributes, aggregateToNodeRegistry); - String corePrefix = "solr_core_update_"; - String nodePrefix = "solr_node_update_"; - addCommandsCumulative = factory.attributedLongUpDownCounter( - AttributedInstrumentFactory.standardNameProvider( - corePrefix, nodePrefix, "cumulative_ops"), + "solr_core_update_cumulative_ops", "Cumulative number of update commands processed. Cumulative can decrease from rollback command", Attributes.of(OPERATION_ATTR, "adds")); deleteByIdCommandsCumulative = factory.attributedLongUpDownCounter( - AttributedInstrumentFactory.standardNameProvider( - corePrefix, nodePrefix, "cumulative_ops"), + "solr_core_update_cumulative_ops", "Cumulative number of update commands processed. Cumulative can decrease from rollback command", Attributes.of(OPERATION_ATTR, "deletes_by_id")); deleteByQueryCommandsCumulative = factory.attributedLongUpDownCounter( - AttributedInstrumentFactory.standardNameProvider( - corePrefix, nodePrefix, "cumulative_ops"), + "solr_core_update_cumulative_ops", "Cumulative number of update commands processed. Cumulative can decrease from rollback command", Attributes.of(OPERATION_ATTR, "deletes_by_query")); commitCommands = factory.attributedLongCounter( - AttributedInstrumentFactory.standardNameProvider(corePrefix, nodePrefix, "commit_ops"), + "solr_core_update_commit_ops", "Total number of commit operations", Attributes.of(OPERATION_ATTR, "commits")); optimizeCommands = factory.attributedLongCounter( - AttributedInstrumentFactory.standardNameProvider(corePrefix, nodePrefix, "commit_ops"), + "solr_core_update_commit_ops", "Total number of commit operations", Attributes.of(OPERATION_ATTR, "optimize")); mergeIndexesCommands = factory.attributedLongCounter( - AttributedInstrumentFactory.standardNameProvider(corePrefix, nodePrefix, "commit_ops"), + "solr_core_update_commit_ops", "Total number of commit operations", Attributes.of(OPERATION_ATTR, "merge_indexes")); expungeDeleteCommands = factory.attributedLongCounter( - AttributedInstrumentFactory.standardNameProvider(corePrefix, nodePrefix, "commit_ops"), + "solr_core_update_commit_ops", "Total number of commit operations", Attributes.of(OPERATION_ATTR, "expunge_deletes")); rollbackCommands = factory.attributedLongCounter( - AttributedInstrumentFactory.standardNameProvider( - corePrefix, nodePrefix, "maintenance_ops"), + "solr_core_update_maintenance_ops", "Total number of maintenance operations", Attributes.of(OPERATION_ATTR, "rollback")); splitCommands = factory.attributedLongCounter( - AttributedInstrumentFactory.standardNameProvider( - corePrefix, nodePrefix, "maintenance_ops"), + "solr_core_update_maintenance_ops", "Total number of maintenance operations", Attributes.of(OPERATION_ATTR, "split")); numErrorsCumulative = factory.attributedLongCounter( - AttributedInstrumentFactory.standardNameProvider(corePrefix, nodePrefix, "errors"), - "Total number of update errors", - Attributes.empty()); + "solr_core_update_errors", "Total number of update errors", Attributes.empty()); submittedAdds = factory.attributedLongCounter( - AttributedInstrumentFactory.standardNameProvider( - corePrefix, nodePrefix, "submitted_ops"), + "solr_core_update_submitted_ops", "Total number of submitted update operations", Attributes.of(OPERATION_ATTR, "adds")); submittedDeleteById = factory.attributedLongCounter( - AttributedInstrumentFactory.standardNameProvider( - corePrefix, nodePrefix, "submitted_ops"), + "solr_core_update_submitted_ops", "Total number of submitted update operations", Attributes.of(OPERATION_ATTR, "deletes_by_id")); submittedDeleteByQuery = factory.attributedLongCounter( - AttributedInstrumentFactory.standardNameProvider( - corePrefix, nodePrefix, "submitted_ops"), + "solr_core_update_submitted_ops", "Total number of submitted update operations", Attributes.of(OPERATION_ATTR, "deletes_by_query")); committedAdds = factory.attributedLongCounter( - AttributedInstrumentFactory.standardNameProvider( - corePrefix, nodePrefix, "committed_ops"), + "solr_core_update_committed_ops", "Total number of committed update operations", Attributes.of(OPERATION_ATTR, "adds")); committedDeleteById = factory.attributedLongCounter( - AttributedInstrumentFactory.standardNameProvider( - corePrefix, nodePrefix, "committed_ops"), + "solr_core_update_committed_ops", "Total number of committed update operations", Attributes.of(OPERATION_ATTR, "deletes_by_id")); committedDeleteByQuery = factory.attributedLongCounter( - AttributedInstrumentFactory.standardNameProvider( - corePrefix, nodePrefix, "committed_ops"), + "solr_core_update_committed_ops", "Total number of committed update operations", Attributes.of(OPERATION_ATTR, "deletes_by_query")); 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 e1f480eaa13..e338939b30c 100644 --- a/solr/core/src/test/org/apache/solr/handler/RequestHandlerMetricsTest.java +++ b/solr/core/src/test/org/apache/solr/handler/RequestHandlerMetricsTest.java @@ -78,83 +78,82 @@ public void testAggregateNodeLevelMetrics() throws SolrServerException, IOExcept cloudClient.query(collection2, solrQuery); var coreContainer = cluster.getJettySolrRunners().get(0).getCoreContainer(); - SolrCore core1 = coreContainer.getCore(coreContainer.getAllCoreNames().get(0)); - SolrCore core2 = coreContainer.getCore(coreContainer.getAllCoreNames().get(1)); - - CounterSnapshot.CounterDataPointSnapshot actualCore1Selects = - SolrMetricTestUtils.newCloudSelectRequestsDatapoint(core1); - CounterSnapshot.CounterDataPointSnapshot actualCore1Updates = - SolrMetricTestUtils.newCloudUpdateRequestsDatapoint(core1); - CounterSnapshot.CounterDataPointSnapshot actualCore2Selects = - SolrMetricTestUtils.newCloudSelectRequestsDatapoint(core2); - CounterSnapshot.CounterDataPointSnapshot actualCore2Updates = - SolrMetricTestUtils.newCloudUpdateRequestsDatapoint(core2); - CounterSnapshot.CounterDataPointSnapshot actualCore1SubmittedOps = - SolrMetricTestUtils.getCounterDatapoint( - core1, - "solr_core_update_submitted_ops", - SolrMetricTestUtils.newCloudLabelsBuilder(core1) - .label("category", "UPDATE") - .label("ops", "adds") - .build()); - CounterSnapshot.CounterDataPointSnapshot actualCore2SubmittedOps = - SolrMetricTestUtils.getCounterDatapoint( - core2, - "solr_core_update_submitted_ops", - SolrMetricTestUtils.newCloudLabelsBuilder(core2) - .label("category", "UPDATE") - .label("ops", "adds") - .build()); - - assertEquals(1.0, actualCore1Selects.getValue(), 0.0); - assertEquals(1.0, actualCore1Updates.getValue(), 0.0); - assertEquals(1.0, actualCore2Updates.getValue(), 0.0); - assertEquals(1.0, actualCore2Selects.getValue(), 0.0); - assertEquals(1.0, actualCore1SubmittedOps.getValue(), 0.0); - assertEquals(1.0, actualCore2SubmittedOps.getValue(), 0.0); - - // Get node metrics and the select/update requests should be the sum of both cores requests - var nodeReader = SolrMetricTestUtils.getPrometheusMetricReader(coreContainer, "solr.node"); - - CounterSnapshot.CounterDataPointSnapshot nodeSelectRequests = - (CounterSnapshot.CounterDataPointSnapshot) - SolrMetricTestUtils.getDataPointSnapshot( - nodeReader, - "solr_node_requests", - Labels.builder() - .label("category", "QUERY") - .label("handler", "/select") - .label("otel_scope_name", "org.apache.solr") - .build()); - CounterSnapshot.CounterDataPointSnapshot nodeUpdateRequests = - (CounterSnapshot.CounterDataPointSnapshot) - SolrMetricTestUtils.getDataPointSnapshot( - nodeReader, - "solr_node_requests", - Labels.builder() - .label("category", "UPDATE") - .label("handler", "/update") - .label("otel_scope_name", "org.apache.solr") - .build()); - CounterSnapshot.CounterDataPointSnapshot nodeSubmittedOps = - (CounterSnapshot.CounterDataPointSnapshot) - SolrMetricTestUtils.getDataPointSnapshot( - nodeReader, - "solr_node_update_submitted_ops", - Labels.builder() - .label("category", "UPDATE") - .label("ops", "adds") - .label("otel_scope_name", "org.apache.solr") - .build()); - - assertNotNull("Node select requests should be recorded", nodeSelectRequests); - assertNotNull("Node update requests should be recorded", nodeUpdateRequests); - assertNotNull("Node submitted update operations should be recorded", nodeUpdateRequests); - assertEquals(2.0, nodeSelectRequests.getValue(), 0.0); - assertEquals(2.0, nodeUpdateRequests.getValue(), 0.0); - assertEquals(2.0, nodeSubmittedOps.getValue(), 0.0); - - core1.close(); - core2.close(); + + try (SolrCore core1 = coreContainer.getCore(coreContainer.getAllCoreNames().get(0)); + SolrCore core2 = coreContainer.getCore(coreContainer.getAllCoreNames().get(1))) { + + CounterSnapshot.CounterDataPointSnapshot actualCore1Selects = + SolrMetricTestUtils.newCloudSelectRequestsDatapoint(core1); + CounterSnapshot.CounterDataPointSnapshot actualCore1Updates = + SolrMetricTestUtils.newCloudUpdateRequestsDatapoint(core1); + CounterSnapshot.CounterDataPointSnapshot actualCore2Selects = + SolrMetricTestUtils.newCloudSelectRequestsDatapoint(core2); + CounterSnapshot.CounterDataPointSnapshot actualCore2Updates = + SolrMetricTestUtils.newCloudUpdateRequestsDatapoint(core2); + CounterSnapshot.CounterDataPointSnapshot actualCore1SubmittedOps = + SolrMetricTestUtils.getCounterDatapoint( + core1, + "solr_core_update_submitted_ops", + SolrMetricTestUtils.newCloudLabelsBuilder(core1) + .label("category", "UPDATE") + .label("ops", "adds") + .build()); + CounterSnapshot.CounterDataPointSnapshot actualCore2SubmittedOps = + SolrMetricTestUtils.getCounterDatapoint( + core2, + "solr_core_update_submitted_ops", + SolrMetricTestUtils.newCloudLabelsBuilder(core2) + .label("category", "UPDATE") + .label("ops", "adds") + .build()); + + assertEquals(1.0, actualCore1Selects.getValue(), 0.0); + assertEquals(1.0, actualCore1Updates.getValue(), 0.0); + assertEquals(1.0, actualCore2Updates.getValue(), 0.0); + assertEquals(1.0, actualCore2Selects.getValue(), 0.0); + assertEquals(1.0, actualCore1SubmittedOps.getValue(), 0.0); + assertEquals(1.0, actualCore2SubmittedOps.getValue(), 0.0); + + // Get node metrics and the select/update requests should be the sum of both cores requests + var nodeReader = SolrMetricTestUtils.getPrometheusMetricReader(coreContainer, "solr.node"); + + CounterSnapshot.CounterDataPointSnapshot nodeSelectRequests = + (CounterSnapshot.CounterDataPointSnapshot) + SolrMetricTestUtils.getDataPointSnapshot( + nodeReader, + "solr_node_requests", + Labels.builder() + .label("category", "QUERY") + .label("handler", "/select") + .label("otel_scope_name", "org.apache.solr") + .build()); + CounterSnapshot.CounterDataPointSnapshot nodeUpdateRequests = + (CounterSnapshot.CounterDataPointSnapshot) + SolrMetricTestUtils.getDataPointSnapshot( + nodeReader, + "solr_node_requests", + Labels.builder() + .label("category", "UPDATE") + .label("handler", "/update") + .label("otel_scope_name", "org.apache.solr") + .build()); + CounterSnapshot.CounterDataPointSnapshot nodeSubmittedOps = + (CounterSnapshot.CounterDataPointSnapshot) + SolrMetricTestUtils.getDataPointSnapshot( + nodeReader, + "solr_node_update_submitted_ops", + Labels.builder() + .label("category", "UPDATE") + .label("ops", "adds") + .label("otel_scope_name", "org.apache.solr") + .build()); + + assertNotNull("Node select requests should be recorded", nodeSelectRequests); + assertNotNull("Node update requests should be recorded", nodeUpdateRequests); + assertNotNull("Node submitted update operations should be recorded", nodeSubmittedOps); + assertEquals(2.0, nodeSelectRequests.getValue(), 0.0); + assertEquals(2.0, nodeUpdateRequests.getValue(), 0.0); + assertEquals(2.0, nodeSubmittedOps.getValue(), 0.0); + } } } From 8c4f11f55503511e956ab3998196067596cbe196 Mon Sep 17 00:00:00 2001 From: Matthew Biscocho Date: Sat, 11 Oct 2025 22:12:10 -0400 Subject: [PATCH 8/8] Make comment javadoc --- .../metrics/otel/instruments/AttributedInstrumentFactory.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/metrics/otel/instruments/AttributedInstrumentFactory.java b/solr/core/src/java/org/apache/solr/metrics/otel/instruments/AttributedInstrumentFactory.java index c73edadb6e4..a443eab92b0 100644 --- a/solr/core/src/java/org/apache/solr/metrics/otel/instruments/AttributedInstrumentFactory.java +++ b/solr/core/src/java/org/apache/solr/metrics/otel/instruments/AttributedInstrumentFactory.java @@ -134,12 +134,12 @@ public AttributedLongTimer attributedLongTimer( } } - // Replace core metric name prefix to node prefix + /** Replace core metric name prefix to node prefix */ private String toNodeMetricName(String coreMetricName) { return coreMetricName.replace("solr_core", "solr_node"); } - // Filter out core attributes and keep all others for node-level metrics + /** Filter out core attributes and keep all others for node-level metrics */ @SuppressWarnings("unchecked") private Attributes createNodeAttributes(Attributes coreAttributes) { var builder = Attributes.builder();