Merge pull request #207 from bionarkos/bugfix/AEROGEAR-10381-http-met…

…ric-uri-label-ignored-for-404 AEROGEAR-10381: in case of 404 or redirection response do not record the uri
aerogear · Jul 8, 2024 · a413d3c · a413d3c
2 parents ac6ec32 + 344dda7
commit a413d3c
Show file tree

Hide file tree

Showing 3 changed files with 90 additions and 15 deletions.
diff --git a/src/main/java/org/jboss/aerogear/keycloak/metrics/MetricsFilter.java b/src/main/java/org/jboss/aerogear/keycloak/metrics/MetricsFilter.java
@@ -12,6 +12,7 @@
 import java.util.Set;
 
 public final class MetricsFilter implements ContainerRequestFilter, ContainerResponseFilter {
+
     private static final Logger LOG = Logger.getLogger(MetricsFilter.class);
 
     private static final String METRICS_REQUEST_TIMESTAMP = "metrics.requestTimestamp";
@@ -21,6 +22,8 @@ public final class MetricsFilter implements ContainerRequestFilter, ContainerRes
 
     // relevant response content types to be measured
     private static final Set<MediaType> contentTypes = new HashSet<>();
+    private static final String REDIRECTION_URI = "REDIRECTION";
+    private static final String NOT_FOUND_URI = "NOT_FOUND";
 
     static {
         contentTypes.add(MediaType.APPLICATION_JSON_TYPE);
@@ -48,6 +51,11 @@ public void filter(ContainerRequestContext req, ContainerResponseContext res) {
 
         String resource = ResourceExtractor.getResource(req.getUriInfo());
         String uri = ResourceExtractor.getURI(req.getUriInfo());
+        if (status >= 300 && status < 400) {
+            uri = REDIRECTION_URI;
+        } else if (status == 404) {
+            uri = NOT_FOUND_URI;
+        }
 
         if (URI_METRICS_ENABLED) {
             PrometheusExporter.instance().recordResponseTotal(status, req.getMethod(), resource, uri);

diff --git a/src/test/java/org/jboss/aerogear/keycloak/metrics/MetricsFilterTest.java b/src/test/java/org/jboss/aerogear/keycloak/metrics/MetricsFilterTest.java
@@ -0,0 +1,75 @@
+package org.jboss.aerogear.keycloak.metrics;
+
+import io.prometheus.client.CollectorRegistry;
+import jakarta.ws.rs.container.ContainerRequestContext;
+import jakarta.ws.rs.container.ContainerResponseContext;
+import jakarta.ws.rs.core.UriInfo;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import uk.org.webcompere.systemstubs.rules.EnvironmentVariablesRule;
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.lang.reflect.Field;
+import java.util.List;
+
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+public class MetricsFilterTest {
+
+  private MetricsFilter metricsFilter;
+
+  @Rule
+  public final EnvironmentVariablesRule environmentVariables = new EnvironmentVariablesRule();
+
+  @Before
+  public void resetSingleton() throws SecurityException, NoSuchFieldException, IllegalArgumentException, IllegalAccessException {
+    environmentVariables.set("URI_METRICS_ENABLED", "true");
+    metricsFilter = MetricsFilter.instance();
+
+    Field instance = PrometheusExporter.class.getDeclaredField("INSTANCE");
+    instance.setAccessible(true);
+    instance.set(null, null);
+    CollectorRegistry.defaultRegistry.clear();
+  }
+
+  @Test
+  public void testHttpMetricForNotFoundUri() throws IOException {
+    var req = mockRequest("GET", List.of("auth", "realms", "not_existing_realm", "openid-connect", "token"));
+
+    var resp = mock(ContainerResponseContext.class);
+    when(resp.getStatus()).thenReturn(404);
+
+    metricsFilter.filter(req, resp);
+
+    ByteArrayOutputStream stream = new ByteArrayOutputStream();
+    PrometheusExporter.instance().export(stream);
+    assertThat(stream.toString(), containsString("keycloak_response_created{code=\"404\",method=\"GET\",resource=\"token,openid-connect\",uri=\"NOT_FOUND\""));
+  }
+
+  @Test
+  public void testHttpMetricForRedirectUri() throws IOException {
+    var req = mockRequest("GET", List.of("auth", "realms", "my_realm", "openid-connect", "login"));
+
+    var resp = mock(ContainerResponseContext.class);
+    when(resp.getStatus()).thenReturn(302);
+
+    metricsFilter.filter(req, resp);
+
+    ByteArrayOutputStream stream = new ByteArrayOutputStream();
+    PrometheusExporter.instance().export(stream);
+    assertThat(stream.toString(), containsString("keycloak_response_created{code=\"302\",method=\"GET\",resource=\"login,openid-connect\",uri=\"REDIRECTION\""));
+  }
+
+  private static ContainerRequestContext mockRequest(String method, List<String> matchedUri) {
+    var req = mock(ContainerRequestContext.class);
+    when(req.getMethod()).thenReturn(method);
+    UriInfo uriInfo = mock(UriInfo.class);
+    when(uriInfo.getMatchedURIs()).thenReturn(matchedUri);
+    when(req.getUriInfo()).thenReturn(uriInfo);
+    return req;
+  }
+}
diff --git a/src/test/java/org/jboss/aerogear/keycloak/metrics/PrometheusExporterTest.java b/src/test/java/org/jboss/aerogear/keycloak/metrics/PrometheusExporterTest.java
@@ -37,10 +37,10 @@ public class PrometheusExporterTest {
     public void setupRealmProvider() {
         RealmModel realm = mock(RealmModel.class);
         when(realm.getName()).thenReturn(DEFAULT_REALM_NAME);
-        when(realmProvider.getRealm(eq(DEFAULT_REALM_ID))).thenReturn(realm);
+        when(realmProvider.getRealm(DEFAULT_REALM_ID)).thenReturn(realm);
         RealmModel otherRealm = mock(RealmModel.class);
         when(otherRealm.getName()).thenReturn("OTHER_REALM");
-        when(realmProvider.getRealm(eq("OTHER_REALM_ID"))).thenReturn(otherRealm);
+        when(realmProvider.getRealm("OTHER_REALM_ID")).thenReturn(otherRealm);
     }
 
     @Rule
@@ -319,15 +319,15 @@ public void shouldTolerateNullLabels() throws IOException {
     }
 
     @Test
-    public void shouldBuildPushgateway() throws IOException {
+    public void shouldBuildPushgateway() {
         final String envVar = "PROMETHEUS_PUSHGATEWAY_ADDRESS";
         final String address = "localhost:9091";
         environmentVariables.set(envVar, address);
         Assert.assertNotNull(PrometheusExporter.instance().PUSH_GATEWAY);
     }
 
     @Test
-    public void shouldBuildPushgatewayWithBasicAuth() throws IOException {
+    public void shouldBuildPushgatewayWithBasicAuth() {
         final String envVarAddress = "PROMETHEUS_PUSHGATEWAY_ADDRESS";
         final String address = "localhost:9091";
         environmentVariables.set(envVarAddress, address);
@@ -341,15 +341,15 @@ public void shouldBuildPushgatewayWithBasicAuth() throws IOException {
     }
 
     @Test
-    public void shouldBuildPushgatewayWithHttps() throws IOException {
+    public void shouldBuildPushgatewayWithHttps() {
         final String envVar = "PROMETHEUS_PUSHGATEWAY_ADDRESS";
         final String address = "https://localhost:9091";
         environmentVariables.set(envVar, address);
         Assert.assertNotNull(PrometheusExporter.instance().PUSH_GATEWAY);
     }
 
     @Test
-    public void shouldNotBuildPushgateway() throws IOException {
+    public void shouldNotBuildPushgateway() {
         Assert.assertNull(PrometheusExporter.instance().PUSH_GATEWAY);
     }
 
@@ -375,7 +375,7 @@ private void assertGenericMetric(String metricName, double metricValue, Tuple<St
     private void assertMetric(String metricName, double metricValue, String realm, Tuple<String, String>... labels) throws IOException {
         try (ByteArrayOutputStream stream = new ByteArrayOutputStream()) {
             PrometheusExporter.instance().export(stream);
-            String result = new String(stream.toByteArray());
+            String result = stream.toString();
 
             final StringBuilder builder = new StringBuilder();
 
@@ -416,18 +416,10 @@ private Event createEvent(EventType type, String realm, String clientId, String
         return event;
     }
 
-    private Event createEvent(EventType type, Tuple<String, String>... tuples) {
-        return this.createEvent(type, DEFAULT_REALM_ID, "THE_CLIENT_ID", (String) null, tuples);
-    }
-
     private Event createEvent(EventType type, String realm, String clientId, Tuple<String, String>... tuples) {
         return this.createEvent(type, realm, clientId, (String) null, tuples);
     }
 
-    private Event createEvent(EventType type, String realm, Tuple<String, String>... tuples) {
-        return this.createEvent(type, realm, "THE_CLIENT_ID", (String) null, tuples);
-    }
-
     private Event createEvent(EventType type) {
         return createEvent(type, DEFAULT_REALM_ID, "THE_CLIENT_ID",(String) null);
     }