HDDS-14814. Unify Fragmented Traces for Freon randomkeys Command#9957
HDDS-14814. Unify Fragmented Traces for Freon randomkeys Command#9957sravani-revuri wants to merge 3 commits intoapache:masterfrom
Conversation
Gargi-jais11
left a comment
There was a problem hiding this comment.
Thanks @sravani-revuri for the patch.
Left few comments.
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/tracing/TracingUtil.java
Outdated
Show resolved
Hide resolved
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/tracing/TracingUtil.java
Show resolved
Hide resolved
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/tracing/TracingUtil.java
Outdated
Show resolved
Hide resolved
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/tracing/TracingUtil.java
Outdated
Show resolved
Hide resolved
sumitagrawl
left a comment
There was a problem hiding this comment.
@sravani-revuri thanks for working over this, few comments given
| spanSamplingConfig = spanStrConfig; | ||
| } | ||
| } catch (Exception ex) { | ||
| // ignore and use the default value. |
There was a problem hiding this comment.
need log warn for exception message and default behavior getting executed
| private static Map<String, LoopSampler> parseSpanSamplingConfig(String configStr) { | ||
| Map<String, LoopSampler> result = new HashMap<>(); | ||
| if (configStr == null || configStr.isEmpty()) { | ||
| return result; |
There was a problem hiding this comment.
can return Collection.emptyMap()
|
|
||
| try { | ||
| // Long.parseLong strictly rejects decimals (throws NumberFormatException) | ||
| long interval = Long.parseLong(val); |
There was a problem hiding this comment.
take value as %cent and convert to sampling to unify with other configuration, %cent is easy configuration
| result.put(name, new LoopSampler(interval)); | ||
|
|
||
| } catch (NumberFormatException e) { | ||
| LOG.error("Invalid ratio '{}' for span '{}': decimals not allowed", val, name); |
There was a problem hiding this comment.
update error log with addition info, as, ignoring sample configuration.
| String currentSpanContext = TracingUtil.exportCurrentSpan(); | ||
| for (int i = 0; i < numOfThreads; i++) { | ||
| executor.execute(new ObjectCreator()); | ||
| executor.execute(new ObjectCreator(currentSpanContext)); |
There was a problem hiding this comment.
We can pass direct Current span, and test if works.
| 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"; |
There was a problem hiding this comment.
this can be renamed as, OTEL_SPAN_SAMPLER_ARG
What changes were proposed in this pull request?
Tracing for Freon randomkeys is currently fragmented and has the following issues:
Proposed Changes:
Proper handling of spans is required to ensure a chain of spans is created for the same trace.
Implement span level sampling through custom sampler and enable it through OTEL_SPAN_SAMPLING.
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-14814
How was this patch tested?
Manual Testing through Jaeger UI and unit tests.
Running Regular Freon Command
Command:
Output:
Running Span Sampling only
Command:
Output:
1 CreateVolume call:

2 CreateBucket calls:

1 CreateKey call:

Running Span Sampling with Trace Sampling
Command:
Output:
8 traces visible when command is run 16 times.
without sampling 180 spans are generated.