Add the Nacos-Client 2.x plugin by peachisai · Pull Request #18758 · open-telemetry/opentelemetry-java-instrumentation

peachisai · 2026-05-15T00:47:46Z

#18290
nacos-client utilizes the Maven Shade plugin to prevent class conflicts. Consequently, gRPC instrumentation is unable to capture the requests.

linux-foundation-easycla · 2026-05-15T00:47:53Z

The committers listed above are authorized under a signed CLA.

✅ login: peachisai / name: peachisai (6226da8)

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated 15 comments.

Comments suppressed due to low confidence (2)

instrumentation/nacos-client-2.0/metadata.yaml:3

[Documentation] This module builds RPC client/consumer spans, but the metadata does not declare any semantic_conventions entries. Existing RPC modules such as instrumentation/grpc-1.6/metadata.yaml:3-7 and instrumentation/apache-dubbo-2.7/metadata.yaml:5-9 declare their RPC span/metric signals so generated docs and the instrumentation registry classify the emitted telemetry correctly.

description: This instrumentation provides a template for Nacos client 2.x spans.
disabled_by_default: true

instrumentation/nacos-client-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/nacosclient/v2_0/NacosClientInstrumentationModule.java:33

[General] The module is registered but explicitly disabled by default, while the PR/linked issue describe adding Nacos 2.x instrumentation to cover requests that shaded gRPC cannot capture. As written, users will still get no Nacos spans after upgrading unless they discover and set otel.instrumentation.nacos-client.enabled=true; either make the instrumentation enabled by default or document the opt-in rationale clearly.

  @Override
  public boolean defaultEnabled() {
    return false;
  }

@@ -0,0 +1,4 @@
+display_name: Nacos Client
+description: This instrumentation provides a template for Nacos client 2.x spans.


+    return asList(
+        new GrpcClientInstrumentation(),
+        new GrpcConnectionInstrumentation(),
+        new RpcClientInstrumentation());


+            .addAttributesExtractor(new NacosClientAttributesExtractor());
+    return builder.buildInstrumenter(SpanKindExtractor.alwaysConsumer());


