wombatu-kun commented on code in PR #16235:
URL: https://github.com/apache/iceberg/pull/16235#discussion_r3361963927


##########
aws/src/main/java/org/apache/iceberg/aws/s3/S3FileIOProperties.java:
##########
@@ -1197,4 +1209,58 @@ private <T> T loadSdkPluginConfigurations(String impl, 
Map<String, String> prope
   public Map<String, String> properties() {
     return allProperties;
   }
+
+  public String metricsPublisherImpl() {
+    return metricsPublisherImpl;
+  }
+
+  /**
+   * Configure a custom {@link MetricPublisher} for an S3 client.
+   *
+   * <p>Sample usage:
+   *
+   * <pre>
+   *     
S3Client.builder().applyMutation(s3FileIOProperties::applyMetricsPublisherConfiguration)
+   * </pre>
+   */
+  public <T extends S3BaseClientBuilder<T, ?>> void 
applyMetricsPublisherConfiguration(T builder) {
+    if (metricsPublisherImpl != null) {
+      builder.overrideConfiguration(c -> 
c.addMetricPublisher(loadMetricPublisher()));

Review Comment:
   The `overrideConfiguration(c -> ...)` Consumer overload builds a fresh 
`ClientOverrideConfiguration` (the SdkClientBuilder default does 
`ClientOverrideConfiguration.builder().applyMutation(consumer).build()`), so it 
discards any override config already set earlier in the chain instead of adding 
to it. On the sync `s3()` path this `applyMutation` runs last, after 
`applySignerConfiguration`, `applyUserAgentConfigurations`, and 
`applyRetryConfigurations` (DefaultS3FileIOAwsClientFactory.java:57-60), so 
configuring a metrics publisher silently drops the tuned S3 retry policy, the 
user-agent prefix, and the remote-signing signer.
   
   Seed from the existing config like the sibling methods 
(S3FileIOProperties.java:1088-1091, 1163-1166) and the predecessor #15122 did:
   
   ```
   ClientOverrideConfiguration.Builder configBuilder =
       builder.overrideConfiguration() != null
           ? builder.overrideConfiguration().toBuilder()
           : ClientOverrideConfiguration.builder();
   
builder.overrideConfiguration(configBuilder.addMetricPublisher(loadMetricPublisher()).build());
   ```



-- 
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]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to