[CURATOR-695] Open Telemetry tracing #494
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 <[email protected]>
|
Any thoughts on how this looks @eolivelli ? |
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 <[email protected]>
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
| @@ -26,11 +26,11 @@ | |||
| <parent> | |||
| <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 ?
|
I have asked for review on the mailing list |
|
also please check CI |
| */ | ||
| 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()); |
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.