+import com.alibaba.nacos.api.remote.request.Request;
+import javax.annotation.Nullable;
+
+public final class NacosClientRequest {


+import io.opentelemetry.context.Context;
+import io.opentelemetry.context.Scope;
+
+public final class ContextAndScope {


+final class NacosClientRpcAttributesGetter
+    implements RpcAttributesGetter<NacosClientRequest, Response> {


+final class NacosClientNetworkAttributesGetter
+    implements ServerAttributesGetter<NacosClientRequest>,
+        NetworkAttributesGetter<NacosClientRequest, Response> {


+final class NacosClientAttributesExtractor
+    implements AttributesExtractor<NacosClientRequest, Response> {


+        request, category, operation, peer, hostPort.host(), hostPort.port());
+  }
+
+  private static final class HostPort {


+    }
+  }
+
+  private static final class ServerCheckRequest extends Request {


Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

This reverts commit 7e763c9.

laurit · 2026-05-26T09:51:44Z

+  @Override
+  public void transform(TypeTransformer transformer) {
+    transformer.applyAdviceToMethod(
+        isMethod()


isMethod() isn't needed

laurit · 2026-05-26T11:11:11Z

+    if (rawRequest instanceof InstanceRequest) {
+      InstanceRequest instanceRequest = (InstanceRequest) rawRequest;
+      put(attributes, NACOS_NAMESPACE, instanceRequest.getNamespace());
+      put(attributes, NACOS_GROUP, instanceRequest.getGroupName());
+      put(attributes, NACOS_SERVICE_NAME, instanceRequest.getServiceName());
+    } else if (rawRequest instanceof ServiceQueryRequest) {
+      ServiceQueryRequest serviceQueryRequest = (ServiceQueryRequest) rawRequest;
+      put(attributes, NACOS_NAMESPACE, serviceQueryRequest.getNamespace());
+      put(attributes, NACOS_GROUP, serviceQueryRequest.getGroupName());
+      put(attributes, NACOS_SERVICE_NAME, serviceQueryRequest.getServiceName());
+    } else if (rawRequest instanceof SubscribeServiceRequest) {
+      SubscribeServiceRequest subscribeServiceRequest = (SubscribeServiceRequest) rawRequest;
+      put(attributes, NACOS_NAMESPACE, subscribeServiceRequest.getNamespace());
+      put(attributes, NACOS_GROUP, subscribeServiceRequest.getGroupName());
+      put(attributes, NACOS_SERVICE_NAME, subscribeServiceRequest.getServiceName());
+    } else if (rawRequest instanceof ServiceListRequest) {
+      ServiceListRequest serviceListRequest = (ServiceListRequest) rawRequest;
+      put(attributes, NACOS_NAMESPACE, serviceListRequest.getNamespace());
+      put(attributes, NACOS_GROUP, serviceListRequest.getGroupName());
+      put(attributes, NACOS_SERVICE_NAME, serviceListRequest.getServiceName());


all of these are subclasses of AbstractNamingRequest could consider using instanceof AbstractNamingRequest

laurit · 2026-05-26T11:25:33Z

+      NacosClientRequest request, @Nullable Response response, @Nullable Throwable error) {
+    if (response != null && !response.isSuccess()) {
+      int errorCode = response.getErrorCode();
+      return errorCode == 0 ? "response_error" : String.valueOf(errorCode);


why response_error?

laurit · 2026-05-26T11:27:08Z

+    return builder.buildInstrumenter(SpanKindExtractor.alwaysClient());
+  }
+
+  private static Instrumenter<NacosClientRequest, Response> createServerInstrumenter() {


consider removing Client from classes shared by client and server instrumentation

laurit · 2026-05-26T11:29:23Z

+    if (response != null && !response.isSuccess()) {
+      String message = response.getMessage();
+      int errorCode = response.getErrorCode();
+      return new IllegalStateException(


Creating an exception here doesn't make sense since the stack trace is not going to be useful for the end user. Consider implementing a SpanStatusExtractor extractor instead that sets span status to error when the response wasn't successful.

laurit · 2026-05-26T11:31:54Z

+    if (serverInfo == null) {
+      serverInfo = rpcClient.getCurrentServer();
+    }
+    return serverInfo != null ? serverInfo.getAddress() : "unknown";


our convention is to leave attributes unfilled if a value can't be extracted

laurit · 2026-05-26T11:38:56Z

+  testImplementation(project(":instrumentation-api"))
+  testImplementation(project(":instrumentation-api-incubator"))
+
+  testImplementation("io.opentelemetry:opentelemetry-api")
+  testImplementation("io.opentelemetry:opentelemetry-context")
+  testImplementation("com.alibaba.nacos:nacos-client:2.0.0")


I think these aren't needed

laurit · 2026-05-26T11:39:35Z

+  systemProperty("collectMetadata", otelProps.collectMetadata)
+}
+
+val testAgentInstrumentation by tasks.registering(Test::class) {


I think you don't need a separate test task for this, could just add jvmArgs("-Dotel.instrumentation.nacos-client.enabled=true") to the main test task.

laurit · 2026-05-26T12:12:56Z

    "co.elastic.clients:elasticsearch-java#+": "9.4.1",
    "co.elastic.clients:elasticsearch-java#8.9.+": "8.9.2",
    "com.alibaba:druid#+": "1.2.28",
+    "com.alibaba.nacos:nacos-client#+": "2.5.2",


did you manually edit this? should the latest version be 3.2.1?

laurit · 2026-05-26T12:17:39Z

+        @Advice.Argument(0) String serverIp,
+        @Advice.Argument(1) int serverPort) {
+      RpcClientServerInfoAccessor.set(rpcClient, new RpcClient.ServerInfo(serverIp, serverPort));
+      return NacosClientSingletons.startClientSpan(


This somewhat deviates from wheat we usually do. Many instrumentations use a class named AdviceScope to pass context, scope and request to end advice. Using some other class is also fine, for example here instead of ContextAndScope you could pass a class that contains context, scope and request and knows how to end the span.

Add the Nacos-Client 2.x plugin

6226da8

Copilot AI review requested due to automatic review settings May 15, 2026 00:47

peachisai requested a review from a team as a code owner May 15, 2026 00:47

Copilot started reviewing on behalf of peachisai May 15, 2026 00:48 View session

Copilot AI reviewed May 15, 2026

github-actions Bot mentioned this pull request May 15, 2026

Pull Request Dashboard #18435

Open

peachisai added 11 commits May 15, 2026 12:33

Merge branch 'main' into dev

e55feaa

fix ci issues

ffa0c0d

Merge branch 'main' into dev

b5b0a64

fix version issues

83a03bb

remove the comment

c951ae5

fix document

9e01421

fix test

2e226f7

mock the rpcClient construct

23b18df

fix spotlessJavaCheck

3ba349d

fix spotlessJavaCheck

8e1878e

Merge remote-tracking branch 'origin/main' into dev

7a37848

trask requested a review from Copilot May 17, 2026 23:18

Copilot started reviewing on behalf of trask May 17, 2026 23:18 View session

Copilot AI reviewed May 17, 2026

View reviewed changes

peachisai added 2 commits May 19, 2026 08:48

fix Copilot suggestion

8243d9f

Merge branch 'main' into dev

fe9c879

peachisai requested a review from Copilot May 19, 2026 00:52

Copilot started reviewing on behalf of peachisai May 19, 2026 00:52 View session

Copilot AI reviewed May 19, 2026

peachisai added 4 commits May 21, 2026 23:31

fix test

5e34659

fix test

3587cab

fix test

e3e1b13

fix test

7e763c9

peachisai added 6 commits May 24, 2026 00:44

Merge branch 'main' into dev

4a8ce06

Revert "fix test"

5b3843e

This reverts commit 7e763c9.

fix test

5903e92

fix spotless

b88dc15

Merge branch 'open-telemetry:main' into dev

2241c20

Merge branch 'main' into dev

150df7d

laurit reviewed May 26, 2026

View reviewed changes

Merge branch 'open-telemetry:main' into dev

066f56d

		@@ -0,0 +1,4 @@
		display_name: Nacos Client
		description: This instrumentation provides a template for Nacos client 2.x spans.

		.addAttributesExtractor(new NacosClientAttributesExtractor());
		return builder.buildInstrumenter(SpanKindExtractor.alwaysConsumer());

		final class NacosClientRpcAttributesGetter
		implements RpcAttributesGetter<NacosClientRequest, Response> {

		final class NacosClientAttributesExtractor
		implements AttributesExtractor<NacosClientRequest, Response> {

Conversation

peachisai commented May 15, 2026

Uh oh!

linux-foundation-easycla Bot commented May 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

linux-foundation-easycla Bot commented May 15, 2026 •

edited

Loading