optimize: metadata discovery support for consul #7745

YoWuwuuuw · 2025-10-29T12:10:50Z

I have read the CONTRIBUTING.md guidelines.
I have registered the PR changes.

Ⅰ. Describe what this PR did

Enhance the Consul registry center with metadata registration and discovery capabilities based on branch 'gsoc-2025-meta-registry'.
Fix the logic in RegistryHeartBeats from the previous refactoring.

Ⅱ. Does this pull request fix one issue?

Ⅲ. Why don't you add test cases (unit test/integration test)?

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

Because the TCP check in the original implementation class would be marked as unhealthy by Consul in the test environment, the integration test class ConsulRegistryServiceImplTest uses a mock implementation class (replaced with TTL connection) for testing.

* fix: fix client spring version compatible * fix: fix client spring version compatible * fix: fix client spring version compatible * bugfix: fix client spring version compatible * bugfix: fix client spring version compatible

# Conflicts: # changes/en-us/2.x.md # changes/zh-cn/2.x.md # common/src/main/java/org/apache/seata/common/metadata/ServiceInstance.java # common/src/test/java/org/apache/seata/common/metadata/ServiceInstanceTest.java # core/src/main/java/org/apache/seata/core/rpc/netty/AbstractNettyRemotingClient.java # discovery/seata-discovery-consul/src/main/java/org/apache/seata/discovery/registry/consul/ConsulRegistryServiceImpl.java # discovery/seata-discovery-core/src/main/java/org/apache/seata/discovery/loadbalance/WeightedRandomLoadBalance.java # discovery/seata-discovery-etcd3/src/main/java/org/apache/seata/discovery/registry/etcd3/EtcdRegistryServiceImpl.java # discovery/seata-discovery-eureka/src/main/java/org/apache/seata/discovery/registry/eureka/EurekaRegistryServiceImpl.java # discovery/seata-discovery-namingserver/src/main/java/org/apache/seata/discovery/registry/namingserver/NamingserverRegistryServiceImpl.java # discovery/seata-discovery-namingserver/src/test/java/org/apache/seata/discovery/registry/namingserver/NamingserverRegistryServiceImplTest.java # discovery/seata-discovery-raft/src/main/java/org/apache/seata/discovery/registry/raft/RaftRegistryServiceImpl.java # seata-spring-autoconfigure/seata-spring-autoconfigure-server/src/test/java/org/apache/seata/spring/boot/autoconfigure/properties/server/filter/ServerHttpFilterXssPropertiesTest.java # server/src/main/resources/application.raft.example.yml

…to gsoc-metadata-support-consul # Conflicts: # README.md # changes/en-us/2.x.md # changes/zh-cn/2.x.md

Copilot

Pull Request Overview

This PR enhances the Consul registry center with metadata registration and discovery capabilities, and fixes the logic in RegistryHeartBeats that was broken in a previous refactoring. The changes migrate the registry system from using InetSocketAddress directly to using ServiceInstance which encapsulates both address and metadata information.

Key changes:

Refactored RegistryHeartBeats to accept ServiceInstance instead of InetSocketAddress for heartbeat management
Enhanced Consul and Redis registry implementations to support metadata registration and discovery
Added utility method getStringMap() to convert metadata for registry services that require string-based metadata storage

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
`discovery/seata-discovery-core/src/main/java/org/apache/seata/discovery/registry/RegistryHeartBeats.java`	Updated heartbeat interface to use `ServiceInstance` instead of `InetSocketAddress`
`discovery/seata-discovery-redis/src/main/java/org/apache/seata/discovery/registry/redis/RedisRegistryServiceImpl.java`	Updated to pass `ServiceInstance` to heartbeat management
`discovery/seata-discovery-consul/src/main/java/org/apache/seata/discovery/registry/consul/ConsulRegistryServiceImpl.java`	Enhanced to support metadata registration and discovery via `ServiceInstance`
`common/src/main/java/org/apache/seata/common/metadata/ServiceInstance.java`	Added constructor for `Instance` type and `getStringMap()` utility method for metadata conversion
`discovery/seata-discovery-consul/src/test/java/org/apache/seata/discovery/registry/consul/MockConsulRegistryServiceImpl.java`	Mock implementation using TTL checks instead of TCP checks for testing metadata features
`discovery/seata-discovery-consul/src/test/java/org/apache/seata/discovery/registry/consul/MockConsulRegistryProvider.java`	Provider for loading the mock implementation in tests
`discovery/seata-discovery-consul/src/test/java/org/apache/seata/discovery/registry/consul/ConsulRegistryServiceImplTest.java`	Integration test for metadata registration and discovery
`discovery/seata-discovery-consul/src/test/resources/registry.conf`	Test configuration updated to use Consul registry
`discovery/seata-discovery-consul/src/test/resources/META-INF/services/org.apache.seata.discovery.registry.RegistryProvider`	Service loader configuration for mock provider

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-18T07:56:27Z

