Skip to content

Commit 28aa123

Browse files
slobodanadamovicslobodanadamovic
authored
Remove high-cardinality security metric's attributes (elastic#136534)
Removing high-cardinality metric attributes to avoid dimensionality explosion that can cause performance issues. These attributes are not used. The original idea was that we may want to track authentication times per e.g. individual API key, but that turned out unnecessary. We are mostly interested in recording average and percentile authentication times.
1 parent e82c457 commit 28aa123

File tree

11 files changed

+24
-109
lines changed

11 files changed

+24
-109
lines changed

‎x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyAuthenticator.java‎

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -21,17 +21,14 @@
2121
import org.elasticsearch.xpack.security.metric.SecurityMetricType;
2222
import org.elasticsearch.xpack.security.metric.SecurityMetrics;
2323

24-
import java.util.HashMap;
2524
import java.util.Map;
2625
import java.util.function.LongSupplier;
2726

2827
import static org.elasticsearch.core.Strings.format;
2928

3029
class ApiKeyAuthenticator implements Authenticator {
3130

32-
public static final String ATTRIBUTE_API_KEY_ID = "es.security.api_key_id";
3331
public static final String ATTRIBUTE_API_KEY_TYPE = "es.security.api_key_type";
34-
public static final String ATTRIBUTE_API_KEY_AUTHC_FAILURE_REASON = "es.security.api_key_authc_failure_reason";
3532

3633
private static final Logger logger = LogManager.getLogger(ApiKeyAuthenticator.class);
3734

@@ -47,7 +44,7 @@ class ApiKeyAuthenticator implements Authenticator {
4744
this.authenticationMetrics = new SecurityMetrics<>(
4845
SecurityMetricType.AUTHC_API_KEY,
4946
meterRegistry,
50-
this::buildMetricAttributes,
47+
credentials -> Map.of(ATTRIBUTE_API_KEY_TYPE, credentials.getExpectedType().value()),
5148
nanoTimeSupplier
5249
);
5350
this.apiKeyService = apiKeyService;
@@ -104,14 +101,4 @@ public void authenticate(Context context, ActionListener<AuthenticationResult<Au
104101
}, e -> listener.onFailure(context.getRequest().exceptionProcessingRequest(e, null))))
105102
);
106103
}
107-
108-
private Map<String, Object> buildMetricAttributes(ApiKeyCredentials credentials, String failureReason) {
109-
final Map<String, Object> attributes = new HashMap<>(failureReason != null ? 3 : 2);
110-
attributes.put(ATTRIBUTE_API_KEY_ID, credentials.getId());
111-
attributes.put(ATTRIBUTE_API_KEY_TYPE, credentials.getExpectedType().value());
112-
if (failureReason != null) {
113-
attributes.put(ATTRIBUTE_API_KEY_AUTHC_FAILURE_REASON, failureReason);
114-
}
115-
return attributes;
116-
}
117104
}

‎x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/OAuth2TokenAuthenticator.java‎

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,6 @@
2626

2727
class OAuth2TokenAuthenticator implements Authenticator {
2828

29-
public static final String ATTRIBUTE_AUTHC_FAILURE_REASON = "es.security.token_authc_failure_reason";
30-
3129
private static final Logger logger = LogManager.getLogger(OAuth2TokenAuthenticator.class);
3230

3331
private final SecurityMetrics<BearerToken> authenticationMetrics;
@@ -41,7 +39,7 @@ class OAuth2TokenAuthenticator implements Authenticator {
4139
this.authenticationMetrics = new SecurityMetrics<>(
4240
SecurityMetricType.AUTHC_OAUTH2_TOKEN,
4341
meterRegistry,
44-
this::buildMetricAttributes,
42+
token -> Map.of(),
4543
nanoTimeSupplier
4644
);
4745
this.tokenService = tokenService;
@@ -88,10 +86,4 @@ private void doAuthenticate(Context context, BearerToken bearerToken, ActionList
8886
}));
8987
}
9088

91-
private Map<String, Object> buildMetricAttributes(BearerToken token, String failureReason) {
92-
if (failureReason != null) {
93-
return Map.of(ATTRIBUTE_AUTHC_FAILURE_REASON, failureReason);
94-
}
95-
return Map.of();
96-
}
9789
}

‎x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/RealmsAuthenticator.java‎

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@
3232

3333
import java.util.ArrayList;
3434
import java.util.Collections;
35-
import java.util.HashMap;
3635
import java.util.LinkedHashMap;
3736
import java.util.List;
3837
import java.util.Map;
@@ -48,7 +47,6 @@ public class RealmsAuthenticator implements Authenticator {
4847

4948
public static final String ATTRIBUTE_REALM_NAME = "es.security.realm_name";
5049
public static final String ATTRIBUTE_REALM_TYPE = "es.security.realm_type";
51-
public static final String ATTRIBUTE_REALM_AUTHC_FAILURE_REASON = "es.security.realm_authc_failure_reason";
5250

5351
private static final Logger logger = LogManager.getLogger(RealmsAuthenticator.class);
5452

@@ -71,7 +69,7 @@ public RealmsAuthenticator(AtomicLong numInvalidation, Cache<String, Realm> last
7169
this.authenticationMetrics = new SecurityMetrics<>(
7270
SecurityMetricType.AUTHC_REALMS,
7371
meterRegistry,
74-
this::buildMetricAttributes,
72+
realm -> Map.ofEntries(Map.entry(ATTRIBUTE_REALM_NAME, realm.name()), Map.entry(ATTRIBUTE_REALM_TYPE, realm.type())),
7573
nanoTimeSupplier
7674
);
7775
}
@@ -418,14 +416,4 @@ public synchronized Throwable fillInStackTrace() {
418416
return this;
419417
}
420418
}
421-
422-
private Map<String, Object> buildMetricAttributes(Realm realm, String failureReason) {
423-
final Map<String, Object> attributes = new HashMap<>(failureReason != null ? 3 : 2);
424-
attributes.put(ATTRIBUTE_REALM_NAME, realm.name());
425-
attributes.put(ATTRIBUTE_REALM_TYPE, realm.type());
426-
if (failureReason != null) {
427-
attributes.put(ATTRIBUTE_REALM_AUTHC_FAILURE_REASON, failureReason);
428-
}
429-
return attributes;
430-
}
431419
}

‎x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ServiceAccountAuthenticator.java‎

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -21,15 +21,12 @@
2121
import org.elasticsearch.xpack.security.metric.SecurityMetricType;
2222
import org.elasticsearch.xpack.security.metric.SecurityMetrics;
2323

24-
import java.util.HashMap;
2524
import java.util.Map;
2625
import java.util.function.LongSupplier;
2726

2827
class ServiceAccountAuthenticator implements Authenticator {
2928

3029
public static final String ATTRIBUTE_SERVICE_ACCOUNT_ID = "es.security.service_account_id";
31-
public static final String ATTRIBUTE_SERVICE_ACCOUNT_TOKEN_NAME = "es.security.service_account_token_name";
32-
public static final String ATTRIBUTE_AUTHC_FAILURE_REASON = "es.security.service_account_authc_failure_reason";
3330

3431
private static final Logger logger = LogManager.getLogger(ServiceAccountAuthenticator.class);
3532
private final ServiceAccountService serviceAccountService;
@@ -52,7 +49,7 @@ class ServiceAccountAuthenticator implements Authenticator {
5249
this.authenticationMetrics = new SecurityMetrics<>(
5350
SecurityMetricType.AUTHC_SERVICE_ACCOUNT,
5451
meterRegistry,
55-
this::buildMetricAttributes,
52+
serviceAccountToken -> Map.of(ATTRIBUTE_SERVICE_ACCOUNT_ID, serviceAccountToken.getAccountId().asPrincipal()),
5653
nanoTimeSupplier
5754
);
5855
}
@@ -99,14 +96,4 @@ private void doAuthenticate(
9996
listener.onFailure(context.getRequest().exceptionProcessingRequest(e, serviceAccountToken));
10097
}));
10198
}
102-
103-
private Map<String, Object> buildMetricAttributes(ServiceAccountToken serviceAccountToken, String failureReason) {
104-
final Map<String, Object> attributes = new HashMap<>(3);
105-
attributes.put(ATTRIBUTE_SERVICE_ACCOUNT_ID, serviceAccountToken.getAccountId().asPrincipal());
106-
attributes.put(ATTRIBUTE_SERVICE_ACCOUNT_TOKEN_NAME, serviceAccountToken.getTokenName());
107-
if (failureReason != null) {
108-
attributes.put(ATTRIBUTE_AUTHC_FAILURE_REASON, failureReason);
109-
}
110-
return attributes;
111-
}
11299
}

‎x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/metric/InstrumentedSecurityActionListener.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,11 @@ public static <R, C> ActionListener<AuthenticationResult<R>> wrapForAuthc(
3333
if (result.isAuthenticated()) {
3434
metrics.recordSuccess(context);
3535
} else {
36-
metrics.recordFailure(context, result.getMessage());
36+
metrics.recordFailure(context);
3737
}
3838
listener.onResponse(result);
3939
}, e -> {
40-
metrics.recordFailure(context, e.getMessage());
40+
metrics.recordFailure(context);
4141
listener.onFailure(e);
4242
}), () -> metrics.recordTime(context, startTimeNano));
4343
}
@@ -57,11 +57,11 @@ public static <R> ActionListener<AuthenticationResult<R>> wrapForAuthc(
5757
if (result.isAuthenticated()) {
5858
metrics.recordSuccess(result);
5959
} else {
60-
metrics.recordFailure(result, result.getMessage());
60+
metrics.recordFailure(result);
6161
}
6262
listener.onResponse(result);
6363
}, e -> {
64-
metrics.recordFailure(null, e.getMessage());
64+
metrics.recordFailure(null);
6565
listener.onFailure(e);
6666
}), () -> metrics.recordTime(null, startTimeNano));
6767
}

‎x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/metric/SecurityMetricAttributesBuilder.java‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,6 @@
1212
@FunctionalInterface
1313
public interface SecurityMetricAttributesBuilder<C> {
1414

15-
Map<String, Object> build(C context, String failureReason);
16-
17-
default Map<String, Object> build(C context) {
18-
return build(context, null);
19-
}
15+
Map<String, Object> build(C context);
2016

2117
}

‎x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/metric/SecurityMetrics.java‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -69,10 +69,9 @@ public void recordSuccess(final C context) {
6969
* Records a single failed execution.
7070
*
7171
* @param context The context object which is used to attach additional attributes to failed metric.
72-
* @param failureReason The optional failure reason which is stored as an attributed with recorded failure metric.
7372
*/
74-
public void recordFailure(final C context, final String failureReason) {
75-
this.failuresCounter.incrementBy(1L, attributesBuilder.build(context, failureReason));
73+
public void recordFailure(final C context) {
74+
this.failuresCounter.incrementBy(1L, attributesBuilder.build(context));
7675
}
7776

7877
/**

‎x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyAuthenticatorTests.java‎

Lines changed: 7 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -114,10 +114,7 @@ public void testRecordingSuccessfulAuthenticationMetrics() {
114114
assertSingleSuccessAuthMetric(
115115
telemetryPlugin,
116116
SecurityMetricType.AUTHC_API_KEY,
117-
Map.ofEntries(
118-
Map.entry(ApiKeyAuthenticator.ATTRIBUTE_API_KEY_ID, apiKeyCredentials.getId()),
119-
Map.entry(ApiKeyAuthenticator.ATTRIBUTE_API_KEY_TYPE, apiKeyCredentials.getExpectedType().value())
120-
)
117+
Map.ofEntries(Map.entry(ApiKeyAuthenticator.ATTRIBUTE_API_KEY_TYPE, apiKeyCredentials.getExpectedType().value()))
121118
);
122119

123120
// verify that there were no failures recorded
@@ -128,10 +125,7 @@ public void testRecordingSuccessfulAuthenticationMetrics() {
128125
telemetryPlugin,
129126
SecurityMetricType.AUTHC_API_KEY,
130127
executionTimeInNanos,
131-
Map.ofEntries(
132-
Map.entry(ApiKeyAuthenticator.ATTRIBUTE_API_KEY_ID, apiKeyCredentials.getId()),
133-
Map.entry(ApiKeyAuthenticator.ATTRIBUTE_API_KEY_TYPE, apiKeyCredentials.getExpectedType().value())
134-
)
128+
Map.ofEntries(Map.entry(ApiKeyAuthenticator.ATTRIBUTE_API_KEY_TYPE, apiKeyCredentials.getExpectedType().value()))
135129
);
136130
}
137131

@@ -175,23 +169,15 @@ public void testRecordingFailedAuthenticationMetrics() {
175169
assertSingleFailedAuthMetric(
176170
telemetryPlugin,
177171
SecurityMetricType.AUTHC_API_KEY,
178-
Map.ofEntries(
179-
Map.entry(ApiKeyAuthenticator.ATTRIBUTE_API_KEY_ID, apiKeyCredentials.getId()),
180-
Map.entry(ApiKeyAuthenticator.ATTRIBUTE_API_KEY_TYPE, apiKeyCredentials.getExpectedType().value()),
181-
Map.entry(ApiKeyAuthenticator.ATTRIBUTE_API_KEY_AUTHC_FAILURE_REASON, "terminated API key auth")
182-
)
172+
Map.ofEntries(Map.entry(ApiKeyAuthenticator.ATTRIBUTE_API_KEY_TYPE, apiKeyCredentials.getExpectedType().value()))
183173
);
184174
} else {
185175
var authResult = future.actionGet();
186176
assertThat(authResult.isAuthenticated(), equalTo(false));
187177
assertSingleFailedAuthMetric(
188178
telemetryPlugin,
189179
SecurityMetricType.AUTHC_API_KEY,
190-
Map.ofEntries(
191-
Map.entry(ApiKeyAuthenticator.ATTRIBUTE_API_KEY_ID, apiKeyCredentials.getId()),
192-
Map.entry(ApiKeyAuthenticator.ATTRIBUTE_API_KEY_TYPE, apiKeyCredentials.getExpectedType().value()),
193-
Map.entry(ApiKeyAuthenticator.ATTRIBUTE_API_KEY_AUTHC_FAILURE_REASON, "unsuccessful API key auth")
194-
)
180+
Map.ofEntries(Map.entry(ApiKeyAuthenticator.ATTRIBUTE_API_KEY_TYPE, apiKeyCredentials.getExpectedType().value()))
195181
);
196182
}
197183

@@ -203,10 +189,7 @@ public void testRecordingFailedAuthenticationMetrics() {
203189
telemetryPlugin,
204190
SecurityMetricType.AUTHC_API_KEY,
205191
executionTimeInNanos,
206-
Map.ofEntries(
207-
Map.entry(ApiKeyAuthenticator.ATTRIBUTE_API_KEY_ID, apiKeyCredentials.getId()),
208-
Map.entry(ApiKeyAuthenticator.ATTRIBUTE_API_KEY_TYPE, apiKeyCredentials.getExpectedType().value())
209-
)
192+
Map.ofEntries(Map.entry(ApiKeyAuthenticator.ATTRIBUTE_API_KEY_TYPE, apiKeyCredentials.getExpectedType().value()))
210193
);
211194
}
212195

@@ -241,11 +224,7 @@ public void testRecordingFailedAuthenticationMetricsOnExceptions() {
241224
assertSingleFailedAuthMetric(
242225
telemetryPlugin,
243226
SecurityMetricType.AUTHC_API_KEY,
244-
Map.ofEntries(
245-
Map.entry(ApiKeyAuthenticator.ATTRIBUTE_API_KEY_ID, apiKeyCredentials.getId()),
246-
Map.entry(ApiKeyAuthenticator.ATTRIBUTE_API_KEY_TYPE, apiKeyCredentials.getExpectedType().value()),
247-
Map.entry(ApiKeyAuthenticator.ATTRIBUTE_API_KEY_AUTHC_FAILURE_REASON, "API key auth exception")
248-
)
227+
Map.ofEntries(Map.entry(ApiKeyAuthenticator.ATTRIBUTE_API_KEY_TYPE, apiKeyCredentials.getExpectedType().value()))
249228
);
250229

251230
// verify that there were no successes recorded
@@ -256,10 +235,7 @@ public void testRecordingFailedAuthenticationMetricsOnExceptions() {
256235
telemetryPlugin,
257236
SecurityMetricType.AUTHC_API_KEY,
258237
executionTimeInNanos,
259-
Map.ofEntries(
260-
Map.entry(ApiKeyAuthenticator.ATTRIBUTE_API_KEY_ID, apiKeyCredentials.getId()),
261-
Map.entry(ApiKeyAuthenticator.ATTRIBUTE_API_KEY_TYPE, apiKeyCredentials.getExpectedType().value())
262-
)
238+
Map.ofEntries(Map.entry(ApiKeyAuthenticator.ATTRIBUTE_API_KEY_TYPE, apiKeyCredentials.getExpectedType().value()))
263239
);
264240
}
265241

‎x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/OAuth2TokenAuthenticatorTests.java‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -112,11 +112,7 @@ public void testRecordingFailedAuthenticationMetrics() {
112112
assertThat(e, sameInstance(failureError));
113113

114114
// verify we recorded failure metric
115-
assertSingleFailedAuthMetric(
116-
telemetryPlugin,
117-
SecurityMetricType.AUTHC_OAUTH2_TOKEN,
118-
Map.ofEntries(Map.entry(OAuth2TokenAuthenticator.ATTRIBUTE_AUTHC_FAILURE_REASON, "failed to authenticate OAuth2 token"))
119-
);
115+
assertSingleFailedAuthMetric(telemetryPlugin, SecurityMetricType.AUTHC_OAUTH2_TOKEN, Map.of());
120116

121117
// verify that there were no successes recorded
122118
assertZeroSuccessAuthMetrics(telemetryPlugin, SecurityMetricType.AUTHC_OAUTH2_TOKEN);

‎x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/RealmsAuthenticatorTests.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -345,8 +345,7 @@ public void testRecordingFailedAuthenticationMetric() {
345345
SecurityMetricType.AUTHC_REALMS,
346346
Map.ofEntries(
347347
Map.entry(RealmsAuthenticator.ATTRIBUTE_REALM_NAME, unsuccessfulRealm.name()),
348-
Map.entry(RealmsAuthenticator.ATTRIBUTE_REALM_TYPE, unsuccessfulRealm.type()),
349-
Map.entry(RealmsAuthenticator.ATTRIBUTE_REALM_AUTHC_FAILURE_REASON, "unsuccessful realms authentication")
348+
Map.entry(RealmsAuthenticator.ATTRIBUTE_REALM_TYPE, unsuccessfulRealm.type())
350349
)
351350
);
352351

0 commit comments

Comments
 (0)