feat(metadata): Clarify metadata report selection for multi-registry and multi-instance scenarios

metadata/client.go

@@ -42,10 +42,10 @@ import (
 
 const defaultTimeout = "5s" // s
 
-func GetMetadataFromMetadataReport(revision string, instance registry.ServiceInstance) (*info.MetadataInfo, error) {
-	report := GetMetadataReport()
+func GetMetadataFromMetadataReport(revision string, instance registry.ServiceInstance, registryId string) (*info.MetadataInfo, error) {
+	report := GetMetadataReportByRegistry(registryId)
 	if report == nil {
-		return nil, perrors.New("no metadata report instance found,please check ")
+		return nil, perrors.Errorf("no metadata report instance found for registryId=%s, please check metadata-report configuration", registryId)
 	}
 	return report.GetAppMetadata(instance.GetServiceName(), revision)
 }

metadata/client_test.go

@@ -35,6 +35,7 @@ import (
 	"dubbo.apache.org/dubbo-go/v3/common/constant"
 	"dubbo.apache.org/dubbo-go/v3/common/extension"
 	"dubbo.apache.org/dubbo-go/v3/metadata/info"
+	"dubbo.apache.org/dubbo-go/v3/metadata/report"
 	"dubbo.apache.org/dubbo-go/v3/protocol/base"
 	"dubbo.apache.org/dubbo-go/v3/protocol/result"
 	_ "dubbo.apache.org/dubbo-go/v3/proxy/proxy_factory"
@@ -62,22 +63,64 @@ var (
 )
 
 func TestGetMetadataFromMetadataReport(t *testing.T) {
+	t.Cleanup(func() { instances = make(map[string]report.MetadataReport) })
+
 	t.Run("no report instance", func(t *testing.T) {
-		_, err := GetMetadataFromMetadataReport("1", ins)
+		instances = make(map[string]report.MetadataReport)
+		_, err := GetMetadataFromMetadataReport("1", ins, "default")
 		require.Error(t, err)
 	})
-	mockReport := new(mockMetadataReport)
-	defer mockReport.AssertExpectations(t)
-	instances["default"] = mockReport
-	t.Run("normal", func(t *testing.T) {
+
+	t.Run("default registry routes to default report", func(t *testing.T) {
+		instances = make(map[string]report.MetadataReport)
+		mockReport := new(mockMetadataReport)
+		defer mockReport.AssertExpectations(t)
+		instances["default"] = mockReport
+
 		mockReport.On("GetAppMetadata").Return(metadataInfo, nil).Once()
-		got, err := GetMetadataFromMetadataReport("1", ins)
+		got, err := GetMetadataFromMetadataReport("1", ins, "default")
+		require.NoError(t, err)
+		assert.Equal(t, metadataInfo, got)
+	})
+
+	t.Run("specific registryId routes to its own report", func(t *testing.T) {
+		instances = make(map[string]report.MetadataReport)
+		defaultReport := new(mockMetadataReport)
+		specificReport := new(mockMetadataReport)
+		defer defaultReport.AssertExpectations(t)
+		defer specificReport.AssertExpectations(t)
+		instances["default"] = defaultReport
+		instances["reg-a"] = specificReport
+
+		// specificReport must be called; defaultReport must NOT be called
+		specificReport.On("GetAppMetadata").Return(metadataInfo, nil).Once()
+		got, err := GetMetadataFromMetadataReport("1", ins, "reg-a")
+		require.NoError(t, err)
+		assert.Equal(t, metadataInfo, got)
+	})
+
+	t.Run("unknown registryId falls back to default report", func(t *testing.T) {
+		instances = make(map[string]report.MetadataReport)
+		defaultReport := new(mockMetadataReport)
+		defer defaultReport.AssertExpectations(t)
+		instances["default"] = defaultReport
+
+		// When the specific registryId is not found, it falls back to "default"
+		// so the default report's GetAppMetadata is called
+		defaultReport.On("GetAppMetadata").Return(metadataInfo, nil).Once()
+		got, err := GetMetadataFromMetadataReport("1", ins, "nonexistent-registry")
 		require.NoError(t, err)
 		assert.Equal(t, metadataInfo, got)
 	})
-	t.Run("error", func(t *testing.T) {
+
+	t.Run("report error propagated", func(t *testing.T) {
+		instances = make(map[string]report.MetadataReport)
+		mockReport := new(mockMetadataReport)
+		defer mockReport.AssertExpectations(t)
+		instances["default"] = mockReport
+
 		mockReport.On("GetAppMetadata").Return(metadataInfo, errors.New("mock error")).Once()
-		_, err := GetMetadataFromMetadataReport("1", ins)
+		_, err := GetMetadataFromMetadataReport("1", ins, "default")
 		require.Error(t, err)
 	})
 }

metadata/mapping/metadata/service_name_mapping.go

@@ -18,6 +18,7 @@
 package metadata
 
 import (
+	"errors"
 	"sync"
 )
 
@@ -89,18 +90,56 @@ func (d *ServiceNameMapping) Map(url *common.URL) error {
 // Get will return the application-level services. If not found, the empty set will be returned.
 func (d *ServiceNameMapping) Get(url *common.URL, listener mapping.MappingListener) (*gxset.HashSet, error) {
 	serviceInterface := url.GetParam(constant.InterfaceKey, "")
-	metadataReport := metadata.GetMetadataReport()
-	if metadataReport == nil {
+	metadataReports := metadata.GetMetadataReports()
+	if len(metadataReports) == 0 {
 		return nil, perrors.New("can not get mapping in remote cause no metadata report instance found")
 	}
-	return metadataReport.GetServiceAppMapping(serviceInterface, DefaultGroup, listener)
+	// Attach the listener to the stable primary report only (GetMetadataReport uses
+	// a deterministic selection: prefer "default", otherwise lexicographic first).
+	// GetMetadataReports() iterates a map so its order is non-deterministic; using
+	// i==0 as the anchor would bind the listener to a random backend each run.
+	primaryReport := metadata.GetMetadataReport()
+	var result *gxset.HashSet
+	var errs []error
+	for _, metadataReport := range metadataReports {
+		var reportListener mapping.MappingListener
+		if metadataReport == primaryReport {
+			reportListener = listener
+		}
+		set, err := metadataReport.GetServiceAppMapping(serviceInterface, DefaultGroup, reportListener)
+		if err != nil {
+			errs = append(errs, err)
+			continue
+		}
+		if result == nil {
+			result = set
+		} else {
+			result.Add(set.Values()...)
+		}
+	}
+	if result == nil {
+		return nil, errors.Join(errs...)
+	}
+	return result, nil
 }
 
+// Remove removes the service-to-app mapping for the given URL from all
+// registered metadata reports. Unlike Map (which stops on the first failure),
+// Remove is best-effort: it attempts every report and returns all errors
+// joined together so the caller can see the full failure picture. The
+// intent is to avoid leaving stale entries in any registry due to a transient
+// error in one of the others.
 func (d *ServiceNameMapping) Remove(url *common.URL) error {
 	serviceInterface := url.GetParam(constant.InterfaceKey, "")
-	metadataReport := metadata.GetMetadataReport()
-	if metadataReport == nil {
+	metadataReports := metadata.GetMetadataReports()
+	if len(metadataReports) == 0 {
 		return perrors.New("can not remove mapping in remote cause no metadata report instance found")
 	}
-	return metadataReport.RemoveServiceAppMappingListener(serviceInterface, DefaultGroup)
+	var errs []error
+	for _, metadataReport := range metadataReports {
+		if err := metadataReport.RemoveServiceAppMappingListener(serviceInterface, DefaultGroup); err != nil {
+			errs = append(errs, err)
+		}
+	}
+	return errors.Join(errs...)
 }

metadata/mapping/metadata/service_name_mapping_test.go

@@ -19,6 +19,7 @@ package metadata
 
 import (
 	"errors"
+	"sync"
 	"testing"
 )
 
@@ -139,6 +140,105 @@ func initMock() (*mockMetadataReport, error) {
 	return metadataReport, err
 }
 
+func initMockWithId(t *testing.T, registryId string) *mockMetadataReport {
+	t.Helper()
+	mockReport := new(mockMetadataReport)
+	extension.SetMetadataReportFactory(registryId, func() report.MetadataReportFactory {
+		return mockReport
+	})
+	opts := metadata.NewReportOptions(
+		metadata.WithRegistryId(registryId),
+		metadata.WithProtocol(registryId),
+		metadata.WithAddress("127.0.0.1"),
+	)
+	require.NoError(t, opts.Init())
+	return mockReport
+}
+
+func TestServiceNameMappingRemoveFansOutToAllReports(t *testing.T) {
+	metadata.ClearMetadataReportInstances()
+	t.Cleanup(metadata.ClearMetadataReportInstances)
+	serviceNameMappingOnce = sync.Once{}
+	serviceNameMappingInstance = nil
+
+	r1 := initMockWithId(t, "reg-a")
+	r2 := initMockWithId(t, "reg-b")
+
+	ins := GetNameMappingInstance()
+	serviceUrl := common.NewURLWithOptions(
+		common.WithInterface("org.example.FooService"),
+		common.WithParamsValue(constant.ApplicationKey, "foo-app"),
+	)
+
+	r1.On("RemoveServiceAppMappingListener").Return(nil).Once()
+	r2.On("RemoveServiceAppMappingListener").Return(nil).Once()
+
+	err := ins.Remove(serviceUrl)
+	require.NoError(t, err)
+	r1.AssertExpectations(t)
+	r2.AssertExpectations(t)
+}
+
+func TestServiceNameMappingRemoveCollectsAllErrors(t *testing.T) {
+	metadata.ClearMetadataReportInstances()
+	t.Cleanup(metadata.ClearMetadataReportInstances)
+	serviceNameMappingOnce = sync.Once{}
+	serviceNameMappingInstance = nil
+
+	r1 := initMockWithId(t, "reg-c")
+	r2 := initMockWithId(t, "reg-d")
+
+	ins := GetNameMappingInstance()
+	serviceUrl := common.NewURLWithOptions(
+		common.WithInterface("org.example.BarService"),
+		common.WithParamsValue(constant.ApplicationKey, "bar-app"),
+	)
+
+	err1 := errors.New("r1 failure")
+	err2 := errors.New("r2 failure")
+
+	// both reports fail
+	r1.On("RemoveServiceAppMappingListener").Return(err1).Once()
+	r2.On("RemoveServiceAppMappingListener").Return(err2).Once()
+
+	err := ins.Remove(serviceUrl)
+	require.Error(t, err)
+	// both individual errors must be present in the returned error
+	require.ErrorIs(t, err, err1)
+	require.ErrorIs(t, err, err2)
+	r1.AssertExpectations(t)
+	r2.AssertExpectations(t)
+}
+
+func TestServiceNameMappingRemoveContinuesAfterPartialFailure(t *testing.T) {
+	metadata.ClearMetadataReportInstances()
+	t.Cleanup(metadata.ClearMetadataReportInstances)
+	serviceNameMappingOnce = sync.Once{}
+	serviceNameMappingInstance = nil
+
+	r1 := initMockWithId(t, "reg-e")
+	r2 := initMockWithId(t, "reg-f")
+
+	ins := GetNameMappingInstance()
+	serviceUrl := common.NewURLWithOptions(
+		common.WithInterface("org.example.BazService"),
+		common.WithParamsValue(constant.ApplicationKey, "baz-app"),
+	)
+
+	removeErr := errors.New("r1 partial failure")
+
+	// r1 fails, r2 succeeds — the loop must not short-circuit
+	r1.On("RemoveServiceAppMappingListener").Return(removeErr).Once()
+	r2.On("RemoveServiceAppMappingListener").Return(nil).Once()
+
+	err := ins.Remove(serviceUrl)
+	// the joined error only contains r1's error; the call should error
+	require.ErrorIs(t, err, removeErr)
+	// both reports must have been called despite r1's failure
+	r1.AssertExpectations(t)
+	r2.AssertExpectations(t)
+}
+
 type listener struct {
 }
 

metadata/report_instance.go

@@ -18,11 +18,12 @@
 package metadata
 
 import (
+	"sort"
 	"time"
 )
 
 import (
-	"github.com/dubbogo/gost/container/set"
+	gxset "github.com/dubbogo/gost/container/set"
 	"github.com/dubbogo/gost/log/logger"
 )
 
@@ -41,6 +42,12 @@ var (
 	instances = make(map[string]report.MetadataReport)
 )
 
+// ClearMetadataReportInstances resets the package-level instances map.
+// Intended for test isolation only; do not call in production code.
+func ClearMetadataReportInstances() {
+	instances = make(map[string]report.MetadataReport)
+}
+
 func addMetadataReport(registryId string, url *common.URL) error {
 	fac := extension.GetMetadataReportFactory(url.Protocol)
 	if fac == nil {
@@ -51,21 +58,46 @@ func addMetadataReport(registryId string, url *common.URL) error {
 	return nil
 }
 
+// GetMetadataReport returns a single metadata report for callers that lack
+// registry context. It prefers the "default" registry's report; when absent
+// it falls back to the lexicographically first registry id so the selection
+// is always stable across calls.
 func GetMetadataReport() report.MetadataReport {
-	for _, v := range instances {
-		return v
+	if r, ok := instances[constant.DefaultKey]; ok {
+		return r
+	}
+	keys := make([]string, 0, len(instances))
+	for k := range instances {
+		keys = append(keys, k)
+	}
+	sort.Strings(keys)
+	if len(keys) > 0 {
+		return instances[keys[0]]
 	}
 	return nil
 }
 
+// GetMetadataReportByRegistry returns the metadata report bound to the given
+// registry id. When the registry id is empty the caller has no registry context,
+// so the stable default returned by GetMetadataReport is used. When a specific
+// (non-empty) registry id is not found, it falls back to the "default" report
+// if one exists. This handles the common case where a standalone metadata-report
+// config is registered under "default" while named registries (e.g. nacos, zk)
+// need to use it. nil is returned only when neither the specific id nor "default"
+// is registered.
 func GetMetadataReportByRegistry(registry string) report.MetadataReport {
 	if len(registry) == 0 {
-		registry = constant.DefaultKey
+		return GetMetadataReport()
 	}
 	if r, ok := instances[registry]; ok {
 		return r
 	}
-	return GetMetadataReport()
+	if r, ok := instances[constant.DefaultKey]; ok {
+		logger.Infof("[Metadata] no metadata report bound to registryId=%s, falling back to default", registry)
+		return r
+	}
+	logger.Warnf("[Metadata] no metadata report found for registryId=%s", registry)
+	return nil
 }
 
 func GetMetadataReports() []report.MetadataReport {