adutra commented on code in PR #2445:
URL: https://github.com/apache/polaris/pull/2445#discussion_r2301454707
##########
runtime/service/src/main/java/org/apache/polaris/service/config/ProductionReadinessChecks.java:
##########
@@ -113,6 +114,17 @@ public void warnOnFailedChecks(
}
}
+ @Produces
+ public ProductionReadinessCheck checkMetricTags(MetricsConfiguration config)
{
+ if (config.userPrincipalTag().enableInApiMetrics()) {
Review Comment:
I think we are missing a readiness check when _both_ tags are enabled, since
this is the most dangerous situation.
##########
runtime/service/src/test/java/org/apache/polaris/service/metrics/RealmIdTagEnabledMetricsTest.java:
##########
@@ -39,7 +39,9 @@ public Map<String, String> getConfigOverrides() {
"polaris.metrics.realm-id-tag.enable-in-api-metrics",
"true",
"polaris.metrics.realm-id-tag.enable-in-http-metrics",
- "true");
+ "true",
Review Comment:
At some point I think we'll need to refactor these tests and create a sort
of "parameterized Quarkus test" that tests all the combinations of (realm tag
on/off) x (principal tag on/off). But we can look into that later.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@polaris.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