.../src/test/java/org/apache/seata/discovery/registry/consul/ConsulRegistryServiceImplTest.java

+        metadata1.put("weight", 100);
+
+        ServiceInstance instance1 = new ServiceInstance(new InetSocketAddress("127.0.0.1", 8080), metadata1);
+        registryService.register(instance1);
+
+        Map<String, Object> metadata2 = new HashMap<>();
+        metadata2.put("version", "2.0.0");
+        metadata2.put("zone", "bj");
+
+        ServiceInstance instance2 = new ServiceInstance(new InetSocketAddress("127.0.0.1", 9090), metadata2);
+        registryService.register(instance2);
+
+        Thread.sleep(3000);
+
+        List<ServiceInstance> instances = registryService.lookup("default_tx_group");
+
+        assertNotNull(instances);
+        assertFalse(instances.isEmpty());
+
+        ServiceInstance foundInstance1 = instances.stream()
+                .filter(inst -> inst.getAddress().equals(instance1.getAddress()))
+                .findFirst()
+                .orElse(null);
+
+        assertNotNull(foundInstance1);
+        Map<String, Object> foundMetadata1 = foundInstance1.getMetadata();
+        assertNotNull(foundMetadata1);
+        assertEquals("1.0.0", foundMetadata1.get("version"));
+        assertEquals("test", foundMetadata1.get("environment"));
+        assertEquals("100", foundMetadata1.get("weight"));


[nitpick] The test stores weight as an Integer (100) but expects it to be retrieved as a String ("100"). While this is technically correct due to the getStringMap conversion, it would be clearer to either: 1) put the value as a String initially to match the expectation, or 2) add a comment explaining the conversion behavior. This improves test clarity and maintainability.

Copilot · 2025-11-18T07:56:27Z

...nsul/src/main/java/org/apache/seata/discovery/registry/consul/ConsulRegistryServiceImpl.java

+                })
+                .collect(Collectors.toList());
+
+        System.out.println("service instances:" + instances);


Debug statement System.out.println should be removed from production code. Consider using a proper logger (LOGGER.debug) if this information is needed for debugging purposes.

Suggested change

System.out.println("service instances:" + instances);

LOGGER.debug("service instances: {}", instances);

Copilot · 2025-11-18T07:56:28Z

common/src/main/java/org/apache/seata/common/metadata/ServiceInstance.java

+    public ServiceInstance(Instance instance) {
+        this.address = new InetSocketAddress(
+                instance.getTransaction().getHost(), instance.getTransaction().getPort());
+        this.metadata = instance.getMetadata();
+    }


Missing JavaDoc documentation for the new constructor. Add documentation explaining the purpose of this constructor and the parameter instance.

Copilot · 2025-11-18T07:56:28Z

common/src/main/java/org/apache/seata/common/metadata/ServiceInstance.java


+    /**
+     * Convert metadata to Map<String, String>. Non-String values will use toString().
+     *


Missing JavaDoc parameter documentation. Add @param metadata to document the metadata parameter in the method signature.

Suggested change

*

*

* @param metadata the metadata map to convert

Copilot · 2025-11-18T07:56:28Z

.../src/test/java/org/apache/seata/discovery/registry/consul/MockConsulRegistryServiceImpl.java

+        // 1.remove notifier for the cluster
+        ConsulNotifier notifier = notifiers.remove(cluster);
+        // 2.stop the notifier
+        notifier.stop();


Potential NullPointerException: If the cluster key doesn't exist in notifiers, the remove operation returns null, and then calling notifier.stop() will throw a NullPointerException. Add a null check before calling stop().

Suggested change

notifier.stop();

if (notifier != null) {

notifier.stop();

}

Copilot · 2025-11-18T07:56:29Z

.../src/test/java/org/apache/seata/discovery/registry/consul/MockConsulRegistryServiceImpl.java

+        System.out.println("Mock: healthy services:" + response.getValue().toString());
+        if (response == null) {
+            return;
+        }


Variable response may be null at this access as suggested by this null guard.

Suggested change

System.out.println("Mock: healthy services:" + response.getValue().toString());

if (response == null) {

return;

}

if (response == null) {

return;

}

System.out.println("Mock: healthy services:" + response.getValue().toString());

Copilot · 2025-11-18T07:56:29Z

.../src/test/java/org/apache/seata/discovery/registry/consul/MockConsulRegistryServiceImpl.java

+            if ((currentIndex != null && currentIndex > consulIndex) || hasError) {
+                hasError = false;
+                List<HealthService> services = response.getValue();
+                consulIndex = currentIndex; /*lgtm[java/dereferenced-value-may-be-null]*/


