Merge pull request #5 from focus-shift/upstream_changes

align with upstream changes rev cd08e02
focus-shift · Nov 25, 2024 · 4214bba · 4214bba
2 parents 1d099be + 80a5d4d
commit 4214bba
Show file tree

Hide file tree

Showing 6 changed files with 128 additions and 27 deletions.
diff --git a/README.md b/README.md
@@ -382,6 +382,25 @@ keycloak_request_duration_count{code="200",method="GET",resource="admin,admin/se
 keycloak_request_duration_sum{code="200",method="GET",resource="admin,admin/serverinfo",uri="",} 19.0
 ```
 
+To replace `users` or `clients` UUID values by a generic `{id}` with ```URI_METRICS_DETAILED``` enabled,
+set ```URI_METRICS_UUID_HIDDEN``` to `true`
+
+```c
+# HELP keycloak_request_duration Request duration
+# TYPE keycloak_request_duration histogram
+keycloak_request_duration_bucket{code="200",method="GET",resource="admin,admin/realms",uri="admin/realms/master/users/{id}",le="50.0",} 6.0
+keycloak_request_duration_bucket{code="200",method="GET",resource="admin,admin/realms",uri="admin/realms/master/users/{id}",le="100.0",} 6.0
+keycloak_request_duration_bucket{code="200",method="GET",resource="admin,admin/realms",uri="admin/realms/master/users/{id}",le="250.0",} 6.0
+keycloak_request_duration_bucket{code="200",method="GET",resource="admin,admin/realms",uri="admin/realms/master/users/{id}",le="500.0",} 6.0
+keycloak_request_duration_bucket{code="200",method="GET",resource="admin,admin/realms",uri="admin/realms/master/users/{id}",le="1000.0",} 6.0
+keycloak_request_duration_bucket{code="200",method="GET",resource="admin,admin/realms",uri="admin/realms/master/users/{id}",le="2000.0",} 6.0
+keycloak_request_duration_bucket{code="200",method="GET",resource="admin,admin/realms",uri="admin/realms/master/users/{id}",le="10000.0",} 6.0
+keycloak_request_duration_bucket{code="200",method="GET",resource="admin,admin/realms",uri="admin/realms/master/users/{id}",le="30000.0",} 6.0
+keycloak_request_duration_bucket{code="200",method="GET",resource="admin,admin/realms",uri="admin/realms/master/users/{id}",le="+Inf",} 6.0
+keycloak_request_duration_count{code="200",method="GET",resource="admin,admin/realms",uri="admin/realms/master/users/{id}",} 6.0
+keycloak_request_duration_sum{code="200",method="GET",resource="admin,admin/realms",uri="admin/realms/master/users/{id}",} 41.0
+```
+
 ## External Access
 
 To disable metrics being externally accessible to a cluster. Set the environment variable 'DISABLE_EXTERNAL_ACCESS'. Once set enable the header 'X-Forwarded-Host' on your proxy. This is enabled by default on HA Proxy on Openshift.

diff --git a/src/main/java/org/jboss/aerogear/keycloak/metrics/MetricsEndpointFactory.java b/src/main/java/org/jboss/aerogear/keycloak/metrics/MetricsEndpointFactory.java
@@ -18,7 +18,6 @@ public void init(Config.Scope config) {
         // nothing to do
     }
 
-
     @Override
     public void postInit(KeycloakSessionFactory factory) {
         // nothing to do

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/main/java/org/jboss/aerogear/keycloak/metrics/ResourceExtractor.java b/src/main/java/org/jboss/aerogear/keycloak/metrics/ResourceExtractor.java
@@ -13,6 +13,7 @@ class ResourceExtractor {
     private static final boolean URI_METRICS_ENABLED = Boolean.parseBoolean(System.getenv("URI_METRICS_ENABLED"));
     private static final boolean URI_METRICS_DETAILED = Boolean.parseBoolean(System.getenv("URI_METRICS_DETAILED"));
     private static final String URI_METRICS_FILTER = System.getenv("URI_METRICS_FILTER");
+    private static final boolean URI_METRICS_UUID_HIDDEN = Boolean.parseBoolean(System.getenv("URI_METRICS_UUID_HIDDEN"));
 
     private ResourceExtractor() {
     }
@@ -65,21 +66,23 @@ static String getResource(UriInfo uriInfo) {
     static String getURI(UriInfo uriInfo) {
         if (URI_METRICS_ENABLED) {
             List<String> matchedURIs = uriInfo.getMatchedURIs();
-            StringBuilder sb = new StringBuilder();
+            if (!matchedURIs.isEmpty()) {
+                StringBuilder sb = new StringBuilder();
 
-            if (URI_METRICS_FILTER != null && URI_METRICS_FILTER.length() != 0) {
-                String[] filter = URI_METRICS_FILTER.split(",");
+                if (URI_METRICS_FILTER != null && URI_METRICS_FILTER.length() != 0) {
+                    String[] filter = URI_METRICS_FILTER.split(",");
 
-                for (int i = 0; i < filter.length; i++) {
-                    if (matchedURIs.get(0).contains(filter[i])) {
+                    for (int i = 0; i < filter.length; i++) {
+                        if (matchedURIs.get(0).contains(filter[i])) {
 
-                        sb = getURIDetailed(sb, matchedURIs);
+                            sb = getURIDetailed(sb, matchedURIs);
+                        }
                     }
+                } else {
+                    sb = getURIDetailed(sb, matchedURIs);
                 }
-            } else {
-                sb = getURIDetailed(sb, matchedURIs);
+                return sb.toString();
             }
-            return sb.toString();
         }
         return "";
     }
@@ -89,13 +92,18 @@ private static StringBuilder getURIDetailed(StringBuilder sb, List<String> match
         String uri = matchedURIs.get(0);
 
         if (URI_METRICS_DETAILED) {
-            sb.append(uri);
+            if (URI_METRICS_UUID_HIDDEN) {
+                uri = uri.replaceAll("\\w{8}-\\w{4}-\\w{4}-\\w{4}-\\w{12}", "{id}");
+                sb.append(uri);
+            } else {
+                sb.append(uri);
+            }
         } else {
             String[] realm = uri.split("/");
             if (realm.length != 1) {
                 if (uri.startsWith("admin/realms/")) {
                     uri = uri.replace(realm[2], "{realm}");
-                    if (realm.length > 4 && realm[3].equals("clients")) {
+                    if (realm.length > 4 && (realm[3].equals("clients") || realm[3].equals("users"))) {
                         uri = uri.replace(realm[4], "{id}");
                     }
                 }

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);
     }