New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
HDDS-3399. Update JaegerTracing #835
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @elek for working on this. I have tested a few operations using Freon, S3 gateway, Shell, S3 Shell. I have found that span.finish()
is missing in 2 places, which breaks span hierarchy (eg. writechunk on datanode at root level, instead of under writechunk in freon). After adding the missing finish()
calls, all traces looked good.
Span span = GlobalTracer.get() | ||
.buildSpan(spanName).start(); | ||
try (Scope scope = GlobalTracer.get().activateSpan(span)) { | ||
return supplier.get(); | ||
} catch (Exception ex) { | ||
span.setTag("failed", true); | ||
throw ex; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Span should be finished.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks to test it @adoroszlai . I didn't notice any problem with the spans when I tested, but yes, it seems to be missing. Added them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
Thanks the review @adoroszlai I am merging it now (rebased checked with CI and got green light). |
What changes were proposed in this pull request?
We currently use JaegerTracing 0.34.0. The latest is 1.2.0. We are several versions behind and should update. Note this update requires the latest version fo OpenTracing and has several breaking changes.
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-3399
How was this patch tested?
Executed freon + s3 crete bucket commands
Traces were displayed in jaeger.