Skip to content

Commit 97d16d3

Browse files
adutraadutra
authored
feat(metrics): Mitigate potential performance issues with realm_id tag (#1662)
As discussed in the ML, this PR introduces two flags to enable the inclusion of realm ID tags in API and HTTP metrics. They are both disabled by default. There is also a new safeguard: if the cardinality of realm IDs in HTTP metrics goes above a configurable threshold (100 by default), a warning is printed and no more HTTP metrics will be recorded. (Quarkus has a similar safeguard for URI tags in HTTP metrics.)
1 parent f93e347 commit 97d16d3

File tree

10 files changed

+296
-89
lines changed

10 files changed

+296
-89
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 enableInApiMetrics();
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 enableInHttpMetrics();
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().enableInApiMetrics()
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().enableInHttpMetrics()) {
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: 62 additions & 24 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;
@@ -46,27 +42,18 @@
4642
import org.junit.jupiter.params.ParameterizedTest;
4743
import org.junit.jupiter.params.provider.ValueSource;
4844

49-
@QuarkusTest
5045
@TestInstance(TestInstance.Lifecycle.PER_CLASS)
5146
@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-
}
47+
public abstract class MetricsTestBase {
6348

6449
private static final int ERROR_CODE = Response.Status.NOT_FOUND.getStatusCode();
6550
private static final String ENDPOINT = "api/management/v1/principals";
66-
private static final String METRIC_NAME = "polaris_principals_getPrincipal_seconds";
51+
private static final String API_METRIC_NAME = "polaris_principals_getPrincipal_seconds";
52+
private static final String HTTP_METRIC_NAME = "http_server_requests_seconds";
6753

6854
@Inject PolarisIntegrationTestHelper helper;
6955
@Inject MeterRegistry registry;
56+
@Inject QuarkusMetricsConfiguration metricsConfiguration;
7057

7158
private TestEnvironment testEnv;
7259
private PolarisIntegrationTestFixture fixture;
@@ -88,15 +75,19 @@ public void testMetricsEmittedOnSuccessfulRequest(String endpoint) {
8875
sendSuccessfulRequest();
8976
Map<String, MetricFamily> allMetrics =
9077
TestMetricsUtil.fetchMetrics(fixture.client, testEnv.baseManagementUri(), endpoint);
91-
assertThat(allMetrics).containsKey(METRIC_NAME);
92-
assertThat(allMetrics.get(METRIC_NAME).getMetrics())
78+
assertThat(allMetrics).containsKey(API_METRIC_NAME);
79+
assertThat(allMetrics.get(API_METRIC_NAME).getMetrics())
9380
.satisfiesOnlyOnce(
9481
metric -> {
9582
assertThat(metric.getLabels())
9683
.contains(
9784
Map.entry("application", "Polaris"),
9885
Map.entry("environment", "prod"),
99-
Map.entry("realm_id", fixture.realm),
86+
Map.entry(
87+
"realm_id",
88+
metricsConfiguration.realmIdTag().enableInApiMetrics()
89+
? fixture.realm
90+
: ""),
10091
Map.entry(
10192
"class", "org.apache.polaris.service.admin.api.PolarisPrincipalsApi"),
10293
Map.entry("exception", "none"),
@@ -106,6 +97,28 @@ public void testMetricsEmittedOnSuccessfulRequest(String endpoint) {
10697
.extracting(Summary::getSampleCount)
10798
.isEqualTo(1L);
10899
});
100+
assertThat(allMetrics).containsKey(HTTP_METRIC_NAME);
101+
assertThat(allMetrics.get(HTTP_METRIC_NAME).getMetrics())
102+
.satisfiesOnlyOnce(
103+
metric -> {
104+
assertThat(metric.getLabels())
105+
.contains(
106+
Map.entry("application", "Polaris"),
107+
Map.entry("environment", "prod"),
108+
Map.entry("method", "GET"),
109+
Map.entry("outcome", "SUCCESS"),
110+
Map.entry("status", "200"),
111+
Map.entry("uri", "/api/management/v1/principals/{principalName}"));
112+
if (metricsConfiguration.realmIdTag().enableInHttpMetrics()) {
113+
assertThat(metric.getLabels()).containsEntry("realm_id", fixture.realm);
114+
} else {
115+
assertThat(metric.getLabels()).doesNotContainKey("realm_id");
116+
}
117+
assertThat(metric)
118+
.asInstanceOf(type(Summary.class))
119+
.extracting(Summary::getSampleCount)
120+
.isEqualTo(1L);
121+
});
109122
}
110123

111124
@ParameterizedTest
@@ -114,15 +127,19 @@ public void testMetricsEmittedOnFailedRequest(String endpoint) {
114127
sendFailingRequest();
115128
Map<String, MetricFamily> allMetrics =
116129
TestMetricsUtil.fetchMetrics(fixture.client, testEnv.baseManagementUri(), endpoint);
117-
assertThat(allMetrics).containsKey(METRIC_NAME);
118-
assertThat(allMetrics.get(METRIC_NAME).getMetrics())
130+
assertThat(allMetrics).containsKey(API_METRIC_NAME);
131+
assertThat(allMetrics.get(API_METRIC_NAME).getMetrics())
119132
.satisfiesOnlyOnce(
120133
metric -> {
121134
assertThat(metric.getLabels())
122135
.contains(
123136
Map.entry("application", "Polaris"),
124137
Map.entry("environment", "prod"),
125-
Map.entry("realm_id", fixture.realm),
138+
Map.entry(
139+
"realm_id",
140+
metricsConfiguration.realmIdTag().enableInApiMetrics()
141+
? fixture.realm
142+
: ""),
126143
Map.entry(
127144
"class", "org.apache.polaris.service.admin.api.PolarisPrincipalsApi"),
128145
Map.entry("exception", "NotFoundException"),
@@ -132,6 +149,27 @@ public void testMetricsEmittedOnFailedRequest(String endpoint) {
132149
.extracting(Summary::getSampleCount)
133150
.isEqualTo(1L);
134151
});
152+
assertThat(allMetrics.get(HTTP_METRIC_NAME).getMetrics())
153+
.satisfiesOnlyOnce(
154+
metric -> {
155+
assertThat(metric.getLabels())
156+
.contains(
157+
Map.entry("application", "Polaris"),
158+
Map.entry("environment", "prod"),
159+
Map.entry("method", "GET"),
160+
Map.entry("outcome", "CLIENT_ERROR"),
161+
Map.entry("status", "404"),
162+
Map.entry("uri", "/api/management/v1/principals/{principalName}"));
163+
if (metricsConfiguration.realmIdTag().enableInHttpMetrics()) {
164+
assertThat(metric.getLabels()).containsEntry("realm_id", fixture.realm);
165+
} else {
166+
assertThat(metric.getLabels()).doesNotContainKey("realm_id");
167+
}
168+
assertThat(metric)
169+
.asInstanceOf(type(Summary.class))
170+
.extracting(Summary::getSampleCount)
171+
.isEqualTo(1L);
172+
});
135173
}
136174

137175
private int sendRequest(String principalName) {

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

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
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+
26+
@QuarkusTest
27+
@TestProfile(RealmIdTagDisabledMetricsTest.Profile.class)
28+
public class RealmIdTagDisabledMetricsTest extends MetricsTestBase {
29+
30+
public static class Profile implements QuarkusTestProfile {
31+
32+
@Override
33+
public Map<String, String> getConfigOverrides() {
34+
return Map.of(
35+
"polaris.metrics.tags.environment", "prod", "polaris.realm-context.type", "test");
36+
}
37+
}
38+
}

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

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
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+
26+
@QuarkusTest
27+
@TestProfile(RealmIdTagEnabledMetricsTest.Profile.class)
28+
public class RealmIdTagEnabledMetricsTest extends MetricsTestBase {
29+
30+
public static class Profile implements QuarkusTestProfile {
31+
32+
@Override
33+
public Map<String, String> getConfigOverrides() {
34+
return Map.of(
35+
"polaris.metrics.tags.environment",
36+
"prod",
37+
"polaris.realm-context.type",
38+
"test",
39+
"polaris.metrics.realm-id-tag.enable-in-api-metrics",
40+
"true",
41+
"polaris.metrics.realm-id-tag.enable-in-http-metrics",
42+
"true");
43+
}
44+
}
45+
}

0 commit comments

Comments
 (0)