obelix74 commented on code in PR #3414:
URL: https://github.com/apache/polaris/pull/3414#discussion_r2692164984


##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/io/StorageAccessConfigProvider.java:
##########
@@ -187,6 +193,62 @@ private CredentialVendingContext 
buildCredentialVendingContext(
       builder.activatedRoles(Optional.of(rolesString));
     }
 
+    // Only include trace ID when the feature flag is enabled.
+    // When enabled, trace IDs are included in AWS STS session tags and become 
part of the
+    // credential cache key (since they affect the vended credentials).
+    // When disabled (default), trace IDs are not included, allowing efficient 
credential
+    // caching across requests with different trace IDs.
+    boolean includeTraceIdInSessionTags =
+        storageCredentialsVendor
+            .getRealmConfig()
+            .getConfig(FeatureConfiguration.INCLUDE_TRACE_ID_IN_SESSION_TAGS);
+    if (includeTraceIdInSessionTags) {
+      builder.traceId(getCurrentTraceId());
+    }
+
+    // Only include request ID when the feature flag is enabled.
+    // When enabled, request IDs are included in AWS STS session tags and 
become part of the
+    // credential cache key (since they affect the vended credentials).
+    // When disabled (default), request IDs are not included, allowing 
efficient credential
+    // caching across requests with different request IDs.
+    boolean includeRequestIdInSessionTags =
+        storageCredentialsVendor
+            .getRealmConfig()
+            
.getConfig(FeatureConfiguration.INCLUDE_REQUEST_ID_IN_SESSION_TAGS);
+    if (includeRequestIdInSessionTags) {
+      builder.requestId(getCurrentRequestId());
+    }
+
     return builder.build();
   }
+
+  /**
+   * Extracts the current OpenTelemetry trace ID from the active span context.
+   *
+   * @return the trace ID if a valid span context exists, empty otherwise
+   */
+  private Optional<String> getCurrentTraceId() {
+    SpanContext spanContext = Span.current().getSpanContext();
+    if (spanContext.isValid()) {
+      return Optional.of(spanContext.getTraceId());
+    }
+    return Optional.empty();
+  }
+
+  /**
+   * Extracts the current request ID from the active RESTEasy Reactive request 
context.
+   *
+   * <p>Note: avoid injecting ContainerRequestContext here because some tests 
may run with no active
+   * request scope.
+   */
+  private Optional<String> getCurrentRequestId() {
+    // See org.jboss.resteasy.reactive.server.injection.ContextProducers
+    ResteasyReactiveRequestContext context = CurrentRequestManager.get();

Review Comment:
   Can I file another feature request to wire the request IDs through a CDI 
pattern similar to realm IDs and fix it cleanly in that PR?  For this PR, I can 
roll back this last commit and merge it with just the trace_id. Once we have a 
reliable way to inject request_ids, we can add it to the STS tags then. 



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to