RATIS-2393 Add Span Context to RaftRpcRequestProto

[taklwu]

What changes were proposed in this pull request?

We're adding a new map to RaftRpcRequestProto that will be used for upcoming Opentelemetry integration.

see the usage of PoC https://github.com/taklwu/ratis/blob/opentelemetry0129/ratis-common/src/main/java/org/apache/ratis/trace/TraceUtils.java#L235-L251

Another official reference for how this map is going to be inject and extract from the http / rpc header

• https://opentelemetry.io/docs/concepts/context-propagation/
• https://opentelemetry.io/docs/specs/otel/context/api-propagators/#textmap-propagator

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/RATIS-2393

How was this patch tested?

running mvn clean package -DskipTests

[taklwu]

@szetszwo Hi Nicholas, should I directly open PRs to master branch ?

in addition , the test failure doesn't seem to be related to my changes

[INFO] Running org.apache.ratis.netty.TestLeaderElectionWithNetty
OpenJDK 64-Bit Server VM warning: Sharing is only supported for boot loader classes because bootstrap classpath has been appended
Error:  Tests run: 25, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 74.14 s <<< FAILURE! -- in org.apache.ratis.netty.TestLeaderElectionWithNetty
Error:  org.apache.ratis.netty.TestLeaderElectionWithNetty.testChangeListenerToFollower -- Time elapsed: 1.509 s <<< FAILURE!
org.opentest4j.AssertionFailedError: expected: <0> but was: <1>
	at org.apache.ratis.server.impl.LeaderElectionTests.testChangeListenerToFollower(LeaderElectionTests.java:562)
	at java.base/java.lang.reflect.Method.invoke(Method.java:569)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)

[szetszwo]

@taklwu , thanks for working on this! Please see the comment inlined.

BTW, this change is very small. Let's combine the usage of the new SpanContextProto such as RATIS-2395 or some part of it. Otherwise, it is hard to determine whether this change is good or not.

[OneSizeFitsQuorum]

@taklwu Please add license for the failed ci!

[taklwu]

@OneSizeFitsQuorum I have updated the license and good catch.

[szetszwo]

@taklwu , thanks for the update!

Please see the comments inlined and also https://issues.apache.org/jira/secure/attachment/13081062/1341_review.patch

[taklwu]

sorry that I rebased on top of master and messed up the review history, but the last commit is the only change for addressing the recent comments by @szetszwo .

[szetszwo]

@taklwu , thanks for the update! Please see the comments inlined.

[szetszwo]

We already have a similar method

• ratis/ratis-common/src/main/java/org/apache/ratis/util/JavaUtils.java

  Line 289 in 3d9f5af

  static Throwable unwrapCompletionException(Throwable t) {

Let's reuse it and move the other methods to JavaUtils.

[szetszwo]

What do you think about this comment?

[taklwu]

reusing existing method is good, sorry that I didn't know unwrapCompletionException was there.

[szetszwo]

We should use lambda and not to implement functional interface in general. See also https://www.oracle.com/technical-resources/articles/java/lambda.html

[taklwu]

I will give a try, it may take a bit.

but the idea was from the hbase implementation that few span builders was created.

[szetszwo]

Sorry that I may not be clear in my last comment -- we should import io.opentelemetry only in ratis-common. Then, it will be easier to change the tracing dependency to be pluggable in the future.

We often see that a library may introduce incompatible changes (such as Dropwizard Metrics v3 vs v4). Since Ratis itself is also a library, we don't want to force our consumer applications to use a particular dependency (e.g. Ratis supports both Dropwizard Metrics v3 and v4 by making it pluggable). Another reason is that a dependency may have CVEs (e.g. the infamous Log4Shell case). Applications may choose to disable it.

BTW, we should add a conf to enable/disable tracing.