Skip to content

Commit a3abce1

Browse files
adutraadutra
committed
feat(metrics)!: Mitigate potential performance issues with realm_id tag
BREAKING CHANGE: The realm_id metric tag is not emitted by default anymore. To retrieve the previous behavior, add
1 parent c969812 commit a3abce1

File tree

10 files changed

+317
-97
lines changed

10 files changed

+317
-97
lines changed

‎quarkus/service/src/main/java/org/apache/polaris/service/quarkus/metrics/QuarkusMeterFilterProducer.java‎

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,21 +20,42 @@
2020

2121
import io.micrometer.core.instrument.Tag;
2222
import io.micrometer.core.instrument.config.MeterFilter;
23+
import io.micrometer.core.instrument.internal.OnlyOnceLoggingDenyMeterFilter;
24+
import io.quarkus.micrometer.runtime.binder.HttpBinderConfiguration;
2325
import jakarta.enterprise.inject.Produces;
2426
import jakarta.inject.Inject;
2527
import jakarta.inject.Singleton;
2628
import java.util.stream.Collectors;
2729

2830
public class QuarkusMeterFilterProducer {
2931

30-
@Inject QuarkusMetricsConfiguration configuration;
32+
@Inject QuarkusMetricsConfiguration metricsConfiguration;
33+
@Inject HttpBinderConfiguration binderConfiguration;
3134

3235
@Produces
3336
@Singleton
34-
public MeterFilter produceGlobalMeterFilter() {
37+
public MeterFilter commonTagsFilter() {
3538
return MeterFilter.commonTags(
36-
this.configuration.tags().entrySet().stream()
39+
this.metricsConfiguration.tags().entrySet().stream()
3740
.map(e -> Tag.of(e.getKey(), e.getValue()))
3841
.collect(Collectors.toSet()));
3942
}
43+
44+
@Produces
45+
@Singleton
46+
public MeterFilter maxRealmIdTagsInHttpMetricsFilter() {
47+
MeterFilter denyFilter =
48+
new OnlyOnceLoggingDenyMeterFilter(
49+
() ->
50+
String.format(
51+
"Reached the maximum number (%s) of '%s' tags for '%s'",
52+
metricsConfiguration.realmIdTag().httpMetricsMaxCardinality(),
53+
RealmIdTagContributor.TAG_REALM,
54+
binderConfiguration.getHttpServerRequestsName()));
55+
return MeterFilter.maximumAllowableTags(
56+
binderConfiguration.getHttpServerRequestsName(),
57+
RealmIdTagContributor.TAG_REALM,
58+
metricsConfiguration.realmIdTag().httpMetricsMaxCardinality(),
59+
denyFilter);
60+
}
4061
}

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

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,50 @@
1919
package org.apache.polaris.service.quarkus.metrics;
2020

2121
import io.smallrye.config.ConfigMapping;
22+
import io.smallrye.config.WithDefault;
23+
import jakarta.validation.constraints.Min;
2224
import java.util.Map;
2325

2426
@ConfigMapping(prefix = "polaris.metrics")
2527
public interface QuarkusMetricsConfiguration {
2628

2729
/** Additional tags to include in the metrics. */
2830
Map<String, String> tags();
31+
32+
/** Configuration for the Realm ID metric tag. */
33+
RealmIdTag realmIdTag();
34+
35+
interface RealmIdTag {
36+
37+
/**
38+
* Whether to include the Realm ID tag in the API request metrics.
39+
*
40+
* <p>Beware that if the cardinality of this tag is too high, it can cause performance issues or
41+
* even crash the server.
42+
*/
43+
@WithDefault("false")
44+
boolean apiMetricsEnabled();
45+
46+
/**
47+
* Whether to include the Realm ID tag in the HTTP server request metrics.
48+
*
49+
* <p>Beware that if the cardinality of this tag is too high, it can cause performance issues or
50+
* even crash the server.
51+
*/
52+
@WithDefault("false")
53+
boolean httpMetricsEnabled();
54+
55+
/**
56+
* The maximum number of Realm ID tag values allowed for the HTTP server request metrics.
57+
*
58+
* <p>This is used to prevent the number of tags from growing indefinitely and causing
59+
* performance issues or crashing the server.
60+
*
61+
* <p>If the number of tags exceeds this value, a warning will be logged and no more HTTP server
62+
* request metrics will be recorded.
63+
*/
64+
@WithDefault("100")
65+
@Min(1)
66+
int httpMetricsMaxCardinality();
67+
}
2968
}

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

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,17 +22,23 @@
2222
import io.micrometer.common.lang.Nullable;
2323
import jakarta.annotation.Nonnull;
2424
import jakarta.enterprise.context.ApplicationScoped;
25+
import jakarta.inject.Inject;
2526
import org.apache.polaris.core.context.RealmContext;
2627

2728
@ApplicationScoped
2829
public class QuarkusValueExpressionResolver implements ValueExpressionResolver {
2930

31+
@Inject QuarkusMetricsConfiguration metricsConfiguration;
32+
3033
@Override
3134
public String resolve(@Nonnull String expression, @Nullable Object parameter) {
3235
// TODO maybe replace with CEL of some expression engine and make this more generic
33-
if (parameter instanceof RealmContext realmContext && expression.equals("realmIdentifier")) {
36+
if (metricsConfiguration.realmIdTag().apiMetricsEnabled()
37+
&& parameter instanceof RealmContext realmContext
38+
&& expression.equals("realmIdentifier")) {
3439
return realmContext.getRealmIdentifier();
3540
}
36-
return null;
41+
// FIXME cannot return null here, see https://github.com/quarkusio/quarkus/issues/47891
42+
return "";
3743
}
3844
}

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,12 +33,16 @@ public class RealmIdTagContributor implements HttpServerMetricsTagsContributor {
3333

3434
private static final Tags UNFINISHED_RESOLUTION_TAGS = Tags.of(TAG_REALM, "???");
3535

36+
@Inject QuarkusMetricsConfiguration metricsConfiguration;
3637
@Inject RealmContextResolver realmContextResolver;
3738

3839
@Override
3940
public Tags contribute(Context context) {
40-
// FIXME retrieve the realm context from context.requestContextLocalData() when this PR is in:
41-
// https://github.com/quarkusio/quarkus/pull/47887
41+
// FIXME retrieve the realm context from context.requestContextLocalData()
42+
// after upgrading to Quarkus 3.24
43+
if (!metricsConfiguration.realmIdTag().httpMetricsEnabled()) {
44+
return Tags.empty();
45+
}
4246
HttpServerRequest request = context.request();
4347
try {
4448
return realmContextResolver

‎quarkus/service/src/test/java/org/apache/polaris/service/quarkus/TimedApplicationEventListenerTest.java‎ renamed to ‎quarkus/service/src/test/java/org/apache/polaris/service/quarkus/metrics/MetricsTestBase.java‎

Lines changed: 55 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -16,20 +16,16 @@
1616
* specific language governing permissions and limitations
1717
* under the License.
1818
*/
19-
package org.apache.polaris.service.quarkus;
19+
package org.apache.polaris.service.quarkus.metrics;
2020

2121
import static org.apache.polaris.service.context.TestRealmContextResolver.REALM_PROPERTY_KEY;
2222
import static org.assertj.core.api.Assertions.assertThat;
2323
import static org.assertj.core.api.InstanceOfAssertFactories.type;
2424

2525
import io.micrometer.core.instrument.MeterRegistry;
26-
import io.quarkus.test.junit.QuarkusTest;
27-
import io.quarkus.test.junit.QuarkusTestProfile;
28-
import io.quarkus.test.junit.TestProfile;
2926
import jakarta.inject.Inject;
3027
import jakarta.ws.rs.core.Response;
3128
import java.util.Map;
32-
import org.apache.polaris.service.quarkus.TimedApplicationEventListenerTest.Profile;
3329
import org.apache.polaris.service.quarkus.test.PolarisIntegrationTestFixture;
3430
import org.apache.polaris.service.quarkus.test.PolarisIntegrationTestHelper;
3531
import org.apache.polaris.service.quarkus.test.TestEnvironment;
@@ -43,27 +39,15 @@
4339
import org.junit.jupiter.api.TestInfo;
4440
import org.junit.jupiter.api.TestInstance;
4541
import org.junit.jupiter.api.extension.ExtendWith;
46-
import org.junit.jupiter.params.ParameterizedTest;
47-
import org.junit.jupiter.params.provider.ValueSource;
4842

49-
@QuarkusTest
5043
@TestInstance(TestInstance.Lifecycle.PER_CLASS)
5144
@ExtendWith(TestEnvironmentExtension.class)
52-
@TestProfile(Profile.class)
53-
public class TimedApplicationEventListenerTest {
54-
55-
public static class Profile implements QuarkusTestProfile {
56-
57-
@Override
58-
public Map<String, String> getConfigOverrides() {
59-
return Map.of(
60-
"polaris.metrics.tags.environment", "prod", "polaris.realm-context.type", "test");
61-
}
62-
}
45+
public abstract class MetricsTestBase {
6346

6447
private static final int ERROR_CODE = Response.Status.NOT_FOUND.getStatusCode();
6548
private static final String ENDPOINT = "api/management/v1/principals";
66-
private static final String METRIC_NAME = "polaris_principals_getPrincipal_seconds";
49+
private static final String API_METRIC_NAME = "polaris_principals_getPrincipal_seconds";
50+
private static final String HTTP_METRIC_NAME = "http_server_requests_seconds";
6751

6852
@Inject PolarisIntegrationTestHelper helper;
6953
@Inject MeterRegistry registry;
@@ -82,21 +66,19 @@ public void clearMetrics() {
8266
registry.clear();
8367
}
8468

85-
@ParameterizedTest
86-
@ValueSource(strings = {"%s/metrics", "%s/q/metrics"})
87-
public void testMetricsEmittedOnSuccessfulRequest(String endpoint) {
69+
protected void testMetricsEmittedOnSuccessfulRequest(String endpoint, boolean realmIdTagEnabled) {
8870
sendSuccessfulRequest();
8971
Map<String, MetricFamily> allMetrics =
9072
TestMetricsUtil.fetchMetrics(fixture.client, testEnv.baseManagementUri(), endpoint);
91-
assertThat(allMetrics).containsKey(METRIC_NAME);
92-
assertThat(allMetrics.get(METRIC_NAME).getMetrics())
73+
assertThat(allMetrics).containsKey(API_METRIC_NAME);
74+
assertThat(allMetrics.get(API_METRIC_NAME).getMetrics())
9375
.satisfiesOnlyOnce(
9476
metric -> {
9577
assertThat(metric.getLabels())
9678
.contains(
9779
Map.entry("application", "Polaris"),
9880
Map.entry("environment", "prod"),
99-
Map.entry("realm_id", fixture.realm),
81+
Map.entry("realm_id", realmIdTagEnabled ? fixture.realm : ""),
10082
Map.entry(
10183
"class", "org.apache.polaris.service.admin.api.PolarisPrincipalsApi"),
10284
Map.entry("exception", "none"),
@@ -106,23 +88,43 @@ public void testMetricsEmittedOnSuccessfulRequest(String endpoint) {
10688
.extracting(Summary::getSampleCount)
10789
.isEqualTo(1L);
10890
});
91+
assertThat(allMetrics).containsKey(HTTP_METRIC_NAME);
92+
assertThat(allMetrics.get(HTTP_METRIC_NAME).getMetrics())
93+
.satisfiesOnlyOnce(
94+
metric -> {
95+
assertThat(metric.getLabels())
96+
.contains(
97+
Map.entry("application", "Polaris"),
98+
Map.entry("environment", "prod"),
99+
Map.entry("method", "GET"),
100+
Map.entry("outcome", "SUCCESS"),
101+
Map.entry("status", "200"),
102+
Map.entry("uri", "/api/management/v1/principals/{principalName}"));
103+
if (realmIdTagEnabled) {
104+
assertThat(metric.getLabels()).containsEntry("realm_id", fixture.realm);
105+
} else {
106+
assertThat(metric.getLabels()).doesNotContainKey("realm_id");
107+
}
108+
assertThat(metric)
109+
.asInstanceOf(type(Summary.class))
110+
.extracting(Summary::getSampleCount)
111+
.isEqualTo(1L);
112+
});
109113
}
110114

111-
@ParameterizedTest
112-
@ValueSource(strings = {"%s/metrics", "%s/q/metrics"})
113-
public void testMetricsEmittedOnFailedRequest(String endpoint) {
115+
protected void testMetricsEmittedOnFailedRequest(String endpoint, boolean realmIdTagEnabled) {
114116
sendFailingRequest();
115117
Map<String, MetricFamily> allMetrics =
116118
TestMetricsUtil.fetchMetrics(fixture.client, testEnv.baseManagementUri(), endpoint);
117-
assertThat(allMetrics).containsKey(METRIC_NAME);
118-
assertThat(allMetrics.get(METRIC_NAME).getMetrics())
119+
assertThat(allMetrics).containsKey(API_METRIC_NAME);
120+
assertThat(allMetrics.get(API_METRIC_NAME).getMetrics())
119121
.satisfiesOnlyOnce(
120122
metric -> {
121123
assertThat(metric.getLabels())
122124
.contains(
123125
Map.entry("application", "Polaris"),
124126
Map.entry("environment", "prod"),
125-
Map.entry("realm_id", fixture.realm),
127+
Map.entry("realm_id", realmIdTagEnabled ? fixture.realm : ""),
126128
Map.entry(
127129
"class", "org.apache.polaris.service.admin.api.PolarisPrincipalsApi"),
128130
Map.entry("exception", "NotFoundException"),
@@ -132,6 +134,27 @@ public void testMetricsEmittedOnFailedRequest(String endpoint) {
132134
.extracting(Summary::getSampleCount)
133135
.isEqualTo(1L);
134136
});
137+
assertThat(allMetrics.get(HTTP_METRIC_NAME).getMetrics())
138+
.satisfiesOnlyOnce(
139+
metric -> {
140+
assertThat(metric.getLabels())
141+
.contains(
142+
Map.entry("application", "Polaris"),
143+
Map.entry("environment", "prod"),
144+
Map.entry("method", "GET"),
145+
Map.entry("outcome", "CLIENT_ERROR"),
146+
Map.entry("status", "404"),
147+
Map.entry("uri", "/api/management/v1/principals/{principalName}"));
148+
if (realmIdTagEnabled) {
149+
assertThat(metric.getLabels()).containsEntry("realm_id", fixture.realm);
150+
} else {
151+
assertThat(metric.getLabels()).doesNotContainKey("realm_id");
152+
}
153+
assertThat(metric)
154+
.asInstanceOf(type(Summary.class))
155+
.extracting(Summary::getSampleCount)
156+
.isEqualTo(1L);
157+
});
135158
}
136159

137160
private int sendRequest(String principalName) {

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

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
package org.apache.polaris.service.quarkus.metrics;
20+
21+
import io.quarkus.test.junit.QuarkusTest;
22+
import io.quarkus.test.junit.QuarkusTestProfile;
23+
import io.quarkus.test.junit.TestProfile;
24+
import java.util.Map;
25+
import org.junit.jupiter.params.ParameterizedTest;
26+
import org.junit.jupiter.params.provider.ValueSource;
27+
28+
@QuarkusTest
29+
@TestProfile(RealmIdTagDisabledMetricsTest.Profile.class)
30+
public class RealmIdTagDisabledMetricsTest extends MetricsTestBase {
31+
32+
public static class Profile implements QuarkusTestProfile {
33+
34+
@Override
35+
public Map<String, String> getConfigOverrides() {
36+
return Map.of(
37+
"polaris.metrics.tags.environment", "prod", "polaris.realm-context.type", "test");
38+
}
39+
}
40+
41+
@ParameterizedTest
42+
@ValueSource(strings = {"%s/metrics", "%s/q/metrics"})
43+
public void testMetricsEmittedOnSuccessfulRequest(String endpoint) {
44+
super.testMetricsEmittedOnSuccessfulRequest(endpoint, false);
45+
}
46+
47+
@ParameterizedTest
48+
@ValueSource(strings = {"%s/metrics", "%s/q/metrics"})
49+
public void testMetricsEmittedOnFailedRequest(String endpoint) {
50+
super.testMetricsEmittedOnFailedRequest(endpoint, false);
51+
}
52+
}

0 commit comments

Comments
 (0)