RATIS-2393 Add Span Context to RaftRpcRequestProto by taklwu · Pull Request #1341 · apache/ratis

taklwu · 2026-02-04T21:16:55Z

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 · 2026-02-04T23:34:38Z

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

ratis-proto/src/main/proto/Raft.proto

OneSizeFitsQuorum · 2026-02-16T10:28:56Z

@taklwu Please add license for the failed ci!

taklwu · 2026-03-03T16:23:16Z

@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

pom.xml

ratis-common/src/main/java/org/apache/ratis/protocol/RaftClientRequest.java

ratis-common/src/main/java/org/apache/ratis/trace/RatisAttributes.java

ratis-common/src/main/java/org/apache/ratis/trace/TraceUtils.java

ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java

- First commit of using OpenTelemetry - use junit5 and default opentelemetry extension in test case. - Add test to mock client level span and inject when request is being sent - only include the server extract

1. use traceAsyncMethod 2. add ClientInvocationId 3. and other places

taklwu · 2026-03-05T19:34:25Z

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.

ratis-common/src/main/java/org/apache/ratis/util/FutureUtils.java

ratis-common/src/main/java/org/apache/ratis/trace/ClientRequestSpanBuilder.java

szetszwo · 2026-03-06T19:25:07Z

ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java

 */
 package org.apache.ratis.server.impl;

+import io.opentelemetry.api.trace.Span;


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.

- use lambda instead of implements supplier - move most the code into ratis-common

taklwu

@szetszwo can you please review it again ? thanks.

ratis-server/src/test/java/org/apache/ratis/server/impl/RaftServerImplTracingTests.java

taklwu · 2026-03-09T21:28:15Z

ratis-server/pom.xml

+      <groupId>io.opentelemetry</groupId>
+      <artifactId>opentelemetry-sdk</artifactId>
+      <scope>test</scope>
+    </dependency>
+    <dependency>
+      <groupId>io.opentelemetry</groupId>
+      <artifactId>opentelemetry-sdk-testing</artifactId>
+      <scope>test</scope>


unless I test differently, otherwise, I see these lines are required...

@taklwu , could you take a look https://issues.apache.org/jira/secure/attachment/13081062/1341_review.patch ? It does not add any dependencies to ratis-server.

ah, I missed that, I moved them all into ratis-common as transitive dependencies.

szetszwo · 2026-03-11T00:11:15Z

@taklwu , thanks for the update! I will review it in details tomorrow.

szetszwo

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

szetszwo · 2026-03-11T23:14:28Z

ratis-common/src/main/java/org/apache/ratis/trace/RatisAttributes.java

+  public static final AttributeKey<String> ATTR_CLIENT_INVOCATION_ID =
+      AttributeKey.stringKey("raft.client.invocation.id");


It should be "raft.client.id".

szetszwo · 2026-03-11T23:16:16Z

ratis-common/src/main/java/org/apache/ratis/trace/TraceConfigKeys.java

+import static org.apache.ratis.conf.ConfUtils.getBoolean;
+import static org.apache.ratis.conf.ConfUtils.setBoolean;
+
+public interface TraceConfigKeys {


Let's have this class temporarily. We should combine it with RaftServerConfigKyes.

at the beginning it was part of the RaftServerConfigKeys in ratis-server, but I was thinking to have this configuration on the client as well, such I moved it to the ratis-common.

anyhow, as you said let's keep it temporarily.

szetszwo · 2026-03-11T23:19:15Z

ratis-common/src/main/java/org/apache/ratis/trace/RatisAttributes.java

+  public static final AttributeKey<String> ATTR_CLIENT_INVOCATION_ID =
+      AttributeKey.stringKey("raft.client.invocation.id");
+  public static final AttributeKey<String> ATTR_MEMBER_ID = AttributeKey.stringKey("raft.member.id");
+  public static final AttributeKey<String> ATTR_CALL_ID = AttributeKey.stringKey("raft.call.id");


Since the fields are already in RatisAttributes, let remove the ATTR_ prefix, i.e.

RatisAttributes.MEMBER_ID

RatisAttributes.CLIENT_ID

RatisAttributes.CALL_ID

szetszwo

+1 the change looks good.

taklwu force-pushed the RATIS-2393 branch from c710d22 to 338208f Compare February 4, 2026 21:18

szetszwo reviewed Feb 7, 2026

View reviewed changes

ratis-proto/src/main/proto/Raft.proto Outdated Show resolved Hide resolved

taklwu force-pushed the RATIS-2393 branch from ddbe06e to 325f2fb Compare February 11, 2026 07:46

taklwu requested a review from szetszwo February 11, 2026 07:53

szetszwo reviewed Mar 3, 2026

View reviewed changes

taklwu added 7 commits March 5, 2026 08:38

RATIS-2393 Add Span Context to RaftRpcRequestProto

c5e0768

retry ci

e74de61

Introduce OpenTelemetry to Ratis

41f12c3

- First commit of using OpenTelemetry - use junit5 and default opentelemetry extension in test case. - Add test to mock client level span and inject when request is being sent - only include the server extract

minor fix

2894843

remove assertj

aba5903

add license

65cad61

Address comments

d097a31

1. use traceAsyncMethod 2. add ClientInvocationId 3. and other places

taklwu force-pushed the RATIS-2393 branch from 983ff43 to d097a31 Compare March 5, 2026 19:32

fix checkstyle

51972da

This comment was marked as off-topic.

Sign in to view

rerun test

4a21524

szetszwo reviewed Mar 6, 2026

View reviewed changes

fix based on comments

decc035

- use lambda instead of implements supplier - move most the code into ratis-common

taklwu commented Mar 9, 2026

View reviewed changes

missed TraceConfigKeys

c748d1b

remove otel dependencies in ratis-server

c1c6e5a

szetszwo reviewed Mar 11, 2026

View reviewed changes

remove ATTR_ prefix and rename client.id

f0dea17

szetszwo approved these changes Mar 12, 2026

View reviewed changes

szetszwo merged commit d132b18 into apache:master Mar 12, 2026
16 checks passed

		public static final AttributeKey<String> ATTR_CLIENT_INVOCATION_ID =
		AttributeKey.stringKey("raft.client.invocation.id");

Conversation

taklwu commented Feb 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

What is the link to the Apache JIRA

How was this patch tested?

Uh oh!

taklwu commented Feb 4, 2026

Uh oh!

szetszwo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

OneSizeFitsQuorum commented Feb 16, 2026

Uh oh!

taklwu commented Mar 3, 2026

Uh oh!

szetszwo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

taklwu commented Mar 5, 2026

Uh oh!

This comment was marked as off-topic.

szetszwo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

szetszwo Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

taklwu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

taklwu Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

szetszwo Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

taklwu Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

szetszwo commented Mar 11, 2026

Uh oh!

szetszwo left a comment

Choose a reason for hiding this comment

Uh oh!

szetszwo Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

szetszwo Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

taklwu Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

szetszwo Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

szetszwo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

taklwu commented Feb 4, 2026 •

edited

Loading

szetszwo Mar 6, 2026 •

edited

Loading