From d5a4582506c574643490265b797e84ad768dc555 Mon Sep 17 00:00:00 2001 From: ankitsethi Date: Thu, 21 Aug 2025 17:57:25 -0500 Subject: [PATCH 1/3] Adding tracing support for the `authenticate` action by wrapping the `authenticator.authenticate` call in AuthenticatorChain with a startTrace/stopTrace action. In synchronous contexts, spans are stored as ThreadLocals under the hood and the currently active span look up (needed to end a span, append metadata, or spawn a child span) can be implicit. However, to track a span across threads as ES requires, we need to make `Authenticator.Context` implement `Traceable` which defines a random, unique `spanId` for each Context object, enabling us to uniquely locate the associated span regardless of which thread is currently executing. --- .../apm/internal/APMAgentSettings.java | 5 ++++ .../apm/internal/tracing/APMTracer.java | 2 +- .../xpack/security/Security.java | 2 +- .../security/authc/AuthenticationService.java | 8 ++++-- .../xpack/security/authc/Authenticator.java | 13 +++++++++- .../security/authc/AuthenticatorChain.java | 25 ++++++++++++++++--- .../authc/AuthenticatorChainTests.java | 2 ++ 7 files changed, 48 insertions(+), 9 deletions(-) diff --git a/modules/apm/src/main/java/org/elasticsearch/telemetry/apm/internal/APMAgentSettings.java b/modules/apm/src/main/java/org/elasticsearch/telemetry/apm/internal/APMAgentSettings.java index 11f8070083b11..c5760e5aef94d 100644 --- a/modules/apm/src/main/java/org/elasticsearch/telemetry/apm/internal/APMAgentSettings.java +++ b/modules/apm/src/main/java/org/elasticsearch/telemetry/apm/internal/APMAgentSettings.java @@ -285,6 +285,11 @@ private static Setting concreteAgentSetting(String namespace, String qua NodeScope ); + // not a setting to reduce the footprint of this change + public static final List TELEMETRY_TRACING_SANITIZE_FIELD_NAMES_EXCLUDES = List.of( + "security.authenticator.type" + ); + public static final Setting TELEMETRY_TRACING_ENABLED_SETTING = Setting.boolSetting( TELEMETRY_SETTING_PREFIX + "tracing.enabled", false, diff --git a/modules/apm/src/main/java/org/elasticsearch/telemetry/apm/internal/tracing/APMTracer.java b/modules/apm/src/main/java/org/elasticsearch/telemetry/apm/internal/tracing/APMTracer.java index f0eced2c56b21..b288f2544b3f8 100644 --- a/modules/apm/src/main/java/org/elasticsearch/telemetry/apm/internal/tracing/APMTracer.java +++ b/modules/apm/src/main/java/org/elasticsearch/telemetry/apm/internal/tracing/APMTracer.java @@ -90,7 +90,7 @@ public APMTracer(Settings settings) { this.labelFilters = APMAgentSettings.TELEMETRY_TRACING_SANITIZE_FIELD_NAMES.get(settings); this.filterAutomaton = buildAutomaton(includeNames, excludeNames); - this.labelFilterAutomaton = buildAutomaton(labelFilters, List.of()); + this.labelFilterAutomaton = buildAutomaton(labelFilters, APMAgentSettings.TELEMETRY_TRACING_SANITIZE_FIELD_NAMES_EXCLUDES); this.enabled = APMAgentSettings.TELEMETRY_TRACING_ENABLED_SETTING.get(settings); } diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java index bf4ae62983c38..83c7520b10888 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java @@ -1085,7 +1085,7 @@ Collection createComponents( serviceAccountService, operatorPrivilegesService.get(), customApiKeyAuthenticator, - telemetryProvider.getMeterRegistry() + telemetryProvider ) ); components.add(authcService.get()); diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/AuthenticationService.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/AuthenticationService.java index a9c513a605fe8..6b08c8cfd9603 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/AuthenticationService.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/AuthenticationService.java @@ -22,7 +22,9 @@ import org.elasticsearch.core.TimeValue; import org.elasticsearch.http.HttpPreRequest; import org.elasticsearch.node.Node; +import org.elasticsearch.telemetry.TelemetryProvider; import org.elasticsearch.telemetry.metric.MeterRegistry; +import org.elasticsearch.telemetry.tracing.Tracer; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.transport.TransportRequest; import org.elasticsearch.xpack.core.security.authc.Authentication; @@ -94,7 +96,7 @@ public AuthenticationService( ServiceAccountService serviceAccountService, OperatorPrivilegesService operatorPrivilegesService, CustomApiKeyAuthenticator customApiKeyAuthenticator, - MeterRegistry meterRegistry + TelemetryProvider telemetryProvider ) { this.realms = realms; this.auditTrailService = auditTrailService; @@ -108,13 +110,15 @@ public AuthenticationService( } else { this.lastSuccessfulAuthCache = null; } - + MeterRegistry meterRegistry = telemetryProvider.getMeterRegistry(); + Tracer tracer = telemetryProvider.getTracer(); final String nodeName = Node.NODE_NAME_SETTING.get(settings); this.authenticatorChain = new AuthenticatorChain( settings, operatorPrivilegesService, anonymousUser, new AuthenticationContextSerializer(), + tracer, new ServiceAccountAuthenticator(serviceAccountService, nodeName, meterRegistry), new OAuth2TokenAuthenticator(tokenService, meterRegistry), new PluggableApiKeyAuthenticator(customApiKeyAuthenticator), diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/Authenticator.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/Authenticator.java index ed218c3eba58d..9d73db88842b7 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/Authenticator.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/Authenticator.java @@ -9,10 +9,13 @@ import org.elasticsearch.ElasticsearchSecurityException; import org.elasticsearch.action.ActionListener; +import org.elasticsearch.common.Randomness; import org.elasticsearch.common.Strings; +import org.elasticsearch.common.UUIDs; import org.elasticsearch.common.settings.SecureString; import org.elasticsearch.common.util.concurrent.ThreadContext; import org.elasticsearch.core.Nullable; +import org.elasticsearch.telemetry.tracing.Traceable; import org.elasticsearch.xpack.core.security.authc.Authentication; import org.elasticsearch.xpack.core.security.authc.AuthenticationResult; import org.elasticsearch.xpack.core.security.authc.AuthenticationToken; @@ -90,7 +93,7 @@ static SecureString extractApiKeyFromHeader(ThreadContext threadContext) { * the next {@link Authenticator} is tried. * The extracted tokens are all appended with {@link #addAuthenticationToken(AuthenticationToken)}. */ - class Context implements Closeable { + class Context implements Closeable, Traceable { private final ThreadContext threadContext; private final AuthenticationService.AuditableRequest request; private final User fallbackUser; @@ -104,6 +107,7 @@ class Context implements Closeable { private SecureString apiKeyString = null; private List defaultOrderedRealmList = null; private List unlicensedRealms = null; + private String requestId; /** * Context constructor that provides the authentication token directly as an argument. @@ -129,6 +133,7 @@ class Context implements Closeable { // if handleNullToken is false, fallbackUser and allowAnonymous are irrelevant this.fallbackUser = null; this.allowAnonymous = false; + this.requestId = UUIDs.randomBase64UUID(Randomness.get());; } /** @@ -149,6 +154,7 @@ public Context( this.fallbackUser = fallbackUser; this.allowAnonymous = allowAnonymous; this.realms = realms; + this.requestId = UUIDs.randomBase64UUID(Randomness.get());; } public ThreadContext getThreadContext() { @@ -241,5 +247,10 @@ public void addUnsuccessfulMessageToMetadata(final ElasticsearchSecurityExceptio ese.addMetadata("es.additional_unsuccessful_credentials", getUnsuccessfulMessages()); } } + + @Override + public String getSpanId() { + return requestId; + } } } diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/AuthenticatorChain.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/AuthenticatorChain.java index f3532dc4c6270..edba594fdf13a 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/AuthenticatorChain.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/AuthenticatorChain.java @@ -14,6 +14,7 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.concurrent.ThreadContext; import org.elasticsearch.node.Node; +import org.elasticsearch.telemetry.tracing.Tracer; import org.elasticsearch.xpack.core.common.IteratingActionListener; import org.elasticsearch.xpack.core.security.authc.Authentication; import org.elasticsearch.xpack.core.security.authc.AuthenticationResult; @@ -26,6 +27,7 @@ import org.elasticsearch.xpack.core.security.user.User; import org.elasticsearch.xpack.security.operator.OperatorPrivileges.OperatorPrivilegesService; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.function.BiConsumer; @@ -46,12 +48,14 @@ class AuthenticatorChain { private final AuthenticationContextSerializer authenticationSerializer; private final RealmsAuthenticator realmsAuthenticator; private final List allAuthenticators; + private final Tracer tracer; AuthenticatorChain( Settings settings, OperatorPrivilegesService operatorPrivilegesService, AnonymousUser anonymousUser, AuthenticationContextSerializer authenticationSerializer, + Tracer tracer, ServiceAccountAuthenticator serviceAccountAuthenticator, OAuth2TokenAuthenticator oAuth2TokenAuthenticator, PluggableApiKeyAuthenticator pluggableApiKeyAuthenticator, @@ -65,6 +69,7 @@ class AuthenticatorChain { this.isAnonymousUserEnabled = AnonymousUser.isAnonymousEnabled(settings); this.authenticationSerializer = authenticationSerializer; this.realmsAuthenticator = realmsAuthenticator; + this.tracer = tracer; this.allAuthenticators = List.of( serviceAccountAuthenticator, oAuth2TokenAuthenticator, @@ -123,7 +128,7 @@ private void doAuthenticate(Authenticator.Context context, ActionListener>> getAuthenticatorConsumer( - Authenticator.Context context + Authenticator.Context context, Tracer tracer ) { return (authenticator, listener) -> { if (context.shouldExtractCredentials()) { @@ -171,7 +176,7 @@ private static BiConsumer { + ActionListener> authResultListener = ActionListener.wrap(result -> { if (result.getStatus() == AuthenticationResult.Status.TERMINATE) { onFailure.accept(result.getException()); return; @@ -180,10 +185,22 @@ private static BiConsumer> afterAuthResultListener = ActionListener.runAfter(authResultListener, + () -> tracer.stopTrace(context)); + + tracer.startTrace(context.getThreadContext(), context, "authenticate", addUsefulMetadata(authenticator)); + authenticator.authenticate(context, afterAuthResultListener); }; } + private static Map addUsefulMetadata(Authenticator authenticator) { + Map attribs = new HashMap<>(); + attribs.put("security.authenticator.type", authenticator.name()); + return attribs; + } + // Package private for test void maybeLookupRunAsUser(Authenticator.Context context, Authentication authentication, ActionListener listener) { if (false == runAsEnabled) { diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/AuthenticatorChainTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/AuthenticatorChainTests.java index bfd122655768b..67e72215b87ae 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/AuthenticatorChainTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/AuthenticatorChainTests.java @@ -19,6 +19,7 @@ import org.elasticsearch.common.util.concurrent.ThreadContext; import org.elasticsearch.core.Tuple; import org.elasticsearch.node.Node; +import org.elasticsearch.telemetry.tracing.Tracer; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.MockLog; import org.elasticsearch.xpack.core.security.action.apikey.ApiKey; @@ -103,6 +104,7 @@ public void init() { operatorPrivilegesService, anonymousUser, authenticationContextSerializer, + Tracer.NOOP, serviceAccountAuthenticator, oAuth2TokenAuthenticator, pluggableApiKeyAuthenticator, From 46c6ee995e20fcaeda7fd01764a1d92be02495e5 Mon Sep 17 00:00:00 2001 From: ankitsethi Date: Thu, 21 Aug 2025 18:12:32 -0500 Subject: [PATCH 2/3] better sounding name --- .../elasticsearch/xpack/security/authc/Authenticator.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/Authenticator.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/Authenticator.java index 9d73db88842b7..93e678bba1426 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/Authenticator.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/Authenticator.java @@ -107,7 +107,7 @@ class Context implements Closeable, Traceable { private SecureString apiKeyString = null; private List defaultOrderedRealmList = null; private List unlicensedRealms = null; - private String requestId; + private String contextId; /** * Context constructor that provides the authentication token directly as an argument. @@ -133,7 +133,7 @@ class Context implements Closeable, Traceable { // if handleNullToken is false, fallbackUser and allowAnonymous are irrelevant this.fallbackUser = null; this.allowAnonymous = false; - this.requestId = UUIDs.randomBase64UUID(Randomness.get());; + this.contextId = UUIDs.randomBase64UUID(Randomness.get()); } /** @@ -154,7 +154,7 @@ public Context( this.fallbackUser = fallbackUser; this.allowAnonymous = allowAnonymous; this.realms = realms; - this.requestId = UUIDs.randomBase64UUID(Randomness.get());; + this.contextId = UUIDs.randomBase64UUID(Randomness.get()); } public ThreadContext getThreadContext() { @@ -250,7 +250,7 @@ public void addUnsuccessfulMessageToMetadata(final ElasticsearchSecurityExceptio @Override public String getSpanId() { - return requestId; + return contextId; } } } From af24ebf8c6df5e1bd2c5798e1b6760000c8cca31 Mon Sep 17 00:00:00 2001 From: elasticsearchmachine Date: Thu, 21 Aug 2025 23:19:49 +0000 Subject: [PATCH 3/3] [CI] Auto commit changes from spotless --- .../telemetry/apm/internal/APMAgentSettings.java | 4 +--- .../xpack/security/authc/AuthenticatorChain.java | 9 ++++++--- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/modules/apm/src/main/java/org/elasticsearch/telemetry/apm/internal/APMAgentSettings.java b/modules/apm/src/main/java/org/elasticsearch/telemetry/apm/internal/APMAgentSettings.java index c5760e5aef94d..76d2c366d01b5 100644 --- a/modules/apm/src/main/java/org/elasticsearch/telemetry/apm/internal/APMAgentSettings.java +++ b/modules/apm/src/main/java/org/elasticsearch/telemetry/apm/internal/APMAgentSettings.java @@ -286,9 +286,7 @@ private static Setting concreteAgentSetting(String namespace, String qua ); // not a setting to reduce the footprint of this change - public static final List TELEMETRY_TRACING_SANITIZE_FIELD_NAMES_EXCLUDES = List.of( - "security.authenticator.type" - ); + public static final List TELEMETRY_TRACING_SANITIZE_FIELD_NAMES_EXCLUDES = List.of("security.authenticator.type"); public static final Setting TELEMETRY_TRACING_ENABLED_SETTING = Setting.boolSetting( TELEMETRY_SETTING_PREFIX + "tracing.enabled", diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/AuthenticatorChain.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/AuthenticatorChain.java index edba594fdf13a..af604edde66b3 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/AuthenticatorChain.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/AuthenticatorChain.java @@ -138,7 +138,8 @@ private void doAuthenticate(Authenticator.Context context, ActionListener>> getAuthenticatorConsumer( - Authenticator.Context context, Tracer tracer + Authenticator.Context context, + Tracer tracer ) { return (authenticator, listener) -> { if (context.shouldExtractCredentials()) { @@ -187,8 +188,10 @@ private static BiConsumer> afterAuthResultListener = ActionListener.runAfter(authResultListener, - () -> tracer.stopTrace(context)); + ActionListener> afterAuthResultListener = ActionListener.runAfter( + authResultListener, + () -> tracer.stopTrace(context) + ); tracer.startTrace(context.getThreadContext(), context, "authenticate", addUsefulMetadata(authenticator)); authenticator.authenticate(context, afterAuthResultListener);