Conversation
This changes adds an Open Telemetry tracing driver. To make this more effective, it also decomposes OperationTrace events so that the start and end of the event can be managed separately.
This changes adds an Open Telemetry tracing driver. To make this more effective, it also decomposes OperationTrace events so that the start and end of the event can be managed separately.
Co-authored-by: shenjianeng <sjn@dxy.cn>
|
Any thoughts on how this looks @eolivelli ? |
eolivelli
left a comment
eolivelli
left a comment
There was a problem hiding this comment.
Nice work.
I left some comment about files that we don't need.
Apart from that I think it is better to also add a test case that uses some basic curator functions and verify that otel tracks them
curator-drivers/LICENSE
Outdated
| @@ -0,0 +1,202 @@ | |||
|
|
|||
| Apache License | |||
curator-drivers/NOTICE
Outdated
| @@ -0,0 +1,5 @@ | |||
| Apache Curator | |||
| @@ -0,0 +1,202 @@ | |||
|
|
|||
| Apache License | |||
| @@ -0,0 +1,5 @@ | |||
| Apache Curator | |||
| <dependency> | ||
| <groupId>org.apache.curator</groupId> | ||
| <artifactId>curator-client</artifactId> | ||
| <version>${project.version}</version> |
There was a problem hiding this comment.
This should be at scope 'provided'
| @@ -0,0 +1,5 @@ | |||
| Apache Curator | |||
There was a problem hiding this comment.
The target directory must not be committed
Signed-off-by: tison <wander4096@gmail.com>
This changes adds an Open Telemetry tracing driver. To make this more effective, it also decomposes OperationTrace events so that the start and end of the event can be managed separately.
# Conflicts: # curator-client/src/main/java/org/apache/curator/drivers/OperationTrace.java # curator-drivers/open-telemetry/pom.xml # curator-drivers/open-telemetry/src/main/java/org/apache/curator/drivers/opentelemetry/OpenTelemetryTracingDriver.java # curator-drivers/pom.xml
| <groupId>org.apache.curator</groupId> | ||
| <artifactId>apache-curator</artifactId> | ||
| <version>5.5.1-SNAPSHOT</version> | ||
| <version>5.6.1-SNAPSHOT</version> |
There was a problem hiding this comment.
we should bump the version in a separate commit, can you please revert ?
eolivelli
left a comment
eolivelli
left a comment
There was a problem hiding this comment.
Overall LGTM
but please revert the change to the pom versions
|
I have asked for review on the mailing list |
|
also please check CI |
kezhuw
left a comment
kezhuw
left a comment
There was a problem hiding this comment.
I like the direction and attach my comments inline.
| * @return The internal trace representation of the driver implementation. Driver dependent, may be {@code null}. | ||
| */ | ||
| public abstract void addTrace(OperationTrace trace); | ||
| public abstract Object startTrace(OperationTrace trace); |
There was a problem hiding this comment.
It would be better to introduce a new sub-class/sub-interface of TracerDriver so we won't break third-party usage of AdvancedTracerDriver.
| } | ||
| // We will close this in endTrace | ||
| @SuppressWarnings("MustBeClosedChecker") | ||
| Scope scope = span.makeCurrent(); |
There was a problem hiding this comment.
Curator invokes callbacks asynchronously.
The timing of callback invocation is indeterministic. Span::makeCurrent in startTrace and Scope::close in endTrace probably will not form try-with-resources as suggested in javadoc of Span::makeCurrent.
Also, endTrace is probably not invoked in thread where startTrace called as far as I known. Given CuratorFramework::getChildren as an example, startTrace is invoked in Curator threads while endTrace is invoked in ZooKeeper thread.
Since startTrace and endTrace are the only interceptions, I think we can drop scope/context propagation here.
| ); | ||
| } | ||
|
|
||
| span.setAttribute(LATENCY_MS_ATTRIBUTE, trace.getRequestBytesLength()); |
5 participants
This changes adds an Open Telemetry tracing driver. To make this more effective, it also decomposes
OperationTraceevents so that the start and end of the event can be handled distinctly.Curator's
OperationTracesare mapped to OTelSpans. When anOperationTraceis started (created), a corresponding OTel span is started and made current. The span is closed when theOperationTraceis committed.OperationTracemembers are mapped to OTel span attributes, and result codes are mapped to appropriate OTel representations.Any Curator
EventTracesare attributed to the current OTel span.