Variable currentIndex may be null at this access as suggested by this null guard.

Suggested change

consulIndex = currentIndex; /*lgtm[java/dereferenced-value-may-be-null]*/

if (currentIndex != null) {

consulIndex = currentIndex;

}

YoWuwuuuw and others added 30 commits June 24, 2025 22:49

feat: add check box

ecaf257

Merge remote-tracking branch 'origin/2.x' into 2.x

6d033b7

feat: Data format for supporting metadata

4446570

test: add test

731b6bd

feat: support server-metadata access to RegistryService

49c54db

opt: solve conflicts and opt discovery-namingserver code

e1ad806

fix: fix some bug and improve code readability

66702a9

test: fix test for discovery-namingserver

3953c32

feat: enhance discovery-loadbalance to support metadata

81465da

feat: enhance discovery-loadbalance to support metadata

8d015b5

feat: support WeightedRandomLoadBalance

bab788c

test: fix test

a3d50dc

test: fix test

d1c4f12

Merge branch '2.x' into gsoc-metadata-support

775c7b1

doc: improve Java doc and improve readability

9f4439d

opt: apply spotless

8ae37ae

opt: rabbit review

27d1572

test: fix test

24ba004

add changes

e562774

test: add test for ServiceInstance

61b95e1

test: fix ci

1c9385d

test: add test for discovery-namingserver

1b29d94

test: fix ci

7cfd97c

optimize: optimize the Readme and change documents (apache#7531)

d481d55

test：refactor test

58806b8

test: fix jakarta UT failed in jdk17+ (apache#7541)

88bba2f

bugfix: fix client spring version compatible (apache#7546)

608785f

* fix: fix client spring version compatible * fix: fix client spring version compatible * fix: fix client spring version compatible * bugfix: fix client spring version compatible * bugfix: fix client spring version compatible

fix: fix spi bug

1d727b2

feat: meta-support for consul

fcfd810

Merge branch '2.x' into gsoc-metadata-support-consul

6817823

YoWuwuuuw added 3 commits October 29, 2025 19:37

opt

34ab943

Merge remote-tracking branch 'origin/gsoc-metadata-support-consul' in…

2a6506e

…to gsoc-metadata-support-consul # Conflicts: # README.md # changes/en-us/2.x.md # changes/zh-cn/2.x.md

slievrly added the gsoc-2025 label Nov 2, 2025

xingfudeshi requested review from Copilot and xingfudeshi November 18, 2025 07:48

Copilot started reviewing on behalf of xingfudeshi November 18, 2025 07:48 View session

Copilot finished reviewing on behalf of xingfudeshi November 18, 2025 07:52

Copilot AI reviewed Nov 18, 2025

View reviewed changes

Merge branch 'gsoc-2025-meta-registry' into gsoc-metadata-support-consul

48b829f

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

optimize: metadata discovery support for consul #7745

optimize: metadata discovery support for consul #7745

Uh oh!

YoWuwuuuw commented Oct 29, 2025 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Nov 18, 2025

Uh oh!

Copilot AI Nov 18, 2025

Uh oh!

Copilot AI Nov 18, 2025

Uh oh!

Copilot AI Nov 18, 2025

Uh oh!

Copilot AI Nov 18, 2025

Uh oh!

Copilot AI Nov 18, 2025

Uh oh!

Copilot AI Nov 18, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	System.out.println("service instances:" + instances);
	LOGGER.debug("service instances: {}", instances);

optimize: metadata discovery support for consul #7745

Are you sure you want to change the base?

optimize: metadata discovery support for consul #7745

Uh oh!

Conversation

YoWuwuuuw commented Oct 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Ⅰ. Describe what this PR did

Ⅱ. Does this pull request fix one issue?

Ⅲ. Why don't you add test cases (unit test/integration test)?

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Nov 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 18, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

YoWuwuuuw commented Oct 29, 2025 •

edited

Loading