This is an automated email from the ASF dual-hosted git repository. mlbiscoc pushed a commit to branch feature/SOLR-17458-rebased in repository https://gitbox.apache.org/repos/asf/solr.git
commit e9ee7836d52f4c8bd7f400471d41ac239f872576 Author: bjacobowitz <[email protected]> AuthorDate: Tue Sep 16 15:11:13 2025 -0400 SOLR-17806 Migrate AuthenticationPlugin metrics to OTEL (#3636) * SOLR-17806 Migrate AuthenticationPlugin metrics to OTEL * SOLR-17806 Code review comments --- .../otel/instruments/AttributedLongTimer.java | 16 +- .../apache/solr/security/AuthenticationPlugin.java | 83 +++++--- .../org/apache/solr/security/BasicAuthPlugin.java | 4 +- .../org/apache/solr/security/MultiAuthPlugin.java | 1 + .../solr/security/PKIAuthenticationPlugin.java | 2 +- .../core/src/java/org/apache/solr/util/RTimer.java | 15 +- .../solr/security/BasicAuthIntegrationTest.java | 29 +-- .../apache/solr/security/CertAuthPluginTest.java | 53 ++++- .../solr/security/TestPKIAuthenticationPlugin.java | 17 ++ solr/modules/jwt-auth/build.gradle | 2 + solr/modules/jwt-auth/gradle.lockfile | 6 +- .../apache/solr/security/jwt/JWTAuthPlugin.java | 6 +- .../security/jwt/JWTAuthPluginIntegrationTest.java | 18 ++ .../apache/solr/cloud/SolrCloudAuthTestCase.java | 218 ++++++++++++++++----- .../org/apache/solr/util/SolrMetricTestUtils.java | 20 +- 15 files changed, 393 insertions(+), 97 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/metrics/otel/instruments/AttributedLongTimer.java b/solr/core/src/java/org/apache/solr/metrics/otel/instruments/AttributedLongTimer.java index 588a81507db..cda4daa4ba5 100644 --- a/solr/core/src/java/org/apache/solr/metrics/otel/instruments/AttributedLongTimer.java +++ b/solr/core/src/java/org/apache/solr/metrics/otel/instruments/AttributedLongTimer.java @@ -18,6 +18,7 @@ package org.apache.solr.metrics.otel.instruments; import io.opentelemetry.api.common.Attributes; import io.opentelemetry.api.metrics.LongHistogram; +import java.util.concurrent.TimeUnit; import org.apache.solr.util.RTimer; /** @@ -32,16 +33,29 @@ public class AttributedLongTimer extends AttributedLongHistogram { /** * Return a {@link MetricTimer} and starts the timer. When the timer calls {@link - * MetricTimer#stop()}, the elapsed time is recorded + * MetricTimer#stop()}, the elapsed time is recorded, in milliseconds */ public MetricTimer start() { return new MetricTimer(this); } + /** + * Return a {@link MetricTimer} and starts the timer. When the timer calls {@link + * MetricTimer#stop()}, the elapsed time is recorded, in the specified TimeUnit + */ + public MetricTimer start(TimeUnit unit) { + return new MetricTimer(this, unit); + } + public static class MetricTimer extends RTimer { private final AttributedLongTimer bound; private MetricTimer(AttributedLongTimer bound) { + this(bound, TimeUnit.MILLISECONDS); + } + + private MetricTimer(AttributedLongTimer bound, TimeUnit unit) { + super(unit); this.bound = bound; } diff --git a/solr/core/src/java/org/apache/solr/security/AuthenticationPlugin.java b/solr/core/src/java/org/apache/solr/security/AuthenticationPlugin.java index 8d588aada46..ed7646ad7b4 100644 --- a/solr/core/src/java/org/apache/solr/security/AuthenticationPlugin.java +++ b/solr/core/src/java/org/apache/solr/security/AuthenticationPlugin.java @@ -16,10 +16,8 @@ */ package org.apache.solr.security; -import com.codahale.metrics.Counter; -import com.codahale.metrics.Meter; -import com.codahale.metrics.Timer; import io.opentelemetry.api.common.Attributes; +import io.opentelemetry.api.metrics.LongCounter; import jakarta.servlet.FilterChain; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletRequestWrapper; @@ -28,8 +26,12 @@ import java.security.Principal; import java.util.Map; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.TimeUnit; import org.apache.solr.core.SolrInfoBean; 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.eclipse.jetty.client.Request; /** @@ -44,14 +46,13 @@ public abstract class AuthenticationPlugin implements SolrInfoBean { private Set<String> metricNames = ConcurrentHashMap.newKeySet(); protected SolrMetricsContext solrMetricsContext; - protected Meter numErrors = new Meter(); - protected Counter requests = new Counter(); - protected Timer requestTimes = new Timer(); - protected Counter totalTime = new Counter(); - protected Counter numAuthenticated = new Counter(); - protected Counter numPassThrough = new Counter(); - protected Counter numWrongCredentials = new Counter(); - protected Counter numMissingCredentials = new Counter(); + protected AttributedLongCounter numErrors; + protected AttributedLongCounter requests; + protected AttributedLongTimer requestTimes; + protected AttributedLongCounter numAuthenticated; + protected AttributedLongCounter numPassThrough; + protected AttributedLongCounter numWrongCredentials; + protected AttributedLongCounter numMissingCredentials; /** * This is called upon loading up of a plugin, used for setting it up. @@ -84,16 +85,16 @@ public abstract class AuthenticationPlugin implements SolrInfoBean { public final boolean authenticate( HttpServletRequest request, HttpServletResponse response, FilterChain filterChain) throws Exception { - Timer.Context timer = requestTimes.time(); + AttributedLongTimer.MetricTimer timer = requestTimes.start(TimeUnit.NANOSECONDS); requests.inc(); try { return doAuthenticate(request, response, filterChain); } catch (Exception e) { - numErrors.mark(); + numErrors.inc(); throw e; } finally { - long elapsed = timer.stop(); - totalTime.inc(elapsed); + // Record the timing metric + timer.stop(); } } @@ -142,24 +143,58 @@ public abstract class AuthenticationPlugin implements SolrInfoBean { return solrMetricsContext; } - // TODO SOLR-17458: Migrate to Otel @Override public void initializeMetrics( SolrMetricsContext parentContext, Attributes attributes, String scope) { this.solrMetricsContext = parentContext.getChildContext(this); + Attributes attrsWithCategory = + Attributes.builder() + .putAll(attributes) + .put("category", getCategory().toString()) + .put("plugin_name", this.getClass().getSimpleName()) + .build(); // Metrics - numErrors = this.solrMetricsContext.meter("errors", getCategory().toString(), scope); - requests = this.solrMetricsContext.counter("requests", getCategory().toString(), scope); + numErrors = + new AttributedLongCounter( + this.solrMetricsContext.longCounter( + "solr_authentication_errors", "Count of errors during authentication"), + attrsWithCategory); + requests = + new AttributedLongCounter( + this.solrMetricsContext.longCounter( + "solr_authentication_requests", "Count of requests for authentication"), + attrsWithCategory); numAuthenticated = - this.solrMetricsContext.counter("authenticated", getCategory().toString(), scope); + new AttributedLongCounter( + this.solrMetricsContext.longCounter( + "solr_authentication_num_authenticated", + "Count of successful requests for authentication"), + attrsWithCategory); numPassThrough = - this.solrMetricsContext.counter("passThrough", getCategory().toString(), scope); + new AttributedLongCounter( + this.solrMetricsContext.longCounter( + "solr_authentication_num_pass_through", + "Count of requests allowed to pass through without authentication credentials (as enabled with configuration \"blockUnknown\": false)"), + attrsWithCategory); + LongCounter solrAuthenticationPluginFail = + this.solrMetricsContext.longCounter( + "solr_authentication_failures", + "Count of authentication failures (unsuccessful, but processed correctly)"); numWrongCredentials = - this.solrMetricsContext.counter("failWrongCredentials", getCategory().toString(), scope); + new AttributedLongCounter( + solrAuthenticationPluginFail, + attrsWithCategory.toBuilder().put(TYPE_ATTR, "wrong_credentials").build()); numMissingCredentials = - this.solrMetricsContext.counter("failMissingCredentials", getCategory().toString(), scope); - requestTimes = this.solrMetricsContext.timer("requestTimes", getCategory().toString(), scope); - totalTime = this.solrMetricsContext.counter("totalTime", getCategory().toString(), scope); + new AttributedLongCounter( + solrAuthenticationPluginFail, + attrsWithCategory.toBuilder().put(TYPE_ATTR, "missing_credentials").build()); + requestTimes = + new AttributedLongTimer( + this.solrMetricsContext.longHistogram( + "solr_authentication_request_times", + "Distribution of authentication request durations", + OtelUnit.NANOSECONDS), + attrsWithCategory); } @Override diff --git a/solr/core/src/java/org/apache/solr/security/BasicAuthPlugin.java b/solr/core/src/java/org/apache/solr/security/BasicAuthPlugin.java index 753a19ad0d1..be9b4e8deba 100644 --- a/solr/core/src/java/org/apache/solr/security/BasicAuthPlugin.java +++ b/solr/core/src/java/org/apache/solr/security/BasicAuthPlugin.java @@ -148,7 +148,7 @@ public class BasicAuthPlugin extends AuthenticationPlugin return true; } } else { - numErrors.mark(); + numErrors.inc(); authenticationFailure(response, isAjaxRequest, "Invalid authentication token"); return false; } @@ -156,7 +156,7 @@ public class BasicAuthPlugin extends AuthenticationPlugin throw new Error("Couldn't retrieve authentication", e); } } else { - numErrors.mark(); + numErrors.inc(); authenticationFailure(response, isAjaxRequest, "Malformed Basic Auth header"); return false; } diff --git a/solr/core/src/java/org/apache/solr/security/MultiAuthPlugin.java b/solr/core/src/java/org/apache/solr/security/MultiAuthPlugin.java index 903218eed56..7c7e5a4b5df 100644 --- a/solr/core/src/java/org/apache/solr/security/MultiAuthPlugin.java +++ b/solr/core/src/java/org/apache/solr/security/MultiAuthPlugin.java @@ -202,6 +202,7 @@ public class MultiAuthPlugin extends AuthenticationPlugin // TODO SOLR-17458: Add Otel plugin.initializeMetrics(parentContext, Attributes.empty(), scope); } + super.initializeMetrics(parentContext, attributes, scope); } private String getSchemeFromAuthHeader(final String authHeader) { diff --git a/solr/core/src/java/org/apache/solr/security/PKIAuthenticationPlugin.java b/solr/core/src/java/org/apache/solr/security/PKIAuthenticationPlugin.java index 3a7009682fd..44251bb324d 100644 --- a/solr/core/src/java/org/apache/solr/security/PKIAuthenticationPlugin.java +++ b/solr/core/src/java/org/apache/solr/security/PKIAuthenticationPlugin.java @@ -224,7 +224,7 @@ public class PKIAuthenticationPlugin extends AuthenticationPlugin */ private boolean sendError(HttpServletResponse response, boolean v2, String message) throws IOException { - numErrors.mark(); + numErrors.inc(); log.error(message); response.setHeader(HttpHeader.WWW_AUTHENTICATE.asString(), v2 ? HEADER_V2 : HEADER); response.sendError(HttpServletResponse.SC_UNAUTHORIZED, message); diff --git a/solr/core/src/java/org/apache/solr/util/RTimer.java b/solr/core/src/java/org/apache/solr/util/RTimer.java index e4a46bffb71..40934cb2868 100644 --- a/solr/core/src/java/org/apache/solr/util/RTimer.java +++ b/solr/core/src/java/org/apache/solr/util/RTimer.java @@ -35,6 +35,7 @@ public class RTimer { private TimerImpl timerImpl; private double time; private double culmTime; + private TimeUnit timeUnit; protected interface TimerImpl { void start(); @@ -43,8 +44,13 @@ public class RTimer { } private static class NanoTimeTimerImpl implements TimerImpl { + private final TimeUnit timeUnit; private long start; + public NanoTimeTimerImpl(TimeUnit timeUnit) { + this.timeUnit = timeUnit; + } + @Override public void start() { start = System.nanoTime(); @@ -52,15 +58,20 @@ public class RTimer { @Override public double elapsed() { - return TimeUnit.MILLISECONDS.convert(System.nanoTime() - start, TimeUnit.NANOSECONDS); + return timeUnit.convert(System.nanoTime() - start, TimeUnit.NANOSECONDS); } } protected TimerImpl newTimerImpl() { - return new NanoTimeTimerImpl(); + return new NanoTimeTimerImpl(timeUnit); } public RTimer() { + this(TimeUnit.MILLISECONDS); + } + + public RTimer(TimeUnit timeUnit) { + this.timeUnit = timeUnit; time = 0; culmTime = 0; timerImpl = newTimerImpl(); diff --git a/solr/core/src/test/org/apache/solr/security/BasicAuthIntegrationTest.java b/solr/core/src/test/org/apache/solr/security/BasicAuthIntegrationTest.java index 4ac0ca904a7..c4035ea0380 100644 --- a/solr/core/src/test/org/apache/solr/security/BasicAuthIntegrationTest.java +++ b/solr/core/src/test/org/apache/solr/security/BasicAuthIntegrationTest.java @@ -19,7 +19,6 @@ package org.apache.solr.security; import static java.nio.charset.StandardCharsets.UTF_8; import static java.util.Collections.singletonMap; -import com.codahale.metrics.MetricRegistry; import java.io.IOException; import java.lang.invoke.MethodHandles; import java.nio.charset.StandardCharsets; @@ -128,7 +127,7 @@ public class BasicAuthIntegrationTest extends SolrCloudAuthTestCase { baseUrl = randomJetty.getBaseUrl().toString(); verifySecurityStatus( cl, baseUrl + authcPrefix, "authentication/class", "solr.BasicAuthPlugin", 20); - assertNumberOfMetrics(16); // Basic auth metrics available + assertAuthMetricsMinimums(1, 0, 1, 0, 0, 0); assertPkiAuthMetricsMinimums(0, 0, 0, 0, 0, 0); @@ -399,16 +398,22 @@ public class BasicAuthIntegrationTest extends SolrCloudAuthTestCase { } } - private void assertNumberOfMetrics(int num) { - MetricRegistry registry0 = - cluster.getJettySolrRunner(0).getCoreContainer().getMetricManager().registry("solr.node"); - assertNotNull(registry0); - - assertEquals( - num, - registry0.getMetrics().entrySet().stream() - .filter(e -> e.getKey().startsWith("SECURITY")) - .count()); + private void assertAuthMetricsMinimums( + int requests, + int authenticated, + int passThrough, + int failWrongCredentials, + int failMissingCredentials, + int errors) + throws InterruptedException { + super.assertAuthMetricsMinimums( + BasicAuthPlugin.class, + requests, + authenticated, + passThrough, + failWrongCredentials, + failMissingCredentials, + errors); } private QueryResponse executeQuery(ModifiableSolrParams params, String user, String pass) 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 e724ad7ab6e..db37c071ec5 100644 --- a/solr/core/src/test/org/apache/solr/security/CertAuthPluginTest.java +++ b/solr/core/src/test/org/apache/solr/security/CertAuthPluginTest.java @@ -23,6 +23,9 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import io.opentelemetry.api.common.Attributes; +import io.prometheus.metrics.model.snapshots.CounterSnapshot; +import io.prometheus.metrics.model.snapshots.Labels; import jakarta.servlet.FilterChain; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; @@ -30,6 +33,11 @@ import java.security.cert.X509Certificate; import java.util.Collections; import javax.security.auth.x500.X500Principal; import org.apache.solr.SolrTestCaseJ4; +import org.apache.solr.core.CoreContainer; +import org.apache.solr.core.SolrCore; +import org.apache.solr.metrics.SolrMetricsContext; +import org.apache.solr.util.SolrMetricTestUtils; +import org.junit.After; import org.junit.Before; import org.junit.BeforeClass; import org.junit.Test; @@ -46,10 +54,23 @@ public class CertAuthPluginTest extends SolrTestCaseJ4 { @Before public void setUp() throws Exception { super.setUp(); + CoreContainer coreContainer = + SolrTestCaseJ4.createCoreContainer( + "collection1", "data", "solrconfig-basic.xml", "schema.xml"); + String registryName = "solr.core.collection1"; plugin = new CertAuthPlugin(); + SolrMetricsContext solrMetricsContext = + new SolrMetricsContext(coreContainer.getMetricManager(), registryName, "foo"); + plugin.initializeMetrics(solrMetricsContext, Attributes.empty(), null); plugin.init(Collections.emptyMap()); } + @After + public void tearDown() throws Exception { + deleteCore(); + super.tearDown(); + } + @Test public void testAuthenticateOk() throws Exception { X500Principal principal = new X500Principal("CN=NAME"); @@ -63,7 +84,17 @@ public class CertAuthPluginTest extends SolrTestCaseJ4 { (req, rsp) -> assertEquals(principal, ((HttpServletRequest) req).getUserPrincipal()); assertTrue(plugin.doAuthenticate(request, null, chain)); - assertEquals(1, plugin.numAuthenticated.getCount()); + Labels labels = + Labels.of( + "otel_scope_name", + "org.apache.solr", + "category", + "SECURITY", + "plugin_name", + "CertAuthPlugin"); + long missingCredentialsCount = + getLongMetricValue("solr_authentication_num_authenticated", labels); + assertEquals(1L, missingCredentialsCount); } @Test @@ -76,6 +107,24 @@ public class CertAuthPluginTest extends SolrTestCaseJ4 { assertFalse(plugin.doAuthenticate(request, response, null)); verify(response).sendError(eq(401), anyString()); - assertEquals(1, plugin.numMissingCredentials.getCount()); + Labels labels = + Labels.of( + "otel_scope_name", + "org.apache.solr", + "category", + "SECURITY", + "type", + "missing_credentials", + "plugin_name", + "CertAuthPlugin"); + long missingCredentialsCount = getLongMetricValue("solr_authentication_failures", labels); + assertEquals(1L, missingCredentialsCount); + } + + private long getLongMetricValue(String metricName, Labels labels) { + SolrCore core = h.getCore(); + CounterSnapshot.CounterDataPointSnapshot metric = + SolrMetricTestUtils.getCounterDatapoint(core, metricName, labels); + return (metric != null) ? (long) metric.getValue() : 0L; } } diff --git a/solr/core/src/test/org/apache/solr/security/TestPKIAuthenticationPlugin.java b/solr/core/src/test/org/apache/solr/security/TestPKIAuthenticationPlugin.java index 699f6249dc3..42e1800274f 100644 --- a/solr/core/src/test/org/apache/solr/security/TestPKIAuthenticationPlugin.java +++ b/solr/core/src/test/org/apache/solr/security/TestPKIAuthenticationPlugin.java @@ -23,6 +23,9 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import io.opentelemetry.api.common.Attributes; +import io.opentelemetry.api.metrics.LongCounter; +import io.opentelemetry.api.metrics.LongHistogram; import jakarta.servlet.FilterChain; import jakarta.servlet.ServletRequest; import jakarta.servlet.http.HttpServletRequest; @@ -39,6 +42,7 @@ import org.apache.http.auth.BasicUserPrincipal; import org.apache.http.message.BasicHttpRequest; import org.apache.solr.SolrTestCaseJ4; import org.apache.solr.common.params.ModifiableSolrParams; +import org.apache.solr.metrics.SolrMetricsContext; import org.apache.solr.request.LocalSolrQueryRequest; import org.apache.solr.request.SolrRequestInfo; import org.apache.solr.response.SolrQueryResponse; @@ -118,9 +122,20 @@ public class TestPKIAuthenticationPlugin extends SolrTestCaseJ4 { mockReq = createMockRequest(header); mock = new MockPKIAuthenticationPlugin(nodeName); + mockMetrics(mock); request = new BasicHttpRequest("GET", "http://localhost:56565";); } + private static void mockMetrics(MockPKIAuthenticationPlugin mock) { + SolrMetricsContext smcMock = mock(SolrMetricsContext.class); + when(smcMock.getChildContext(any())).thenReturn(smcMock); + LongCounter longCounterMock = mock(LongCounter.class); + LongHistogram longHistogramMock = mock(LongHistogram.class); + when(smcMock.longCounter(any(), any())).thenReturn(longCounterMock); + when(smcMock.longHistogram(any(), any(), any())).thenReturn(longHistogramMock); + mock.initializeMetrics(smcMock, Attributes.empty(), ""); + } + @Override public void tearDown() throws Exception { if (mock != null) { @@ -165,6 +180,7 @@ public class TestPKIAuthenticationPlugin extends SolrTestCaseJ4 { } } }; + mockMetrics(mock1); // Setup regular superuser request mock.solrRequestInfo = null; @@ -191,6 +207,7 @@ public class TestPKIAuthenticationPlugin extends SolrTestCaseJ4 { System.setProperty(PKIAuthenticationPlugin.SEND_VERSION, "v1"); System.setProperty(PKIAuthenticationPlugin.ACCEPT_VERSIONS, "v2"); mock = new MockPKIAuthenticationPlugin(nodeName); + mockMetrics(mock); principal.set(new BasicUserPrincipal("solr")); mock.solrRequestInfo = new SolrRequestInfo(localSolrQueryRequest, new SolrQueryResponse()); diff --git a/solr/modules/jwt-auth/build.gradle b/solr/modules/jwt-auth/build.gradle index 9f4078f0478..be4ae8f38bc 100644 --- a/solr/modules/jwt-auth/build.gradle +++ b/solr/modules/jwt-auth/build.gradle @@ -46,6 +46,8 @@ dependencies { testImplementation project(':solr:test-framework') testImplementation libs.apache.lucene.testframework testImplementation libs.junit.junit + testImplementation libs.prometheus.metrics.model + testImplementation libs.opentelemetry.exporter.prometheus testImplementation(libs.mockito.core, { exclude group: "net.bytebuddy", module: "byte-buddy-agent" diff --git a/solr/modules/jwt-auth/gradle.lockfile b/solr/modules/jwt-auth/gradle.lockfile index 88772eaf17e..bf36af6c0b0 100644 --- a/solr/modules/jwt-auth/gradle.lockfile +++ b/solr/modules/jwt-auth/gradle.lockfile @@ -77,10 +77,12 @@ io.opentelemetry:opentelemetry-api:1.53.0=compileClasspath,jarValidation,runtime io.opentelemetry:opentelemetry-common:1.53.0=compileClasspath,jarValidation,runtimeClasspath,runtimeLibs,solrPlatformLibs,testCompileClasspath,testRuntimeClasspath io.opentelemetry:opentelemetry-context:1.53.0=compileClasspath,jarValidation,runtimeClasspath,runtimeLibs,solrPlatformLibs,testCompileClasspath,testRuntimeClasspath io.opentelemetry:opentelemetry-exporter-common:1.50.0=jarValidation,runtimeClasspath,runtimeLibs,solrPlatformLibs,testRuntimeClasspath -io.opentelemetry:opentelemetry-exporter-prometheus:1.50.0-alpha=jarValidation,runtimeClasspath,runtimeLibs,solrPlatformLibs,testRuntimeClasspath +io.opentelemetry:opentelemetry-exporter-prometheus:1.50.0-alpha=jarValidation,runtimeClasspath,runtimeLibs,solrPlatformLibs,testCompileClasspath,testRuntimeClasspath +io.opentelemetry:opentelemetry-sdk-common:1.50.0=testCompileClasspath io.opentelemetry:opentelemetry-sdk-common:1.53.0=jarValidation,runtimeClasspath,runtimeLibs,solrPlatformLibs,testRuntimeClasspath io.opentelemetry:opentelemetry-sdk-extension-autoconfigure-spi:1.50.0=jarValidation,runtimeClasspath,runtimeLibs,solrPlatformLibs,testRuntimeClasspath io.opentelemetry:opentelemetry-sdk-logs:1.53.0=jarValidation,runtimeClasspath,runtimeLibs,solrPlatformLibs,testRuntimeClasspath +io.opentelemetry:opentelemetry-sdk-metrics:1.50.0=testCompileClasspath io.opentelemetry:opentelemetry-sdk-metrics:1.53.0=jarValidation,runtimeClasspath,runtimeLibs,solrPlatformLibs,testRuntimeClasspath io.opentelemetry:opentelemetry-sdk-trace:1.53.0=jarValidation,runtimeClasspath,runtimeLibs,solrPlatformLibs,testRuntimeClasspath io.opentelemetry:opentelemetry-sdk:1.53.0=jarValidation,runtimeClasspath,runtimeLibs,solrPlatformLibs,testRuntimeClasspath @@ -90,7 +92,7 @@ io.prometheus:prometheus-metrics-exporter-httpserver:1.3.6=jarValidation,testRun io.prometheus:prometheus-metrics-exposition-formats:1.1.0=runtimeClasspath,runtimeLibs,solrPlatformLibs io.prometheus:prometheus-metrics-exposition-formats:1.3.6=jarValidation,testRuntimeClasspath io.prometheus:prometheus-metrics-exposition-textformats:1.3.6=jarValidation,testRuntimeClasspath -io.prometheus:prometheus-metrics-model:1.1.0=runtimeClasspath,runtimeLibs,solrPlatformLibs +io.prometheus:prometheus-metrics-model:1.1.0=runtimeClasspath,runtimeLibs,solrPlatformLibs,testCompileClasspath io.prometheus:prometheus-metrics-model:1.3.6=jarValidation,testRuntimeClasspath io.sgr:s2-geometry-library-java:1.0.0=jarValidation,runtimeClasspath,runtimeLibs,solrPlatformLibs,testRuntimeClasspath io.swagger.core.v3:swagger-annotations-jakarta:2.2.22=compileClasspath,jarValidation,runtimeClasspath,runtimeLibs,solrPlatformLibs,testCompileClasspath,testRuntimeClasspath diff --git a/solr/modules/jwt-auth/src/java/org/apache/solr/security/jwt/JWTAuthPlugin.java b/solr/modules/jwt-auth/src/java/org/apache/solr/security/jwt/JWTAuthPlugin.java index bf7068588db..323e4a6646f 100644 --- a/solr/modules/jwt-auth/src/java/org/apache/solr/security/jwt/JWTAuthPlugin.java +++ b/solr/modules/jwt-auth/src/java/org/apache/solr/security/jwt/JWTAuthPlugin.java @@ -443,7 +443,7 @@ public class JWTAuthPlugin extends AuthenticationPlugin } if (jwtConsumer == null) { log.warn("JWTAuth not configured"); - numErrors.mark(); + numErrors.inc(); throw new SolrException( SolrException.ErrorCode.SERVER_ERROR, "JWTAuth plugin not correctly configured"); } @@ -484,7 +484,7 @@ public class JWTAuthPlugin extends AuthenticationPlugin final Principal principal = authResponse.getPrincipal(); request = wrapWithPrincipal(request, principal); if (!(principal instanceof JWTPrincipal)) { - numErrors.mark(); + numErrors.inc(); throw new SolrException( SolrException.ErrorCode.SERVER_ERROR, "JWTAuth plugin says AUTHENTICATED but no token extracted"); @@ -510,7 +510,7 @@ public class JWTAuthPlugin extends AuthenticationPlugin "Authentication failed. {}, {}", authResponse.getAuthCode(), authResponse.getAuthCode().getMsg()); - numErrors.mark(); + numErrors.inc(); authenticationFailure( response, authResponse.getAuthCode().getMsg(), diff --git a/solr/modules/jwt-auth/src/test/org/apache/solr/security/jwt/JWTAuthPluginIntegrationTest.java b/solr/modules/jwt-auth/src/test/org/apache/solr/security/jwt/JWTAuthPluginIntegrationTest.java index 45e35d753fa..f0ca70fe55c 100644 --- a/solr/modules/jwt-auth/src/test/org/apache/solr/security/jwt/JWTAuthPluginIntegrationTest.java +++ b/solr/modules/jwt-auth/src/test/org/apache/solr/security/jwt/JWTAuthPluginIntegrationTest.java @@ -342,6 +342,24 @@ public class JWTAuthPluginIntegrationTest extends SolrCloudAuthTestCase { return "Bearer " + jws.getCompactSerialization(); } + private void assertAuthMetricsMinimums( + int requests, + int authenticated, + int passThrough, + int failWrongCredentials, + int failMissingCredentials, + int errors) + throws InterruptedException { + super.assertAuthMetricsMinimums( + JWTAuthPlugin.class, + requests, + authenticated, + passThrough, + failWrongCredentials, + failMissingCredentials, + errors); + } + /** * Configure solr cluster with a security.json talking to MockOAuth2 server * diff --git a/solr/test-framework/src/java/org/apache/solr/cloud/SolrCloudAuthTestCase.java b/solr/test-framework/src/java/org/apache/solr/cloud/SolrCloudAuthTestCase.java index 99085837700..c9ed378aa27 100644 --- a/solr/test-framework/src/java/org/apache/solr/cloud/SolrCloudAuthTestCase.java +++ b/solr/test-framework/src/java/org/apache/solr/cloud/SolrCloudAuthTestCase.java @@ -24,6 +24,11 @@ import com.codahale.metrics.Meter; import com.codahale.metrics.Metric; import com.codahale.metrics.MetricRegistry; import com.codahale.metrics.Timer; +import io.opentelemetry.exporter.prometheus.PrometheusMetricReader; +import io.prometheus.metrics.model.snapshots.CounterSnapshot; +import io.prometheus.metrics.model.snapshots.DataPointSnapshot; +import io.prometheus.metrics.model.snapshots.HistogramSnapshot; +import io.prometheus.metrics.model.snapshots.Labels; import java.io.IOException; import java.lang.invoke.MethodHandles; import java.util.ArrayList; @@ -44,7 +49,10 @@ import org.apache.http.util.EntityUtils; import org.apache.solr.client.solrj.impl.HttpClientUtil; import org.apache.solr.common.util.StrUtils; import org.apache.solr.common.util.Utils; +import org.apache.solr.core.CoreContainer; import org.apache.solr.embedded.JettySolrRunner; +import org.apache.solr.security.AuthenticationPlugin; +import org.apache.solr.util.SolrMetricTestUtils; import org.apache.solr.util.TimeOut; import org.junit.AfterClass; import org.junit.BeforeClass; @@ -60,32 +68,22 @@ public class SolrCloudAuthTestCase extends SolrCloudTestCase { private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); private static final List<String> AUTH_METRICS_KEYS = Arrays.asList( - "errors", - "requests", - "authenticated", - "passThrough", - "failWrongCredentials", - "failMissingCredentials", - "requestTimes", - "totalTime"); - private static final List<String> AUTH_METRICS_METER_KEYS = Arrays.asList("errors", "count"); + "solr_authentication_errors", + "solr_authentication_requests", + "solr_authentication_num_authenticated", + "solr_authentication_num_pass_through", + "solr_authentication_failures/wrong_credentials", + "solr_authentication_failures/missing_credentials", + "solr_authentication_request_times_nanoseconds"); + private static final List<String> AUTH_METRICS_METER_KEYS = + Arrays.asList("solr_authentication_errors", "count"); private static final List<String> AUTH_METRICS_TIMER_KEYS = - Collections.singletonList("requestTimes"); - private static final String METRICS_PREFIX_PKI = "SECURITY./authentication/pki."; - private static final String METRICS_PREFIX = "SECURITY./authentication."; + Collections.singletonList("solr_authentication_request_times_nanoseconds"); @SuppressWarnings({"rawtypes"}) public static final Predicate NOT_NULL_PREDICATE = o -> o != null; private static final List<String> AUDIT_METRICS_KEYS = Arrays.asList("count"); - private static final List<String> AUTH_METRICS_TO_COMPARE = - Arrays.asList( - "requests", - "authenticated", - "passThrough", - "failWrongCredentials", - "failMissingCredentials", - "errors"); private static final List<String> AUDIT_METRICS_TO_COMPARE = Arrays.asList("count"); @BeforeClass @@ -107,8 +105,22 @@ public class SolrCloudAuthTestCase extends SolrCloudTestCase { int failMissingCredentials, int errors) throws InterruptedException { - assertAuthMetricsMinimums( - METRICS_PREFIX_PKI, + String handler = "/authentication/pki"; + String registryName = "solr.node"; + Labels labels = + Labels.of( + "otel_scope_name", + "org.apache.solr", + "category", + "SECURITY", + "handler", + handler, + "plugin_name", + org.apache.solr.security.PKIAuthenticationPlugin.class.getSimpleName()); + assertAuthMetricsMinimumsPrometheus( + handler, + registryName, + labels, requests, authenticated, passThrough, @@ -124,6 +136,7 @@ public class SolrCloudAuthTestCase extends SolrCloudTestCase { * desired params and timeout */ protected void assertAuthMetricsMinimums( + Class<? extends AuthenticationPlugin> authPluginClass, int requests, int authenticated, int passThrough, @@ -131,8 +144,22 @@ public class SolrCloudAuthTestCase extends SolrCloudTestCase { int failMissingCredentials, int errors) throws InterruptedException { - assertAuthMetricsMinimums( - METRICS_PREFIX, + String handler = "/authentication"; + String registryName = "solr.node"; + Labels labels = + Labels.of( + "otel_scope_name", + "org.apache.solr", + "category", + "SECURITY", + "handler", + handler, + "plugin_name", + authPluginClass.getSimpleName()); + assertAuthMetricsMinimumsPrometheus( + handler, + registryName, + labels, requests, authenticated, passThrough, @@ -170,35 +197,35 @@ public class SolrCloudAuthTestCase extends SolrCloudTestCase { return counts; } - /** - * Common test method to be able to check auth metrics from any authentication plugin - * - * @param prefix the metrics key prefix, currently "SECURITY./authentication." for basic auth and - * "SECURITY./authentication/pki." for PKI - */ - private void assertAuthMetricsMinimums( - String prefix, + /** Common test method to be able to check auth metrics from any authentication plugin */ + void assertAuthMetricsMinimumsPrometheus( + String handler, + String registryName, + Labels labels, int requests, int authenticated, int passThrough, int failWrongCredentials, int failMissingCredentials, - int errors) - throws InterruptedException { + int errors) { Map<String, Long> expectedCounts = new HashMap<>(); - expectedCounts.put("requests", (long) requests); - expectedCounts.put("authenticated", (long) authenticated); - expectedCounts.put("passThrough", (long) passThrough); - expectedCounts.put("failWrongCredentials", (long) failWrongCredentials); - expectedCounts.put("failMissingCredentials", (long) failMissingCredentials); - expectedCounts.put("errors", (long) errors); + expectedCounts.put("solr_authentication_requests", (long) requests); + expectedCounts.put("solr_authentication_num_authenticated", (long) authenticated); + expectedCounts.put("solr_authentication_num_pass_through", (long) passThrough); + expectedCounts.put( + "solr_authentication_failures/wrong_credentials", (long) failWrongCredentials); + expectedCounts.put( + "solr_authentication_failures/missing_credentials", (long) failMissingCredentials); + expectedCounts.put("solr_authentication_errors", (long) errors); - final Map<String, Long> counts = countSecurityMetrics(cluster, prefix, AUTH_METRICS_KEYS); - final boolean success = isMetricsEqualOrLarger(AUTH_METRICS_TO_COMPARE, expectedCounts, counts); + final Map<String, Long> counts = + countSecurityMetricsPrometheus(cluster, AUTH_METRICS_KEYS, registryName, labels); + final boolean success = + expectedCounts.keySet().stream().allMatch(k -> counts.get(k) >= expectedCounts.get(k)); assertTrue( - "Expected metric minimums for prefix " - + prefix + "Expected metric minimums for handler " + + handler + ": " + expectedCounts + ", but got: " @@ -207,10 +234,107 @@ public class SolrCloudAuthTestCase extends SolrCloudTestCase { + "security.json; see SOLR-13464 for test work around)", success); - if (counts.get("requests") > 0) { - assertTrue("requestTimes count not > 1", counts.get("requestTimes") > 1); - assertTrue("totalTime not > 0", counts.get("totalTime") > 0); + if (counts.get("solr_authentication_requests") > 0) { + assertTrue( + "requestTimes count not > 0", + counts.get("solr_authentication_request_times_nanoseconds") > 0); + } + } + + /** + * Common test method to sum the prometheus metrics from any authentication plugin from all solr + * core containers + */ + Map<String, Long> countSecurityMetricsPrometheus( + MiniSolrCloudCluster cluster, List<String> keys, String registryName, Labels labels) { + List<Map<String, DataPointSnapshot>> metrics = new ArrayList<>(); + cluster + .getJettySolrRunners() + .forEach( + r -> { + metrics.add(getMetricValues(r.getCoreContainer(), keys, registryName, labels)); + }); + + Map<String, Long> counts = new HashMap<>(); + keys.forEach( + k -> { + counts.put(k, sumCountPrometheus(k, metrics)); + }); + return counts; + } + + private long counterToLong(CounterSnapshot.CounterDataPointSnapshot metric) { + if (metric == null) { + return 0L; + } + return (long) metric.getValue(); + } + + private long histogramToLongCount(HistogramSnapshot.HistogramDataPointSnapshot metric) { + if (metric == null) { + return 0; + } + return metric.getCount(); + } + + // Have to sum the metrics from all three shards/nodes + private long sumCountPrometheus(String key, List<Map<String, DataPointSnapshot>> metricsPerNode) { + assertTrue("Metric " + key + " does not exist", metricsPerNode.get(0).containsKey(key)); + if (AUTH_METRICS_METER_KEYS.contains(key)) { + return metricsPerNode.stream() + .mapToLong( + nodeMap -> counterToLong((CounterSnapshot.CounterDataPointSnapshot) nodeMap.get(key))) + .sum(); + } else if (AUTH_METRICS_TIMER_KEYS.contains(key)) { + // Sum of the count of timer metrics (NOT the sum of their values) + return metricsPerNode.stream() + .mapToLong( + nodeMap -> + histogramToLongCount( + (HistogramSnapshot.HistogramDataPointSnapshot) nodeMap.get(key))) + .sum(); + } else { + return metricsPerNode.stream() + .mapToLong( + nodeMap -> counterToLong((CounterSnapshot.CounterDataPointSnapshot) nodeMap.get(key))) + .sum(); + } + } + + private static Map<String, DataPointSnapshot> getMetricValues( + CoreContainer coreContainer, List<String> metricNames, String registryName, Labels labels) { + Map<String, DataPointSnapshot> metrics = new HashMap<>(); + PrometheusMetricReader prometheusMetricReader = + SolrMetricTestUtils.getPrometheusMetricReader(coreContainer, registryName); + for (String metricName : metricNames) { + if ("solr_authentication_request_times_nanoseconds".equals(metricName)) { + HistogramSnapshot.HistogramDataPointSnapshot metric = + SolrMetricTestUtils.getHistogramDatapoint(prometheusMetricReader, metricName, labels); + metrics.put(metricName, metric); + } else if ("solr_authentication_failures/wrong_credentials".equals(metricName)) { + // Fake metric name, actual metric will be in solr_authentication_failures with label type: + // wrong_credentials + Labels wrongCredsLabels = Labels.of("type", "wrong_credentials").merge(labels); + CounterSnapshot.CounterDataPointSnapshot wrongCredsMetric = + SolrMetricTestUtils.getCounterDatapoint( + prometheusMetricReader, "solr_authentication_failures", wrongCredsLabels); + + metrics.put("solr_authentication_failures/wrong_credentials", wrongCredsMetric); + } else if ("solr_authentication_failures/missing_credentials".equals(metricName)) { + // Fake metric name, actual metric will be in solr_authentication_failures with label type: + // missing_credentials + Labels missingCredsLabels = Labels.of("type", "missing_credentials").merge(labels); + CounterSnapshot.CounterDataPointSnapshot missingCredsMetric = + SolrMetricTestUtils.getCounterDatapoint( + prometheusMetricReader, "solr_authentication_failures", missingCredsLabels); + metrics.put("solr_authentication_failures/missing_credentials", missingCredsMetric); + } else { + CounterSnapshot.CounterDataPointSnapshot metric = + SolrMetricTestUtils.getCounterDatapoint(prometheusMetricReader, metricName, labels); + metrics.put(metricName, metric); + } } + return metrics; } /** diff --git a/solr/test-framework/src/java/org/apache/solr/util/SolrMetricTestUtils.java b/solr/test-framework/src/java/org/apache/solr/util/SolrMetricTestUtils.java index 3de85bf3018..8fcafc125c9 100644 --- a/solr/test-framework/src/java/org/apache/solr/util/SolrMetricTestUtils.java +++ b/solr/test-framework/src/java/org/apache/solr/util/SolrMetricTestUtils.java @@ -25,6 +25,7 @@ import io.prometheus.metrics.model.snapshots.DataPointSnapshot; import io.prometheus.metrics.model.snapshots.GaugeSnapshot; import io.prometheus.metrics.model.snapshots.HistogramSnapshot; import io.prometheus.metrics.model.snapshots.Labels; +import io.prometheus.metrics.model.snapshots.MetricSnapshots; import java.util.ArrayList; import java.util.HashMap; import java.util.List; @@ -169,7 +170,8 @@ public final class SolrMetricTestUtils { public static DataPointSnapshot getDataPointSnapshot( PrometheusMetricReader reader, String metricName, Labels labels) { - return reader.collect().stream() + MetricSnapshots metricSnapshots = reader.collect(); + return metricSnapshots.stream() .filter(ms -> ms.getMetadata().getPrometheusName().equals(metricName)) .findFirst() .flatMap( @@ -211,6 +213,11 @@ public final class SolrMetricTestUtils { private static <T> T getDatapoint( SolrCore core, String metricName, Labels labels, Class<T> snapshotType) { var reader = getPrometheusMetricReader(core); + return getDataPoint(reader, metricName, labels, snapshotType); + } + + private static <T> T getDataPoint( + PrometheusMetricReader reader, String metricName, Labels labels, Class<T> snapshotType) { return snapshotType.cast(SolrMetricTestUtils.getDataPointSnapshot(reader, metricName, labels)); } @@ -224,6 +231,17 @@ public final class SolrMetricTestUtils { return getDatapoint(core, metricName, labels, CounterSnapshot.CounterDataPointSnapshot.class); } + public static CounterSnapshot.CounterDataPointSnapshot getCounterDatapoint( + PrometheusMetricReader reader, String metricName, Labels labels) { + return getDataPoint(reader, metricName, labels, CounterSnapshot.CounterDataPointSnapshot.class); + } + + public static HistogramSnapshot.HistogramDataPointSnapshot getHistogramDatapoint( + PrometheusMetricReader reader, String metricName, Labels labels) { + return getDataPoint( + reader, metricName, labels, HistogramSnapshot.HistogramDataPointSnapshot.class); + } + public static HistogramSnapshot.HistogramDataPointSnapshot getHistogramDatapoint( SolrCore core, String metricName, Labels labels) { return getDatapoint(
