review

Author: adutra

quarkus/service/src/main/java/org/apache/polaris/service/quarkus/metrics/QuarkusMetricsConfiguration.java

@@ -41,7 +41,7 @@ interface RealmIdTag {
      * even crash the server.
      */
     @WithDefault("false")
-    boolean apiMetricsEnabled();
+    boolean enableInApiMetrics();
 
     /**
      * Whether to include the Realm ID tag in the HTTP server request metrics.
@@ -50,7 +50,7 @@ interface RealmIdTag {
      * even crash the server.
      */
     @WithDefault("false")
-    boolean httpMetricsEnabled();
+    boolean enableInHttpMetrics();
 
     /**
      * The maximum number of Realm ID tag values allowed for the HTTP server request metrics.

quarkus/service/src/main/java/org/apache/polaris/service/quarkus/metrics/QuarkusValueExpressionResolver.java

@@ -33,7 +33,7 @@ public class QuarkusValueExpressionResolver implements ValueExpressionResolver {
   @Override
   public String resolve(@Nonnull String expression, @Nullable Object parameter) {
     // TODO maybe replace with CEL of some expression engine and make this more generic
-    if (metricsConfiguration.realmIdTag().apiMetricsEnabled()
+    if (metricsConfiguration.realmIdTag().enableInApiMetrics()
         && parameter instanceof RealmContext realmContext
         && expression.equals("realmIdentifier")) {
       return realmContext.getRealmIdentifier();

quarkus/service/src/main/java/org/apache/polaris/service/quarkus/metrics/RealmIdTagContributor.java

@@ -40,7 +40,7 @@ public class RealmIdTagContributor implements HttpServerMetricsTagsContributor {
   public Tags contribute(Context context) {
     // FIXME retrieve the realm context from context.requestContextLocalData()
     // after upgrading to Quarkus 3.24
-    if (!metricsConfiguration.realmIdTag().httpMetricsEnabled()) {
+    if (!metricsConfiguration.realmIdTag().enableInHttpMetrics()) {
       return Tags.empty();
     }
     HttpServerRequest request = context.request();

quarkus/service/src/test/java/org/apache/polaris/service/quarkus/metrics/MetricsTestBase.java

@@ -39,6 +39,8 @@
 import org.junit.jupiter.api.TestInfo;
 import org.junit.jupiter.api.TestInstance;
 import org.junit.jupiter.api.extension.ExtendWith;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
 
 @TestInstance(TestInstance.Lifecycle.PER_CLASS)
 @ExtendWith(TestEnvironmentExtension.class)
@@ -51,6 +53,7 @@ public abstract class MetricsTestBase {
 
   @Inject PolarisIntegrationTestHelper helper;
   @Inject MeterRegistry registry;
+  @Inject QuarkusMetricsConfiguration metricsConfiguration;
 
   private TestEnvironment testEnv;
   private PolarisIntegrationTestFixture fixture;
@@ -66,7 +69,9 @@ public void clearMetrics() {
     registry.clear();
   }
 
-  protected void testMetricsEmittedOnSuccessfulRequest(String endpoint, boolean realmIdTagEnabled) {
+  @ParameterizedTest
+  @ValueSource(strings = {"%s/metrics", "%s/q/metrics"})
+  public void testMetricsEmittedOnSuccessfulRequest(String endpoint) {
     sendSuccessfulRequest();
     Map<String, MetricFamily> allMetrics =
         TestMetricsUtil.fetchMetrics(fixture.client, testEnv.baseManagementUri(), endpoint);
@@ -78,7 +83,11 @@ protected void testMetricsEmittedOnSuccessfulRequest(String endpoint, boolean re
                   .contains(
                       Map.entry("application", "Polaris"),
                       Map.entry("environment", "prod"),
-                      Map.entry("realm_id", realmIdTagEnabled ? fixture.realm : ""),
+                      Map.entry(
+                          "realm_id",
+                          metricsConfiguration.realmIdTag().enableInApiMetrics()
+                              ? fixture.realm
+                              : ""),
                       Map.entry(
                           "class", "org.apache.polaris.service.admin.api.PolarisPrincipalsApi"),
                       Map.entry("exception", "none"),
@@ -100,7 +109,7 @@ protected void testMetricsEmittedOnSuccessfulRequest(String endpoint, boolean re
                       Map.entry("outcome", "SUCCESS"),
                       Map.entry("status", "200"),
                       Map.entry("uri", "/api/management/v1/principals/{principalName}"));
-              if (realmIdTagEnabled) {
+              if (metricsConfiguration.realmIdTag().enableInHttpMetrics()) {
                 assertThat(metric.getLabels()).containsEntry("realm_id", fixture.realm);
               } else {
                 assertThat(metric.getLabels()).doesNotContainKey("realm_id");
@@ -112,7 +121,9 @@ protected void testMetricsEmittedOnSuccessfulRequest(String endpoint, boolean re
             });
   }
 
-  protected void testMetricsEmittedOnFailedRequest(String endpoint, boolean realmIdTagEnabled) {
+  @ParameterizedTest
+  @ValueSource(strings = {"%s/metrics", "%s/q/metrics"})
+  public void testMetricsEmittedOnFailedRequest(String endpoint) {
     sendFailingRequest();
     Map<String, MetricFamily> allMetrics =
         TestMetricsUtil.fetchMetrics(fixture.client, testEnv.baseManagementUri(), endpoint);
@@ -124,7 +135,11 @@ protected void testMetricsEmittedOnFailedRequest(String endpoint, boolean realmI
                   .contains(
                       Map.entry("application", "Polaris"),
                       Map.entry("environment", "prod"),
-                      Map.entry("realm_id", realmIdTagEnabled ? fixture.realm : ""),
+                      Map.entry(
+                          "realm_id",
+                          metricsConfiguration.realmIdTag().enableInApiMetrics()
+                              ? fixture.realm
+                              : ""),
                       Map.entry(
                           "class", "org.apache.polaris.service.admin.api.PolarisPrincipalsApi"),
                       Map.entry("exception", "NotFoundException"),
@@ -145,7 +160,7 @@ protected void testMetricsEmittedOnFailedRequest(String endpoint, boolean realmI
                       Map.entry("outcome", "CLIENT_ERROR"),
                       Map.entry("status", "404"),
                       Map.entry("uri", "/api/management/v1/principals/{principalName}"));
-              if (realmIdTagEnabled) {
+              if (metricsConfiguration.realmIdTag().enableInHttpMetrics()) {
                 assertThat(metric.getLabels()).containsEntry("realm_id", fixture.realm);
               } else {
                 assertThat(metric.getLabels()).doesNotContainKey("realm_id");

quarkus/service/src/test/java/org/apache/polaris/service/quarkus/metrics/RealmIdTagDisabledMetricsTest.java

@@ -22,8 +22,6 @@
 import io.quarkus.test.junit.QuarkusTestProfile;
 import io.quarkus.test.junit.TestProfile;
 import java.util.Map;
-import org.junit.jupiter.params.ParameterizedTest;
-import org.junit.jupiter.params.provider.ValueSource;
 
 @QuarkusTest
 @TestProfile(RealmIdTagDisabledMetricsTest.Profile.class)
@@ -37,16 +35,4 @@ public Map<String, String> getConfigOverrides() {
           "polaris.metrics.tags.environment", "prod", "polaris.realm-context.type", "test");
     }
   }
-
-  @ParameterizedTest
-  @ValueSource(strings = {"%s/metrics", "%s/q/metrics"})
-  public void testMetricsEmittedOnSuccessfulRequest(String endpoint) {
-    super.testMetricsEmittedOnSuccessfulRequest(endpoint, false);
-  }
-
-  @ParameterizedTest
-  @ValueSource(strings = {"%s/metrics", "%s/q/metrics"})
-  public void testMetricsEmittedOnFailedRequest(String endpoint) {
-    super.testMetricsEmittedOnFailedRequest(endpoint, false);
-  }
 }

quarkus/service/src/test/java/org/apache/polaris/service/quarkus/metrics/RealmIdTagEnabledMetricsTest.java

@@ -22,8 +22,6 @@
 import io.quarkus.test.junit.QuarkusTestProfile;
 import io.quarkus.test.junit.TestProfile;
 import java.util.Map;
-import org.junit.jupiter.params.ParameterizedTest;
-import org.junit.jupiter.params.provider.ValueSource;
 
 @QuarkusTest
 @TestProfile(RealmIdTagEnabledMetricsTest.Profile.class)
@@ -38,22 +36,10 @@ public Map<String, String> getConfigOverrides() {
           "prod",
           "polaris.realm-context.type",
           "test",
-          "polaris.metrics.realm-id-tag.api-metrics-enabled",
+          "polaris.metrics.realm-id-tag.enable-in-api-metrics",
           "true",
-          "polaris.metrics.realm-id-tag.http-metrics-enabled",
+          "polaris.metrics.realm-id-tag.enable-in-http-metrics",
           "true");
     }
   }
-
-  @ParameterizedTest
-  @ValueSource(strings = {"%s/metrics", "%s/q/metrics"})
-  public void testMetricsEmittedOnSuccessfulRequest(String endpoint) {
-    super.testMetricsEmittedOnSuccessfulRequest(endpoint, true);
-  }
-
-  @ParameterizedTest
-  @ValueSource(strings = {"%s/metrics", "%s/q/metrics"})
-  public void testMetricsEmittedOnFailedRequest(String endpoint) {
-    super.testMetricsEmittedOnFailedRequest(endpoint, true);
-  }
 }

quarkus/service/src/test/java/org/apache/polaris/service/quarkus/ratelimiter/RateLimiterFilterTest.java

@@ -77,8 +77,8 @@ public Map<String, String> getConfigOverrides() {
               String.valueOf(REQUESTS_PER_SECOND))
           .put("polaris.rate-limiter.token-bucket.window", WINDOW.toString())
           .put("polaris.metrics.tags.environment", "prod")
-          .put("polaris.metrics.realm-id-tag.api-metrics-enabled", "true")
-          .put("polaris.metrics.realm-id-tag.http-metrics-enabled", "true")
+          .put("polaris.metrics.realm-id-tag.enable-in-api-metrics", "true")
+          .put("polaris.metrics.realm-id-tag.enable-in-http-metrics", "true")
           .put("polaris.realm-context.type", "test")
           .put("polaris.authentication.token-broker.type", "symmetric-key")
           .put("polaris.authentication.token-broker.symmetric-key.secret", "secret")