Skip to content

RATIS-2393 Add Span Context to RaftRpcRequestProto#1341

Open
taklwu wants to merge 2 commits intoapache:masterfrom
taklwu:RATIS-2393
Open

RATIS-2393 Add Span Context to RaftRpcRequestProto#1341
taklwu wants to merge 2 commits intoapache:masterfrom
taklwu:RATIS-2393

Conversation

@taklwu
Copy link

@taklwu taklwu commented Feb 4, 2026

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

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
Copy link
Author

taklwu commented Feb 4, 2026

@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)


Copy link
Contributor

@szetszwo szetszwo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

uint64 timeoutMs = 13;
RoutingTableProto routingTable = 14;
SlidingWindowEntry slidingWindowEntry = 15;
SpanContextProto spanContext = 16;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should not use 16; see https://protobuf.dev/programming-guides/proto3/#assigning :

You should use the field numbers 1 through 15 for the most-frequently-set fields. Lower field number values take less space in the wire format. ...

Let's use 11?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants