This is an automated email from the ASF dual-hosted git repository. angela pushed a commit to branch SLING-10164 in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-auth-core.git
commit ae4225da79e0b0549fec9dd59fb4e868b2acf784 Author: angela <[email protected]> AuthorDate: Tue Mar 2 11:18:22 2021 +0100 SLING-10164 : Add basic metrics to SlingAuthenticator (initial proposal with minimal testing) --- pom.xml | 17 +++++ .../auth/core/impl/SlingAuthenticationMetrics.java | 48 ++++++++++++ .../sling/auth/core/impl/SlingAuthenticator.java | 21 ++++- .../core/impl/SlingAuthenticationMetricsTest.java | 75 ++++++++++++++++++ .../auth/core/impl/SlingAuthenticatorOsgiTest.java | 89 ++++++++++++++++++++++ 5 files changed, 248 insertions(+), 2 deletions(-) diff --git a/pom.xml b/pom.xml index fb78d39..081fb04 100644 --- a/pom.xml +++ b/pom.xml @@ -124,6 +124,17 @@ <version>1.4</version> <scope>provided</scope> </dependency> + <dependency> + <groupId>org.apache.sling</groupId> + <artifactId>org.apache.sling.commons.metrics</artifactId> + <version>1.2.8</version> + </dependency> + <dependency> + <groupId>org.jetbrains</groupId> + <artifactId>annotations</artifactId> + <version>18.0.0</version> + <scope>provided</scope> + </dependency> <!-- Test Dependencies --> <dependency> @@ -152,5 +163,11 @@ <version>15.0</version> <scope>test</scope> </dependency> + <dependency> + <groupId>org.apache.sling</groupId> + <artifactId>org.apache.sling.testing.osgi-mock</artifactId> + <version>3.0.0</version> + <scope>test</scope> + </dependency> </dependencies> </project> diff --git a/src/main/java/org/apache/sling/auth/core/impl/SlingAuthenticationMetrics.java b/src/main/java/org/apache/sling/auth/core/impl/SlingAuthenticationMetrics.java new file mode 100644 index 0000000..6edbc4f --- /dev/null +++ b/src/main/java/org/apache/sling/auth/core/impl/SlingAuthenticationMetrics.java @@ -0,0 +1,48 @@ +/* + * 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.sling.auth.core.impl; + +import org.apache.sling.commons.metrics.Meter; +import org.apache.sling.commons.metrics.MetricsService; +import org.apache.sling.commons.metrics.Timer; +import org.jetbrains.annotations.NotNull; + +class SlingAuthenticationMetrics { + + private final Timer authenticateTimer; + private final Meter authenticateSuccess; + private final Meter authenticateFailed; + + SlingAuthenticationMetrics(@NotNull MetricsService metricsService) { + authenticateTimer = metricsService.timer("sling.auth.core.authenticate.timer"); + authenticateSuccess = metricsService.meter("sling.auth.core.authenticate.success"); + authenticateFailed = metricsService.meter("sling.auth.core.authenticate.failed"); + } + + @NotNull + Timer.Context authenticationTimerContext() { + return authenticateTimer.time(); + } + + void authenticateCompleted(boolean failed) { + if (failed) { + authenticateFailed.mark(); + } else { + authenticateSuccess.mark(); + } + } +} \ No newline at end of file diff --git a/src/main/java/org/apache/sling/auth/core/impl/SlingAuthenticator.java b/src/main/java/org/apache/sling/auth/core/impl/SlingAuthenticator.java index 6e203cc..6877ea4 100644 --- a/src/main/java/org/apache/sling/auth/core/impl/SlingAuthenticator.java +++ b/src/main/java/org/apache/sling/auth/core/impl/SlingAuthenticator.java @@ -60,6 +60,8 @@ import org.apache.sling.auth.core.spi.AuthenticationHandler; import org.apache.sling.auth.core.spi.AuthenticationInfo; import org.apache.sling.auth.core.spi.AuthenticationInfoPostProcessor; import org.apache.sling.auth.core.spi.DefaultAuthenticationFeedbackHandler; +import org.apache.sling.commons.metrics.MetricsService; +import org.apache.sling.commons.metrics.Timer; import org.osgi.framework.BundleContext; import org.osgi.framework.Constants; import org.osgi.framework.ServiceReference; @@ -333,6 +335,10 @@ public class SlingAuthenticator implements Authenticator, @Reference(policy=ReferencePolicy.DYNAMIC, cardinality = ReferenceCardinality.OPTIONAL) private volatile EventAdmin eventAdmin; + @Reference + private MetricsService metricsService; + private SlingAuthenticationMetrics metrics; + // ---------- SCR integration @Activate @@ -363,6 +369,8 @@ public class SlingAuthenticator implements Authenticator, bundleContext, authHandlerCache); authInfoPostProcessorTracker = new ServiceTracker(bundleContext, AuthenticationInfoPostProcessor.class, null); authInfoPostProcessorTracker.open(); + + metrics = new SlingAuthenticationMetrics(metricsService); } @Modified @@ -457,6 +465,13 @@ public class SlingAuthenticator implements Authenticator, webConsolePlugin.unregister(); webConsolePlugin = null; } + + if (metricsService != null) { + metricsService = null; + } + if (metrics != null) { + metrics = null; + } } // --------- AuthenticationSupport interface @@ -492,15 +507,17 @@ public class SlingAuthenticator implements Authenticator, request.removeAttribute(REQUEST_ATTRIBUTE_RESOLVER); } + Timer.Context ctx = metrics.authenticationTimerContext(); boolean process = doHandleSecurity(request, response); if (process && expectAuthenticationHandler(request)) { log.warn("handleSecurity: AuthenticationHandler did not block request; access denied"); request.removeAttribute(AuthenticationHandler.FAILURE_REASON); request.removeAttribute(AuthenticationHandler.FAILURE_REASON_CODE); AuthUtil.sendInvalid(request, response); - return false; + process = false; } - + ctx.stop(); + metrics.authenticateCompleted(process); return process; } diff --git a/src/test/java/org/apache/sling/auth/core/impl/SlingAuthenticationMetricsTest.java b/src/test/java/org/apache/sling/auth/core/impl/SlingAuthenticationMetricsTest.java new file mode 100644 index 0000000..50b0f51 --- /dev/null +++ b/src/test/java/org/apache/sling/auth/core/impl/SlingAuthenticationMetricsTest.java @@ -0,0 +1,75 @@ +/* + * 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.sling.auth.core.impl; + +import org.apache.sling.api.resource.ResourceResolverFactory; +import org.apache.sling.commons.metrics.Meter; +import org.apache.sling.commons.metrics.MetricsService; +import org.apache.sling.commons.metrics.Timer; +import org.junit.Before; +import org.junit.Test; + +import static org.junit.Assert.assertSame; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; +import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.Mockito.when; + +public class SlingAuthenticationMetricsTest { + + private Meter meter = mock(Meter.class); + private Timer.Context ctx = mock(Timer.Context.class); + private Timer timer = mock(Timer.class); + private final MetricsService metricsService = mock(MetricsService.class); + + private SlingAuthenticationMetrics metrics; + + @Before + public void before() { + when(timer.time()).thenReturn(ctx); + when(metricsService.meter(anyString())).thenReturn(meter); + when(metricsService.timer(anyString())).thenReturn(timer); + + metrics = new SlingAuthenticationMetrics(metricsService); + + verify(metricsService).timer(anyString()); + verify(metricsService, times(2)).meter(anyString()); + } + + @Test + public void testAuthenticationCompleted() { + metrics.authenticateCompleted(true); + metrics.authenticateCompleted(false); + verify(meter, times(2)).mark(); + verifyNoMoreInteractions(meter); + verifyNoInteractions(timer, ctx); + } + + @Test + public void testAuthenticationTimerContext() { + Timer.Context timerContext = metrics.authenticationTimerContext(); + timerContext.stop(); + + verify(timer).time(); + verify(ctx).stop(); + verifyNoMoreInteractions(timer, ctx); + verifyNoInteractions(meter); + } +} \ No newline at end of file diff --git a/src/test/java/org/apache/sling/auth/core/impl/SlingAuthenticatorOsgiTest.java b/src/test/java/org/apache/sling/auth/core/impl/SlingAuthenticatorOsgiTest.java new file mode 100644 index 0000000..1a5d4fe --- /dev/null +++ b/src/test/java/org/apache/sling/auth/core/impl/SlingAuthenticatorOsgiTest.java @@ -0,0 +1,89 @@ +/* + * 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.sling.auth.core.impl; + +import org.apache.sling.api.resource.LoginException; +import org.apache.sling.api.resource.ResourceResolver; +import org.apache.sling.api.resource.ResourceResolverFactory; +import org.apache.sling.auth.core.spi.AuthenticationInfo; +import org.apache.sling.commons.metrics.Meter; +import org.apache.sling.commons.metrics.MetricsService; +import org.apache.sling.commons.metrics.Timer; +import org.apache.sling.testing.mock.osgi.junit.OsgiContext; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.mockito.Mockito; + +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.Mockito.when; + +public class SlingAuthenticatorOsgiTest { + + @Rule + public final OsgiContext context = new OsgiContext(); + + private Meter meter = mock(Meter.class); + private Timer.Context ctx = mock(Timer.Context.class); + private Timer timer = mock(Timer.class); + private final MetricsService metricsService = mock(MetricsService.class); + + private final SlingAuthenticator authenticator = new SlingAuthenticator(); + + @Before + public void before() throws Exception { + ResourceResolver rr = mock(ResourceResolver.class); + ResourceResolverFactory resourceResolverFactory = mock(ResourceResolverFactory.class); + when(resourceResolverFactory.getResourceResolver(any(AuthenticationInfo.class))).thenReturn(rr); + + when(timer.time()).thenReturn(ctx); + when(metricsService.meter(anyString())).thenReturn(meter); + when(metricsService.timer(anyString())).thenReturn(timer); + + context.registerService(ResourceResolverFactory.class, resourceResolverFactory); + context.registerService(MetricsService.class, metricsService); + context.registerInjectActivateService(authenticator); + } + + @Test + public void testHandleSecurity() { + HttpServletRequest req = mock(HttpServletRequest.class); + when(req.getServletPath()).thenReturn("/"); + when(req.getServerName()).thenReturn("localhost"); + when(req.getServerPort()).thenReturn(80); + when(req.getScheme()).thenReturn("http"); + when(req.getRequestURI()).thenReturn("http://localhost:80/";); + + HttpServletResponse resp = mock(HttpServletResponse.class); + authenticator.handleSecurity(req, resp); + + verify(timer).time(); + verify(ctx).stop(); + verify(meter).mark(); + verifyNoMoreInteractions(timer, meter, ctx); + } + +} \ No newline at end of file
