This is an automated email from the ASF dual-hosted git repository. dsmiley pushed a commit to branch feature/SOLR-17458-rebased in repository https://gitbox.apache.org/repos/asf/solr.git
commit 0261c4bc1d2ad1eadbc33bf29d15dc71e4cf9729 Author: David Smiley <[email protected]> AuthorDate: Tue Oct 14 01:06:17 2025 -0400 SOLR-17458: fix precommit source matters --- .../java/org/apache/solr/core/CoreContainer.java | 2 +- .../src/java/org/apache/solr/core/SolrCores.java | 1 + .../apache/solr/metrics/SolrCoreMetricManager.java | 1 - .../instruments/AttributedInstrumentFactory.java | 8 ++------ .../solr/cloud/TestRandomRequestDistribution.java | 5 ++--- .../apache/solr/metrics/SolrMetricManagerTest.java | 2 ++ .../solr/security/AuditLoggerIntegrationTest.java | 8 ++++++-- .../apache/solr/security/CertAuthPluginTest.java | 1 + .../security/MockSolrMetricsContextFactory.java | 21 ++++++++++++++++----- .../stats/OtelInstrumentedExecutorServiceTest.java | 16 +++++++++++----- .../solr/opentelemetry/OtlpExporterFactory.java | 1 + .../solr/opentelemetry/TestMetricExemplars.java | 16 ++++++++++++++++ .../solr/client/solrj/SolrJMetricTestUtils.java | 10 +++++----- .../client/solrj/impl/CloudHttp2SolrClientTest.java | 4 ++-- .../solr/client/solrj/impl/CloudSolrClientTest.java | 4 ++-- 15 files changed, 68 insertions(+), 32 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 c50531d8715..c10aa2b5c11 100644 --- a/solr/core/src/java/org/apache/solr/core/CoreContainer.java +++ b/solr/core/src/java/org/apache/solr/core/CoreContainer.java @@ -916,7 +916,7 @@ public class CoreContainer { solrMetricsContext.observableLongGauge( "solr_disk_space", - String.format("Disk metrics for Solr's data home directory (%s)", dataHome.toString()), + "Disk metrics for Solr's data home directory (" + dataHome + ")", measurement -> { try { var fileStore = Files.getFileStore(dataHome); diff --git a/solr/core/src/java/org/apache/solr/core/SolrCores.java b/solr/core/src/java/org/apache/solr/core/SolrCores.java index 45d7a651c27..74c06e11961 100644 --- a/solr/core/src/java/org/apache/solr/core/SolrCores.java +++ b/solr/core/src/java/org/apache/solr/core/SolrCores.java @@ -92,6 +92,7 @@ public class SolrCores implements SolrInfoBean { // We are shutting down. You can't hold the lock on the various lists of cores while they shut // down, so we need to make a temporary copy of the names and shut them down outside the lock. + @Override public void close() { waitForLoadingCoresToFinish(30 * 1000); 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 ec0fc589ef9..19e40835fea 100644 --- a/solr/core/src/java/org/apache/solr/metrics/SolrCoreMetricManager.java +++ b/solr/core/src/java/org/apache/solr/metrics/SolrCoreMetricManager.java @@ -112,7 +112,6 @@ public class SolrCoreMetricManager implements Closeable { * in the case of core swapping/renaming * * @param producer producer of metrics to be registered - * @param attributes */ public void registerMetricProducer(SolrMetricProducer producer, Attributes attributes) { if (attributes == null || producer == null) { 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 0f2c05f848e..189672bcb0f 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 @@ -28,7 +28,6 @@ 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; import org.apache.solr.metrics.otel.OtelUnit; @@ -58,17 +57,14 @@ public class AttributedInstrumentFactory { // Primary could be a node this.primaryIsNodeRegistry = - primaryMetricsContext - .getRegistryName() - .equals(SolrMetricManager.NODE_REGISTRY); + primaryMetricsContext.getRegistryName().equals(SolrMetricManager.NODE_REGISTRY); // 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.NODE_REGISTRY); + primaryMetricsContext.getMetricManager(), SolrMetricManager.NODE_REGISTRY); this.nodeAttributes = createNodeAttributes(primaryAttributes); } } 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 6b6673c06f7..463bb677dc4 100644 --- a/solr/core/src/test/org/apache/solr/cloud/TestRandomRequestDistribution.java +++ b/solr/core/src/test/org/apache/solr/cloud/TestRandomRequestDistribution.java @@ -134,8 +134,7 @@ public class TestRandomRequestDistribution extends AbstractFullDistribZkTestBase assertEquals( "Sanity Check: Num Queries So Far Doesn't Match Total????", expectedTotalRequests, - actualTotalRequests, - 0.0); + (long) actualTotalRequests); } log.info("Total requests: {}", expectedTotalRequests); assertEquals( @@ -272,7 +271,7 @@ public class TestRandomRequestDistribution extends AbstractFullDistribZkTestBase double c = getSelectRequestCount(leaderCore); - assertEquals("Query wasn't served by leader", count, c, 0.0); + assertEquals("Query wasn't served by leader", count, (long) c); } } } diff --git a/solr/core/src/test/org/apache/solr/metrics/SolrMetricManagerTest.java b/solr/core/src/test/org/apache/solr/metrics/SolrMetricManagerTest.java index fa15ea4cb9a..51587663fa2 100644 --- a/solr/core/src/test/org/apache/solr/metrics/SolrMetricManagerTest.java +++ b/solr/core/src/test/org/apache/solr/metrics/SolrMetricManagerTest.java @@ -51,6 +51,7 @@ public class SolrMetricManagerTest extends SolrTestCaseJ4 { private PrometheusMetricReader reader; @Before + @Override public void setUp() throws Exception { super.setUp(); this.metricManager = new SolrMetricManager(InMemoryMetricExporter.create()); @@ -60,6 +61,7 @@ public class SolrMetricManagerTest extends SolrTestCaseJ4 { } @After + @Override public void tearDown() throws Exception { metricManager.closeAllRegistries(); super.tearDown(); diff --git a/solr/core/src/test/org/apache/solr/security/AuditLoggerIntegrationTest.java b/solr/core/src/test/org/apache/solr/security/AuditLoggerIntegrationTest.java index ad5c047775e..bf946962968 100644 --- a/solr/core/src/test/org/apache/solr/security/AuditLoggerIntegrationTest.java +++ b/solr/core/src/test/org/apache/solr/security/AuditLoggerIntegrationTest.java @@ -49,6 +49,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.List; +import java.util.Locale; import java.util.concurrent.BlockingQueue; import java.util.concurrent.LinkedBlockingDeque; import java.util.concurrent.Semaphore; @@ -585,7 +586,7 @@ public class AuditLoggerIntegrationTest extends SolrCloudAuthTestCase { (GaugeSnapshot.GaugeDataPointSnapshot) SolrMetricTestUtils.getDataPointSnapshot(metricsReader, metricName, labels); assertNotNull(dataPointSnapshot); - assertEquals(expectedValue, dataPointSnapshot.getValue(), 0.0); + assertEquals(expectedValue, (long) dataPointSnapshot.getValue()); } private void assertAuditMetricsMinimums(int count, int errors) throws InterruptedException { @@ -609,7 +610,10 @@ public class AuditLoggerIntegrationTest extends SolrCloudAuthTestCase { assertTrue( String.format( - "Counter metric '%s' did not meet minimum %d after retry", metricName, expectedMinimum), + Locale.ROOT, + "Counter metric '%s' did not meet minimum %d after retry", + metricName, + expectedMinimum), success); } diff --git a/solr/core/src/test/org/apache/solr/security/CertAuthPluginTest.java b/solr/core/src/test/org/apache/solr/security/CertAuthPluginTest.java index 42a596ba9e7..a0574c1327f 100644 --- a/solr/core/src/test/org/apache/solr/security/CertAuthPluginTest.java +++ b/solr/core/src/test/org/apache/solr/security/CertAuthPluginTest.java @@ -65,6 +65,7 @@ public class CertAuthPluginTest extends SolrTestCaseJ4 { plugin.init(Collections.emptyMap()); } + @Override @After public void tearDown() throws Exception { deleteCore(); diff --git a/solr/core/src/test/org/apache/solr/security/MockSolrMetricsContextFactory.java b/solr/core/src/test/org/apache/solr/security/MockSolrMetricsContextFactory.java index 2703b8b2737..27f7ad57965 100644 --- a/solr/core/src/test/org/apache/solr/security/MockSolrMetricsContextFactory.java +++ b/solr/core/src/test/org/apache/solr/security/MockSolrMetricsContextFactory.java @@ -1,3 +1,19 @@ +/* + * 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.security; import static org.mockito.ArgumentMatchers.any; @@ -5,7 +21,6 @@ import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; -import com.codahale.metrics.Timer; import io.opentelemetry.api.metrics.LongCounter; import io.opentelemetry.api.metrics.LongGauge; import io.opentelemetry.api.metrics.LongHistogram; @@ -23,10 +38,6 @@ public final class MockSolrMetricsContextFactory { LongCounter mockOtelLongCounter = mock(LongCounter.class); when(mockChildContext.longCounter(anyString(), any())).thenReturn(mockOtelLongCounter); - Timer mockTimer = mock(Timer.class); - Timer.Context mockTimerContext = mock(Timer.Context.class); - when(mockTimer.time()).thenReturn(mockTimerContext); - LongHistogram mockLongHistogram = mock(LongHistogram.class); when(mockChildContext.longHistogram(anyString(), anyString(), any(OtelUnit.class))) .thenReturn(mockLongHistogram); diff --git a/solr/core/src/test/org/apache/solr/util/stats/OtelInstrumentedExecutorServiceTest.java b/solr/core/src/test/org/apache/solr/util/stats/OtelInstrumentedExecutorServiceTest.java index 8a024296a75..3ae36b14db6 100644 --- a/solr/core/src/test/org/apache/solr/util/stats/OtelInstrumentedExecutorServiceTest.java +++ b/solr/core/src/test/org/apache/solr/util/stats/OtelInstrumentedExecutorServiceTest.java @@ -30,6 +30,8 @@ import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.TimeUnit; import org.apache.solr.SolrTestCase; +import org.apache.solr.common.util.ExecutorUtil; +import org.apache.solr.common.util.SolrNamedThreadFactory; import org.apache.solr.core.SolrInfoBean; import org.apache.solr.metrics.SolrMetricManager; import org.apache.solr.metrics.SolrMetricsContext; @@ -43,7 +45,7 @@ public class OtelInstrumentedExecutorServiceTest extends SolrTestCase { public static TimeUnit EXEC_TIMEOUT_UNITS = TimeUnit.SECONDS; public static final String REGISTRY_NAME = "solr-test-otel-registry"; public static final String TAG_NAME = "solr-test-otel-tag"; - public static final double DELTA = 1E-8; + public static final double DELTA = 2.4e-07; public static SolrMetricsContext metricsContext; @@ -54,7 +56,7 @@ public class OtelInstrumentedExecutorServiceTest extends SolrTestCase { @Test public void taskCount() throws InterruptedException { - try (var exec = testExecutor("taskCount", Executors.newFixedThreadPool(PARALLELISM))) { + try (var exec = testExecutor("taskCount", newFixedThreadPoolExecutor())) { final int numTasks = 225; for (int i = 0; i < numTasks; ++i) { exec.submit(() -> {}); @@ -82,7 +84,7 @@ public class OtelInstrumentedExecutorServiceTest extends SolrTestCase { @Test public void taskRandomCount() throws InterruptedException { - try (var exec = testExecutor("taskRandomCount", Executors.newFixedThreadPool(PARALLELISM))) { + try (var exec = testExecutor("taskRandomCount", newFixedThreadPoolExecutor())) { final int numTasks = randomIntBetween(1, 500); for (int i = 0; i < numTasks; ++i) { exec.submit(() -> {}); @@ -110,7 +112,7 @@ public class OtelInstrumentedExecutorServiceTest extends SolrTestCase { @Test public void taskTimers() throws InterruptedException { - try (var exec = testExecutor("taskTimers", Executors.newFixedThreadPool(PARALLELISM))) { + try (var exec = testExecutor("taskTimers", newFixedThreadPoolExecutor())) { final long durationMs = 200; final double durationDeltaMs = 10.0; exec.submit( @@ -150,7 +152,7 @@ public class OtelInstrumentedExecutorServiceTest extends SolrTestCase { @Test public void threadPoolTasks() throws InterruptedException { - try (var exec = testExecutor("threadPoolTasks", Executors.newFixedThreadPool(PARALLELISM))) { + try (var exec = testExecutor("threadPoolTasks", newFixedThreadPoolExecutor())) { final int numTasks = 225; for (int i = 0; i < numTasks; ++i) { exec.submit(() -> {}); @@ -186,6 +188,10 @@ public class OtelInstrumentedExecutorServiceTest extends SolrTestCase { } } + private static ExecutorService newFixedThreadPoolExecutor() { + return ExecutorUtil.newMDCAwareFixedThreadPool(PARALLELISM, new SolrNamedThreadFactory("test")); + } + @Test public void forkJoinPoolTasks() throws InterruptedException { try (var exec = testExecutor("forkJoinPoolTasks", Executors.newWorkStealingPool(PARALLELISM))) { diff --git a/solr/modules/opentelemetry/src/java/org/apache/solr/opentelemetry/OtlpExporterFactory.java b/solr/modules/opentelemetry/src/java/org/apache/solr/opentelemetry/OtlpExporterFactory.java index 8237596be42..329ff764c84 100644 --- a/solr/modules/opentelemetry/src/java/org/apache/solr/opentelemetry/OtlpExporterFactory.java +++ b/solr/modules/opentelemetry/src/java/org/apache/solr/opentelemetry/OtlpExporterFactory.java @@ -34,6 +34,7 @@ public class OtlpExporterFactory implements MetricExporterFactory { private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); + @Override public MetricExporter getExporter() { if (!OTLP_EXPORTER_ENABLED) { log.info("OTLP metric exporter is disabled."); diff --git a/solr/modules/opentelemetry/src/test/org/apache/solr/opentelemetry/TestMetricExemplars.java b/solr/modules/opentelemetry/src/test/org/apache/solr/opentelemetry/TestMetricExemplars.java index e0660f8559a..f564a452ebf 100644 --- a/solr/modules/opentelemetry/src/test/org/apache/solr/opentelemetry/TestMetricExemplars.java +++ b/solr/modules/opentelemetry/src/test/org/apache/solr/opentelemetry/TestMetricExemplars.java @@ -1,3 +1,19 @@ +/* + * 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.opentelemetry; import static org.apache.solr.opentelemetry.TestDistributedTracing.getAndClearSpans; diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/SolrJMetricTestUtils.java b/solr/solrj/src/test/org/apache/solr/client/solrj/SolrJMetricTestUtils.java index d48f79cc55a..f6f2d135920 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/SolrJMetricTestUtils.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/SolrJMetricTestUtils.java @@ -74,9 +74,9 @@ public final class SolrJMetricTestUtils { .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))) + && l.contains("category=\"" + category + "\"") + && l.contains("collection=\"" + collectionName + "\"") + && l.contains("handler=\"" + handler + "\"")) .mapToDouble(s -> Double.parseDouble(s.substring(s.lastIndexOf(" ")))) .sum(); } @@ -106,8 +106,8 @@ public final class SolrJMetricTestUtils { .filter( l -> l.contains(metricName) - && l.contains(String.format("category=\"%s\"", category)) - && l.contains(String.format("handler=\"%s\"", handler))) + && l.contains("category=\"" + category + "\"") + && l.contains("handler=\"" + handler + "\"")) .mapToDouble(s -> Double.parseDouble(s.substring(s.lastIndexOf(" ")))) .sum(); } 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 76ce6a9f90e..e0d393c82c1 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 @@ -674,7 +674,7 @@ public class CloudHttp2SolrClientTest extends SolrCloudTestCase { runner.getBaseUrl().toString(), SolrRequest.SolrRequestType.ADMIN.name(), adminPath); - errorsBefore += numRequests; + errorsBefore += numRequests.longValue(); if (log.isInfoEnabled()) { log.info( "Found {} requests to {} on {}", numRequests, adminPath, runner.getBaseUrl()); @@ -700,7 +700,7 @@ public class CloudHttp2SolrClientTest extends SolrCloudTestCase { runner.getBaseUrl().toString(), SolrRequest.SolrRequestType.ADMIN.name(), adminPath); - errorsAfter += numRequests; + errorsAfter += numRequests.longValue(); if (log.isInfoEnabled()) { log.info( "Found {} requests to {} on {}", numRequests, adminPath, runner.getBaseUrl()); 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 9fb3bc4a9b0..8768f209268 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 @@ -614,7 +614,7 @@ public class CloudSolrClientTest extends SolrCloudTestCase { Double numRequests = SolrJMetricTestUtils.getNumNodeRequestErrors( runner.getBaseUrl().toString(), SolrRequestType.ADMIN.name(), adminPath); - errorsBefore += numRequests; + errorsBefore += numRequests.longValue(); if (log.isInfoEnabled()) { log.info( "Found {} requests to {} on {}", numRequests, adminPath, runner.getBaseUrl()); @@ -637,7 +637,7 @@ public class CloudSolrClientTest extends SolrCloudTestCase { Double numRequests = SolrJMetricTestUtils.getNumNodeRequestErrors( runner.getBaseUrl().toString(), SolrRequestType.ADMIN.name(), adminPath); - errorsAfter += numRequests; + errorsAfter += numRequests.longValue(); if (log.isInfoEnabled()) { log.info( "Found {} requests to {} on {}", numRequests, adminPath, runner.getBaseUrl());
