From eec30dbc4e149e1dd180f3bd42a2c9eb23b8a5e5 Mon Sep 17 00:00:00 2001 From: Kostas Zoumpatianos Date: Mon, 25 Aug 2025 10:18:57 +0530 Subject: [PATCH 1/7] Added API change to enable tag --- .../service/metrics/MetricsConfiguration.java | 16 ++++++++++++++++ .../metrics/PolarisValueExpressionResolver.java | 8 ++++++++ server-templates/api.mustache | 2 +- 3 files changed, 25 insertions(+), 1 deletion(-) diff --git a/runtime/service/src/main/java/org/apache/polaris/service/metrics/MetricsConfiguration.java b/runtime/service/src/main/java/org/apache/polaris/service/metrics/MetricsConfiguration.java index 504895158a..ed0be47fd5 100644 --- a/runtime/service/src/main/java/org/apache/polaris/service/metrics/MetricsConfiguration.java +++ b/runtime/service/src/main/java/org/apache/polaris/service/metrics/MetricsConfiguration.java @@ -32,6 +32,9 @@ public interface MetricsConfiguration { /** Configuration for the Realm ID metric tag. */ RealmIdTag realmIdTag(); + /** Configuration for the user principal metric tag. */ + UserPrincipalTag userPrincipalTag(); + interface RealmIdTag { /** @@ -65,4 +68,17 @@ interface RealmIdTag { @Min(1) int httpMetricsMaxCardinality(); } + + interface UserPrincipalTag { + + /** + * Whether to include the User Principal tag in the API request metrics. + * + *

Beware that if the cardinality of this tag is too high, it can cause performance issues or + * even crash the server. + */ + @WithDefault("false") + boolean enableInApiMetrics(); + + } } diff --git a/runtime/service/src/main/java/org/apache/polaris/service/metrics/PolarisValueExpressionResolver.java b/runtime/service/src/main/java/org/apache/polaris/service/metrics/PolarisValueExpressionResolver.java index 2c71427702..8dbadafe58 100644 --- a/runtime/service/src/main/java/org/apache/polaris/service/metrics/PolarisValueExpressionResolver.java +++ b/runtime/service/src/main/java/org/apache/polaris/service/metrics/PolarisValueExpressionResolver.java @@ -23,6 +23,7 @@ import jakarta.annotation.Nonnull; import jakarta.enterprise.context.ApplicationScoped; import jakarta.inject.Inject; +import jakarta.ws.rs.core.SecurityContext; import org.apache.polaris.core.context.RealmContext; @ApplicationScoped @@ -39,6 +40,13 @@ public String resolve(@Nonnull String expression, @Nullable Object parameter) { && expression.equals("realmIdentifier")) { return realmContext.getRealmIdentifier(); } + + if (metricsConfiguration.userPrincipalTag().enableInApiMetrics() && + parameter instanceof SecurityContext securityContext + && expression.equals("userPrincipal") && securityContext.getUserPrincipal() != null) { + return securityContext.getUserPrincipal().getName(); + } return null; } + } diff --git a/server-templates/api.mustache b/server-templates/api.mustache index 8928bc356e..6d42e277f7 100644 --- a/server-templates/api.mustache +++ b/server-templates/api.mustache @@ -93,7 +93,7 @@ public class {{classname}} { {{#authMethods}}{{#isOAuth}}@RolesAllowed("**"){{/isOAuth}}{{/authMethods}}{{/hasAuthMethods}} @Timed("{{metricsPrefix}}.{{baseName}}.{{nickname}}") @Timeout - public Response {{nickname}}({{#isMultipart}}MultipartFormDataInput input,{{/isMultipart}}{{#allParams}}{{>queryParams}}{{>pathParams}}{{>headerParams}}{{>bodyParams}}{{^isMultipart}}{{>formParams}},{{/isMultipart}}{{#isMultipart}}{{^isFormParam}},{{/isFormParam}}{{/isMultipart}}{{/allParams}}@Context @MeterTag(key="realm_id",expression="realmIdentifier") RealmContext realmContext,@Context SecurityContext securityContext) { + public Response {{nickname}}({{#isMultipart}}MultipartFormDataInput input,{{/isMultipart}}{{#allParams}}{{>queryParams}}{{>pathParams}}{{>headerParams}}{{>bodyParams}}{{^isMultipart}}{{>formParams}},{{/isMultipart}}{{#isMultipart}}{{^isFormParam}},{{/isFormParam}}{{/isMultipart}}{{/allParams}}@Context @MeterTag(key="realm_id",expression="realmIdentifier") RealmContext realmContext,@Context @MeterTag(key="principal",expression="userPrincipal") SecurityContext securityContext) { {{! Don't log form or header params in case there are secrets, e.g., OAuth tokens }} LOGGER.atDebug().setMessage("Invoking {{baseName}} with params") .addKeyValue("operation", "{{nickname}}"){{#allParams}}{{^isHeaderParam}}{{^isFormParam}} From 4580c0b88e560a15f1aa1688670c24e93a58c71b Mon Sep 17 00:00:00 2001 From: Kostas Zoumpatianos Date: Mon, 25 Aug 2025 11:10:18 +0530 Subject: [PATCH 2/7] Added test --- .../service/metrics/MetricsTestBase.java | 10 +++++ .../metrics/RealmIdTagEnabledMetricsTest.java | 2 + .../UserPrincipalTagDisabledMetricsTest.java | 38 ++++++++++++++++ .../UserPrincipalTagEnabledMetricsTest.java | 45 +++++++++++++++++++ 4 files changed, 95 insertions(+) create mode 100644 runtime/service/src/test/java/org/apache/polaris/service/metrics/UserPrincipalTagDisabledMetricsTest.java create mode 100644 runtime/service/src/test/java/org/apache/polaris/service/metrics/UserPrincipalTagEnabledMetricsTest.java diff --git a/runtime/service/src/test/java/org/apache/polaris/service/metrics/MetricsTestBase.java b/runtime/service/src/test/java/org/apache/polaris/service/metrics/MetricsTestBase.java index 34722731e7..d0d9636e81 100644 --- a/runtime/service/src/test/java/org/apache/polaris/service/metrics/MetricsTestBase.java +++ b/runtime/service/src/test/java/org/apache/polaris/service/metrics/MetricsTestBase.java @@ -105,6 +105,11 @@ public void testMetricsEmittedOnSuccessfulRequest(String endpoint) { metricsConfiguration.realmIdTag().enableInApiMetrics() ? fixture.realm : ""), + Map.entry( + "principal", + metricsConfiguration.userPrincipalTag().enableInApiMetrics() + ? "root" + : ""), Map.entry( "class", "org.apache.polaris.service.admin.api.PolarisPrincipalsApi"), Map.entry("exception", "none"), @@ -156,6 +161,11 @@ public void testMetricsEmittedOnFailedRequest(String endpoint) { metricsConfiguration.realmIdTag().enableInApiMetrics() ? fixture.realm : ""), + Map.entry( + "principal", + metricsConfiguration.userPrincipalTag().enableInApiMetrics() + ? "root" + : ""), Map.entry( "class", "org.apache.polaris.service.admin.api.PolarisPrincipalsApi"), Map.entry("exception", "NotFoundException"), diff --git a/runtime/service/src/test/java/org/apache/polaris/service/metrics/RealmIdTagEnabledMetricsTest.java b/runtime/service/src/test/java/org/apache/polaris/service/metrics/RealmIdTagEnabledMetricsTest.java index 7235dccd18..a9a6321650 100644 --- a/runtime/service/src/test/java/org/apache/polaris/service/metrics/RealmIdTagEnabledMetricsTest.java +++ b/runtime/service/src/test/java/org/apache/polaris/service/metrics/RealmIdTagEnabledMetricsTest.java @@ -39,6 +39,8 @@ public Map getConfigOverrides() { "polaris.metrics.realm-id-tag.enable-in-api-metrics", "true", "polaris.metrics.realm-id-tag.enable-in-http-metrics", + "true", + "polaris.metrics.user-principal-tag.enable-in-api-metrics", "true"); } } diff --git a/runtime/service/src/test/java/org/apache/polaris/service/metrics/UserPrincipalTagDisabledMetricsTest.java b/runtime/service/src/test/java/org/apache/polaris/service/metrics/UserPrincipalTagDisabledMetricsTest.java new file mode 100644 index 0000000000..fd8feecb35 --- /dev/null +++ b/runtime/service/src/test/java/org/apache/polaris/service/metrics/UserPrincipalTagDisabledMetricsTest.java @@ -0,0 +1,38 @@ +/* + * 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.polaris.service.metrics; + +import io.quarkus.test.junit.QuarkusTest; +import io.quarkus.test.junit.QuarkusTestProfile; +import io.quarkus.test.junit.TestProfile; +import java.util.Map; + +@QuarkusTest +@TestProfile(UserPrincipalTagDisabledMetricsTest.Profile.class) +public class UserPrincipalTagDisabledMetricsTest extends MetricsTestBase { + + public static class Profile implements QuarkusTestProfile { + + @Override + public Map getConfigOverrides() { + return Map.of( + "polaris.metrics.tags.environment", "prod", "polaris.realm-context.type", "test"); + } + } +} diff --git a/runtime/service/src/test/java/org/apache/polaris/service/metrics/UserPrincipalTagEnabledMetricsTest.java b/runtime/service/src/test/java/org/apache/polaris/service/metrics/UserPrincipalTagEnabledMetricsTest.java new file mode 100644 index 0000000000..759acf2236 --- /dev/null +++ b/runtime/service/src/test/java/org/apache/polaris/service/metrics/UserPrincipalTagEnabledMetricsTest.java @@ -0,0 +1,45 @@ +/* + * 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.polaris.service.metrics; + +import io.quarkus.test.junit.QuarkusTest; +import io.quarkus.test.junit.QuarkusTestProfile; +import io.quarkus.test.junit.TestProfile; +import java.util.Map; + +@QuarkusTest +@TestProfile(UserPrincipalTagEnabledMetricsTest.Profile.class) +public class UserPrincipalTagEnabledMetricsTest extends MetricsTestBase { + + public static class Profile implements QuarkusTestProfile { + + @Override + public Map getConfigOverrides() { + return Map.of( + "polaris.metrics.tags.environment", + "prod", + "polaris.metrics.user-principal-tag.enable-in-api-metrics", + "true", + "polaris.metrics.realm-id-tag.enable-in-api-metrics", + "false", + "polaris.metrics.realm-id-tag.enable-in-http-metrics", + "false"); + } + } +} From 9905344a08f80bcec5c04d34cc61128e38759329 Mon Sep 17 00:00:00 2001 From: Kostas Zoumpatianos Date: Mon, 25 Aug 2025 11:45:19 +0530 Subject: [PATCH 3/7] Fix --- .../polaris/service/metrics/RealmIdTagEnabledMetricsTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/runtime/service/src/test/java/org/apache/polaris/service/metrics/RealmIdTagEnabledMetricsTest.java b/runtime/service/src/test/java/org/apache/polaris/service/metrics/RealmIdTagEnabledMetricsTest.java index a9a6321650..86ddf1fb99 100644 --- a/runtime/service/src/test/java/org/apache/polaris/service/metrics/RealmIdTagEnabledMetricsTest.java +++ b/runtime/service/src/test/java/org/apache/polaris/service/metrics/RealmIdTagEnabledMetricsTest.java @@ -41,7 +41,7 @@ public Map getConfigOverrides() { "polaris.metrics.realm-id-tag.enable-in-http-metrics", "true", "polaris.metrics.user-principal-tag.enable-in-api-metrics", - "true"); + "false"); } } } From 7702e78b2e145744a661ceb2fd51b68868bb80ed Mon Sep 17 00:00:00 2001 From: Kostas Zoumpatianos Date: Tue, 26 Aug 2025 09:59:47 +0530 Subject: [PATCH 4/7] Spottless apply --- .../service/metrics/MetricsConfiguration.java | 1 - .../PolarisValueExpressionResolver.java | 8 +++--- .../service/metrics/MetricsTestBase.java | 8 +++--- .../UserPrincipalTagDisabledMetricsTest.java | 12 ++++----- .../UserPrincipalTagEnabledMetricsTest.java | 26 +++++++++---------- 5 files changed, 27 insertions(+), 28 deletions(-) diff --git a/runtime/service/src/main/java/org/apache/polaris/service/metrics/MetricsConfiguration.java b/runtime/service/src/main/java/org/apache/polaris/service/metrics/MetricsConfiguration.java index ed0be47fd5..8b8c0a2475 100644 --- a/runtime/service/src/main/java/org/apache/polaris/service/metrics/MetricsConfiguration.java +++ b/runtime/service/src/main/java/org/apache/polaris/service/metrics/MetricsConfiguration.java @@ -79,6 +79,5 @@ interface UserPrincipalTag { */ @WithDefault("false") boolean enableInApiMetrics(); - } } diff --git a/runtime/service/src/main/java/org/apache/polaris/service/metrics/PolarisValueExpressionResolver.java b/runtime/service/src/main/java/org/apache/polaris/service/metrics/PolarisValueExpressionResolver.java index 8dbadafe58..19b8df4801 100644 --- a/runtime/service/src/main/java/org/apache/polaris/service/metrics/PolarisValueExpressionResolver.java +++ b/runtime/service/src/main/java/org/apache/polaris/service/metrics/PolarisValueExpressionResolver.java @@ -41,12 +41,12 @@ public String resolve(@Nonnull String expression, @Nullable Object parameter) { return realmContext.getRealmIdentifier(); } - if (metricsConfiguration.userPrincipalTag().enableInApiMetrics() && - parameter instanceof SecurityContext securityContext - && expression.equals("userPrincipal") && securityContext.getUserPrincipal() != null) { + if (metricsConfiguration.userPrincipalTag().enableInApiMetrics() + && parameter instanceof SecurityContext securityContext + && expression.equals("userPrincipal") + && securityContext.getUserPrincipal() != null) { return securityContext.getUserPrincipal().getName(); } return null; } - } diff --git a/runtime/service/src/test/java/org/apache/polaris/service/metrics/MetricsTestBase.java b/runtime/service/src/test/java/org/apache/polaris/service/metrics/MetricsTestBase.java index d0d9636e81..ebff8b97b0 100644 --- a/runtime/service/src/test/java/org/apache/polaris/service/metrics/MetricsTestBase.java +++ b/runtime/service/src/test/java/org/apache/polaris/service/metrics/MetricsTestBase.java @@ -106,7 +106,7 @@ public void testMetricsEmittedOnSuccessfulRequest(String endpoint) { ? fixture.realm : ""), Map.entry( - "principal", + "principal", metricsConfiguration.userPrincipalTag().enableInApiMetrics() ? "root" : ""), @@ -163,9 +163,9 @@ public void testMetricsEmittedOnFailedRequest(String endpoint) { : ""), Map.entry( "principal", - metricsConfiguration.userPrincipalTag().enableInApiMetrics() - ? "root" - : ""), + metricsConfiguration.userPrincipalTag().enableInApiMetrics() + ? "root" + : ""), Map.entry( "class", "org.apache.polaris.service.admin.api.PolarisPrincipalsApi"), Map.entry("exception", "NotFoundException"), diff --git a/runtime/service/src/test/java/org/apache/polaris/service/metrics/UserPrincipalTagDisabledMetricsTest.java b/runtime/service/src/test/java/org/apache/polaris/service/metrics/UserPrincipalTagDisabledMetricsTest.java index fd8feecb35..9f45bb51c4 100644 --- a/runtime/service/src/test/java/org/apache/polaris/service/metrics/UserPrincipalTagDisabledMetricsTest.java +++ b/runtime/service/src/test/java/org/apache/polaris/service/metrics/UserPrincipalTagDisabledMetricsTest.java @@ -27,12 +27,12 @@ @TestProfile(UserPrincipalTagDisabledMetricsTest.Profile.class) public class UserPrincipalTagDisabledMetricsTest extends MetricsTestBase { - public static class Profile implements QuarkusTestProfile { + public static class Profile implements QuarkusTestProfile { - @Override - public Map getConfigOverrides() { - return Map.of( - "polaris.metrics.tags.environment", "prod", "polaris.realm-context.type", "test"); - } + @Override + public Map getConfigOverrides() { + return Map.of( + "polaris.metrics.tags.environment", "prod", "polaris.realm-context.type", "test"); } + } } diff --git a/runtime/service/src/test/java/org/apache/polaris/service/metrics/UserPrincipalTagEnabledMetricsTest.java b/runtime/service/src/test/java/org/apache/polaris/service/metrics/UserPrincipalTagEnabledMetricsTest.java index 759acf2236..b7e706fea0 100644 --- a/runtime/service/src/test/java/org/apache/polaris/service/metrics/UserPrincipalTagEnabledMetricsTest.java +++ b/runtime/service/src/test/java/org/apache/polaris/service/metrics/UserPrincipalTagEnabledMetricsTest.java @@ -27,19 +27,19 @@ @TestProfile(UserPrincipalTagEnabledMetricsTest.Profile.class) public class UserPrincipalTagEnabledMetricsTest extends MetricsTestBase { - public static class Profile implements QuarkusTestProfile { + public static class Profile implements QuarkusTestProfile { - @Override - public Map getConfigOverrides() { - return Map.of( - "polaris.metrics.tags.environment", - "prod", - "polaris.metrics.user-principal-tag.enable-in-api-metrics", - "true", - "polaris.metrics.realm-id-tag.enable-in-api-metrics", - "false", - "polaris.metrics.realm-id-tag.enable-in-http-metrics", - "false"); - } + @Override + public Map getConfigOverrides() { + return Map.of( + "polaris.metrics.tags.environment", + "prod", + "polaris.metrics.user-principal-tag.enable-in-api-metrics", + "true", + "polaris.metrics.realm-id-tag.enable-in-api-metrics", + "false", + "polaris.metrics.realm-id-tag.enable-in-http-metrics", + "false"); } + } } From 1f17793999a2ba0600a124636f4c1d62c6d00b4d Mon Sep 17 00:00:00 2001 From: Kostas Zoumpatianos Date: Tue, 26 Aug 2025 14:47:13 +0530 Subject: [PATCH 5/7] Added production readiness check --- .../service/config/ProductionReadinessChecks.java | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/runtime/service/src/main/java/org/apache/polaris/service/config/ProductionReadinessChecks.java b/runtime/service/src/main/java/org/apache/polaris/service/config/ProductionReadinessChecks.java index 9f8ceb9a55..4dc2536ce7 100644 --- a/runtime/service/src/main/java/org/apache/polaris/service/config/ProductionReadinessChecks.java +++ b/runtime/service/src/main/java/org/apache/polaris/service/config/ProductionReadinessChecks.java @@ -44,6 +44,7 @@ import org.apache.polaris.service.context.TestRealmContextResolver; import org.apache.polaris.service.events.PolarisEventListener; import org.apache.polaris.service.events.TestPolarisEventListener; +import org.apache.polaris.service.metrics.MetricsConfiguration; import org.apache.polaris.service.persistence.InMemoryPolarisMetaStoreManagerFactory; import org.eclipse.microprofile.config.Config; import org.eclipse.microprofile.config.ConfigValue; @@ -113,6 +114,17 @@ public void warnOnFailedChecks( } } + @Produces + public ProductionReadinessCheck checkMetricTags(MetricsConfiguration config) { + if (config.userPrincipalTag().enableInApiMetrics()) { + return ProductionReadinessCheck.of( + Error.of( + "Metrics configuration includes user principal name in tags.", + "polaris.metrics.user-principal-tag.enable-in-api-metrics")); + } + return ProductionReadinessCheck.OK; + } + @Produces public ProductionReadinessCheck checkTokenBrokers(AuthenticationConfiguration configuration) { List errors = new ArrayList<>(); From b0e2cd79830a5df9a1b41c6cfa9d88a501b99b93 Mon Sep 17 00:00:00 2001 From: Kostas Zoumpatianos Date: Tue, 26 Aug 2025 23:20:27 +0530 Subject: [PATCH 6/7] Fixed production readiness tests --- .../service/config/ProductionReadinessChecks.java | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/runtime/service/src/main/java/org/apache/polaris/service/config/ProductionReadinessChecks.java b/runtime/service/src/main/java/org/apache/polaris/service/config/ProductionReadinessChecks.java index 4dc2536ce7..f2b8bddc19 100644 --- a/runtime/service/src/main/java/org/apache/polaris/service/config/ProductionReadinessChecks.java +++ b/runtime/service/src/main/java/org/apache/polaris/service/config/ProductionReadinessChecks.java @@ -116,11 +116,16 @@ public void warnOnFailedChecks( @Produces public ProductionReadinessCheck checkMetricTags(MetricsConfiguration config) { - if (config.userPrincipalTag().enableInApiMetrics()) { + if (config.userPrincipalTag().enableInApiMetrics() + && config.realmIdTag().enableInApiMetrics()) { return ProductionReadinessCheck.of( Error.of( - "Metrics configuration includes user principal name in tags.", + "Metrics configuration includes both user principal name and realm id in tags this could cause performance implications.", "polaris.metrics.user-principal-tag.enable-in-api-metrics")); + } else { + LOGGER.warn( + "Metrics configuration includes user principal name in tags. " + + "This could have performance implications."); } return ProductionReadinessCheck.OK; } From 8830a3dcd4fd114d43650b9db8538666bf32bd1f Mon Sep 17 00:00:00 2001 From: Kostas Zoumpatianos Date: Tue, 26 Aug 2025 23:26:45 +0530 Subject: [PATCH 7/7] Broken down production readiness tests --- .../config/ProductionReadinessChecks.java | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/runtime/service/src/main/java/org/apache/polaris/service/config/ProductionReadinessChecks.java b/runtime/service/src/main/java/org/apache/polaris/service/config/ProductionReadinessChecks.java index f2b8bddc19..ca1373ea5f 100644 --- a/runtime/service/src/main/java/org/apache/polaris/service/config/ProductionReadinessChecks.java +++ b/runtime/service/src/main/java/org/apache/polaris/service/config/ProductionReadinessChecks.java @@ -115,17 +115,25 @@ public void warnOnFailedChecks( } @Produces - public ProductionReadinessCheck checkMetricTags(MetricsConfiguration config) { + public ProductionReadinessCheck checkUserPrincipalMetricTag(MetricsConfiguration config) { + if (config.userPrincipalTag().enableInApiMetrics()) { + return ProductionReadinessCheck.of( + Error.of( + "Metrics configuration includes user principal name and this could have security implications.", + "polaris.metrics.user-principal-tag.enable-in-api-metrics")); + } + return ProductionReadinessCheck.OK; + } + + @Produces + public ProductionReadinessCheck checkUserPrincipalAndRealmIdMetricTags( + MetricsConfiguration config) { if (config.userPrincipalTag().enableInApiMetrics() && config.realmIdTag().enableInApiMetrics()) { return ProductionReadinessCheck.of( Error.of( - "Metrics configuration includes both user principal name and realm id in tags this could cause performance implications.", + "Metrics configuration includes both user principal name and realm id in tags and this could have performance implications.", "polaris.metrics.user-principal-tag.enable-in-api-metrics")); - } else { - LOGGER.warn( - "Metrics configuration includes user principal name in tags. " - + "This could have performance implications."); } return ProductionReadinessCheck.OK; }