Copilot commented on code in PR #10569:
URL: https://github.com/apache/ozone/pull/10569#discussion_r3482919423


##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/tracing/TracingUtil.java:
##########
@@ -95,13 +97,52 @@ public static synchronized void reconfigureTracing(
     initTracing(serviceName, tracingConfig);
   }
 
+  /**
+   * Drain the BatchSpanProcessor queue without shutting down.
+   * Call from short-lived CLIs before the JVM exits.
+   */
+  public static synchronized void flushTracing() {
+    if (batchSpanProcessor == null) {
+      return;
+    }
+    try {
+      // Best-effort: wait up to 10s for span export; remaining spans may be 
dropped on exit.
+      batchSpanProcessor.forceFlush().join(10, TimeUnit.SECONDS);
+    } catch (Exception e) {
+      LOG.warn("Tracing flush: forceFlush failed", e);
+    }

Review Comment:
   flushTracing() currently ignores the boolean result of 
forceFlush().join(timeout). If the flush times out (or otherwise returns 
false), the CLI will exit without any signal that spans may still be 
buffered/dropped. Logging a warning on timeout would make tracing reliability 
issues diagnosable in the field.



##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/tracing/TracingUtil.java:
##########
@@ -119,7 +160,7 @@ private static void initialize(String serviceName, 
TracingConfig tracingConfig)
         .setEndpoint(otelEndPoint)
         .build();
 
-    SimpleSpanProcessor spanProcessor = 
SimpleSpanProcessor.builder(spanExporter).build();
+    batchSpanProcessor = BatchSpanProcessor.builder(spanExporter).build();

Review Comment:
   BatchSpanProcessor buffers spans, so on JVM/service shutdown any spans still 
in the queue may be lost unless the tracer provider is explicitly flushed/shut 
down. This PR adds a CLI-only flush via TracingUtil.execute(...), but 
long-lived services (OM/SCM/DN, gateway) still only call initTracing(...) and 
never trigger a final flush/shutdown via this utility. Consider registering a 
shutdown hook (preferably via ShutdownHookManager) to call shutdown/flush when 
tracing is enabled, to avoid losing the last spans on graceful process 
termination.



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