singhpk234 commented on code in PR #14360:
URL: https://github.com/apache/iceberg/pull/14360#discussion_r2649565837


##########
core/src/main/java/org/apache/iceberg/rest/HTTPClient.java:
##########
@@ -536,6 +540,11 @@ public Builder withAuthSession(AuthSession session) {
       return this;
     }
 
+    public Builder withOpenTelemetry(OpenTelemetry openTelemetry) {

Review Comment:
   do we need to protect this with a precondition of this being not null ? 



##########
gradle/libs.versions.toml:
##########
@@ -78,6 +78,7 @@ mockserver = "5.15.0"
 nessie = "0.106.0"
 netty-buffer = "4.2.9.Final"
 object-client-bundle = "3.3.2"
+opentelemetry-httpclient = "2.20.1-alpha"

Review Comment:
   please check on the licence part, if we are ok and if we need to update it.



##########
core/src/main/java/org/apache/iceberg/rest/HTTPClient.java:
##########
@@ -113,13 +114,15 @@ private HTTPClient(
       ObjectMapper objectMapper,
       Map<String, String> properties,
       HttpClientConnectionManager connectionManager,
-      AuthSession session) {
+      AuthSession session,
+      OpenTelemetry openTelemetry) {
     this.baseUri = baseUri;
     this.baseHeaders = baseHeaders;
     this.mapper = objectMapper;
     this.authSession = session;
 
-    HttpClientBuilder clientBuilder = HttpClients.custom();
+    HttpClientBuilder clientBuilder =
+        
ApacheHttpClientTelemetry.builder(openTelemetry).build().newHttpClientBuilder();

Review Comment:
   can we only set openTelemetry when there is something set via builder, i 
understand default value is NoOp but it would still add interceptor to call of 
HTTP but i guess it will do nothing, having set by builder would  make sure its 
only added when required, WDYT ? 
   
   ```suggestion
           openTelemetry ? 
ApacheHttpClientTelemetry.builder(openTelemetry).build().newHttpClientBuilder() 
: HttpClients.custom();
   ```
   



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