sumitagrawl commented on code in PR #9957:
URL: https://github.com/apache/ozone/pull/9957#discussion_r2979506018


##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/tracing/TracingUtil.java:
##########
@@ -177,6 +202,41 @@ public static boolean isTracingEnabled(
         ScmConfigKeys.HDDS_TRACING_ENABLED_DEFAULT);
   }
 
+  /** Function to parse span sampling config. The input is in the form 
<span_name>:<sample_rate>.
+   * The sample rate must be a natural number (1,2,3). Any value other than 
that will LOG an error.
+   * */
+  private static Map<String, LoopSampler> parseSpanSamplingConfig(String 
configStr) {
+    Map<String, LoopSampler> result = new HashMap<>();
+    if (configStr == null || configStr.isEmpty()) {
+      return result;

Review Comment:
   can return Collection.emptyMap()



##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/tracing/TracingUtil.java:
##########
@@ -177,6 +202,41 @@ public static boolean isTracingEnabled(
         ScmConfigKeys.HDDS_TRACING_ENABLED_DEFAULT);
   }
 
+  /** Function to parse span sampling config. The input is in the form 
<span_name>:<sample_rate>.
+   * The sample rate must be a natural number (1,2,3). Any value other than 
that will LOG an error.
+   * */
+  private static Map<String, LoopSampler> parseSpanSamplingConfig(String 
configStr) {
+    Map<String, LoopSampler> result = new HashMap<>();
+    if (configStr == null || configStr.isEmpty()) {
+      return result;
+    }
+
+    for (String entry : configStr.split(",")) {
+      String trimmed = entry.trim();
+      int colon = trimmed.indexOf(':');
+      if (colon <= 0 || colon >= trimmed.length() - 1) {
+        continue;
+      }
+
+      String name = trimmed.substring(0, colon).trim();
+      String val = trimmed.substring(colon + 1).trim();
+
+      try {
+        // Long.parseLong strictly rejects decimals (throws 
NumberFormatException)
+        long interval = Long.parseLong(val);
+
+        // LoopSampler constructor strictly rejects <= 0 (throws 
IllegalArgumentException)
+        result.put(name, new LoopSampler(interval));
+
+      } catch (NumberFormatException e) {
+        LOG.error("Invalid ratio '{}' for span '{}': decimals not allowed", 
val, name);

Review Comment:
   update error log with addition info, as, `ignoring sample configuration`.



##########
hadoop-ozone/freon/src/main/java/org/apache/hadoop/ozone/freon/RandomKeyGenerator.java:
##########
@@ -333,8 +333,10 @@ public Void call() throws Exception {
     LOG.info("validateWrites : {}", validateWrites);
     LOG.info("Number of Validate Threads: {}", numOfValidateThreads);
     LOG.info("cleanObjects : {}", cleanObjects);
+
+    String currentSpanContext = TracingUtil.exportCurrentSpan();
     for (int i = 0; i < numOfThreads; i++) {
-      executor.execute(new ObjectCreator());
+      executor.execute(new ObjectCreator(currentSpanContext));

Review Comment:
   We can pass direct Current span, and test if works.



##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/tracing/TracingUtil.java:
##########
@@ -93,16 +95,39 @@ private static void initialize(String serviceName) {
       // ignore and use the default value.
     }
 
+    String spanSamplingConfig = OTEL_TRACES_SAMPLER_CONFIG_DEFAULT;
+    try {
+      String spanStrConfig = System.getenv(OTEL_SPAN_SAMPLING);
+      if (spanStrConfig != null && !spanStrConfig.isEmpty()) {
+        spanSamplingConfig = spanStrConfig;
+      }
+    } catch (Exception ex) {
+      // ignore and use the default value.

Review Comment:
   need log warn for exception message and default behavior getting executed



##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/tracing/TracingUtil.java:
##########
@@ -177,6 +202,41 @@ public static boolean isTracingEnabled(
         ScmConfigKeys.HDDS_TRACING_ENABLED_DEFAULT);
   }
 
+  /** Function to parse span sampling config. The input is in the form 
<span_name>:<sample_rate>.
+   * The sample rate must be a natural number (1,2,3). Any value other than 
that will LOG an error.
+   * */
+  private static Map<String, LoopSampler> parseSpanSamplingConfig(String 
configStr) {
+    Map<String, LoopSampler> result = new HashMap<>();
+    if (configStr == null || configStr.isEmpty()) {
+      return result;
+    }
+
+    for (String entry : configStr.split(",")) {
+      String trimmed = entry.trim();
+      int colon = trimmed.indexOf(':');
+      if (colon <= 0 || colon >= trimmed.length() - 1) {
+        continue;
+      }
+
+      String name = trimmed.substring(0, colon).trim();
+      String val = trimmed.substring(colon + 1).trim();
+
+      try {
+        // Long.parseLong strictly rejects decimals (throws 
NumberFormatException)
+        long interval = Long.parseLong(val);

Review Comment:
   take value as %cent and convert to sampling to unify with other 
configuration, %cent is easy configuration



##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/tracing/TracingUtil.java:
##########
@@ -52,6 +52,8 @@ public final class TracingUtil {
   private static final String OTEL_EXPORTER_OTLP_ENDPOINT_DEFAULT = 
"http://localhost:4317";;
   private static final String OTEL_TRACES_SAMPLER_ARG = 
"OTEL_TRACES_SAMPLER_ARG";
   private static final double OTEL_TRACES_SAMPLER_RATIO_DEFAULT = 1.0;
+  private static final String OTEL_SPAN_SAMPLING = "OTEL_SPAN_SAMPLING";

Review Comment:
   this can be renamed as, OTEL_SPAN_SAMPLER_ARG



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