This is an automated email from the ASF dual-hosted git repository.
Alanxtl pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/dubbo-go.git
The following commit(s) were added to refs/heads/develop by this push:
new 17ce297fb feat(metadata): classify errors and add useful context
(#3707)
17ce297fb is described below
commit 17ce297fbf55de96c9d6050d83443b44aa116a82
Author: Modo <[email protected]>
AuthorDate: Wed Sep 2 21:23:39 2026 +0800
feat(metadata): classify errors and add useful context (#3707)
* feat: classify errors, enrich them with useful context, and add
failure-path tests
* fix test
---
metadata/client.go | 28 ++++++---
metadata/client_test.go | 69 +++++++++++++++++++++-
metadata/mapping/metadata/service_name_mapping.go | 35 +++++++++--
.../mapping/metadata/service_name_mapping_test.go | 42 ++++++++++++-
metadata/report_instance.go | 23 +++++++-
metadata/report_instance_test.go | 35 +++++++++--
.../servicediscovery/service_discovery_registry.go | 6 +-
.../service_discovery_registry_test.go | 8 ++-
.../service_instances_changed_listener_impl.go | 38 +++++++++---
...service_instances_changed_listener_impl_test.go | 17 +++++-
10 files changed, 262 insertions(+), 39 deletions(-)
diff --git a/metadata/client.go b/metadata/client.go
index 15169d605..791ce0a49 100644
--- a/metadata/client.go
+++ b/metadata/client.go
@@ -45,11 +45,12 @@ const defaultTimeout = "5s" // s
func GetMetadataFromMetadataReport(revision string, instance
registry.ServiceInstance, registryId string) (*info.MetadataInfo, error) {
report := GetMetadataReportByRegistry(registryId)
if report == nil {
- return nil, fmt.Errorf("no metadata report instance found for
registryId=%s, please check metadata-report configuration", registryId)
+ return nil, fmt.Errorf("metadata_report failed: operation=get
app=%s revision=%s registry_id=%s storage_type=%s: no metadata report instance
found, please check metadata-report configuration",
+ instance.GetServiceName(), revision, registryId,
constant.RemoteMetadataStorageType)
}
meta, err := report.GetAppMetadata(instance.GetServiceName(), revision)
if err != nil {
- return nil, fmt.Errorf("failed to get app metadata app=%s
revision=%s: %w", instance.GetServiceName(), revision, err)
+ return nil, fmt.Errorf("%w; registry_id=%s", err, registryId)
}
return meta, nil
}
@@ -64,18 +65,25 @@ func GetMetadataFromRpcWithContext(ctx context.Context,
revision string, instanc
if ctx == nil {
ctx = context.Background()
}
+ storageType := constant.DefaultMetadataStorageType
+ if instanceMetadata := instance.GetMetadata(); instanceMetadata != nil
&& instanceMetadata[constant.MetadataStorageTypePropertyName] != "" {
+ storageType =
instanceMetadata[constant.MetadataStorageTypePropertyName]
+ }
if err := ctx.Err(); err != nil {
- return nil, err
+ return nil, fmt.Errorf("rpc_metadata failed: app=%s revision=%s
instance_id=%s host=%s storage_type=%s: %w",
+ instance.GetServiceName(), revision, instance.GetID(),
instance.GetHost(), storageType, err)
}
url, err := buildStandardMetadataServiceURL(instance)
if err != nil {
- return nil, err
+ return nil, fmt.Errorf("url_construction failed: app=%s
revision=%s instance_id=%s host=%s storage_type=%s: %w",
+ instance.GetServiceName(), revision, instance.GetID(),
instance.GetHost(), storageType, err)
}
url.SetParam(constant.TimeoutKey, defaultTimeout)
p := extension.GetProtocol(url.Protocol)
invoker := p.Refer(url)
if invoker == nil { // can't connect instance
- return nil, errors.New("can not connect to remote metadata
service host: " + url.Ip)
+ return nil, fmt.Errorf("rpc_metadata failed: app=%s revision=%s
instance_id=%s host=%s storage_type=%s: can not connect to remote metadata
service",
+ instance.GetServiceName(), revision, instance.GetID(),
instance.GetHost(), storageType)
}
var remoteService remoteMetadataService
if url.Protocol == constant.TriProtocol &&
instance.GetMetadata()[constant.MetadataVersion] ==
constant.MetadataServiceV2Version {
@@ -86,7 +94,12 @@ func GetMetadataFromRpcWithContext(ctx context.Context,
revision string, instanc
defer func() {
invoker.Destroy()
}()
- return remoteService.getMetadataInfo(ctx, revision)
+ metadataInfo, err := remoteService.getMetadataInfo(ctx, revision)
+ if err != nil {
+ return metadataInfo, fmt.Errorf("rpc_metadata failed: app=%s
revision=%s instance_id=%s host=%s storage_type=%s: %w",
+ instance.GetServiceName(), revision, instance.GetID(),
instance.GetHost(), storageType, err)
+ }
+ return metadataInfo, nil
}
// remoteMetadataService is the internal interface for fetching MetadataInfo
via RPC.
@@ -287,7 +300,8 @@ func getMetadataServiceUrlParams(ins
registry.ServiceInstance) map[string]string
if str, ok := ps[constant.MetadataServiceURLParamsPropertyName]; ok &&
len(str) > 0 {
err := json.Unmarshal([]byte(str), &res)
if err != nil {
- logger.Errorf("[Metadata][URL] could not parse the
metadata service url parameters to map, err=%v", err)
+ logger.Errorf("[Metadata][URL] url_construction failed:
app=%s instance_id=%s host=%s: could not parse metadata service URL parameters:
%v",
+ ins.GetServiceName(), ins.GetID(),
ins.GetHost(), err)
}
}
diff --git a/metadata/client_test.go b/metadata/client_test.go
index c33413438..1c1536823 100644
--- a/metadata/client_test.go
+++ b/metadata/client_test.go
@@ -91,6 +91,20 @@ func TestGetMetadataFromMetadataReport(t *testing.T) {
instances = make(map[string]report.MetadataReport)
_, err := GetMetadataFromMetadataReport("1", ins, "default")
require.Error(t, err)
+ assert.Contains(t, err.Error(), "metadata_report failed:")
+ assert.Contains(t, err.Error(), "operation=get")
+ assert.Contains(t, err.Error(), "app=dubbo-app")
+ assert.Contains(t, err.Error(), "revision=1")
+ assert.Contains(t, err.Error(), "registry_id=default")
+ assert.Contains(t, err.Error(), "storage_type=remote")
+ })
+
+ t.Run("no report instance with empty registry id", func(t *testing.T) {
+ instances = make(map[string]report.MetadataReport)
+ _, err := GetMetadataFromMetadataReport("1", ins, "")
+ require.Error(t, err)
+ assert.Contains(t, err.Error(), "metadata_report failed:")
+ assert.Contains(t, err.Error(), "registry_id=")
})
t.Run("default registry routes to default report", func(t *testing.T) {
@@ -139,11 +153,19 @@ func TestGetMetadataFromMetadataReport(t *testing.T) {
instances = make(map[string]report.MetadataReport)
mockReport := new(mockMetadataReport)
defer mockReport.AssertExpectations(t)
- instances["default"] = mockReport
+ instances["default"] = &DelegateMetadataReport{instance:
mockReport}
- mockReport.On("GetAppMetadata").Return(metadataInfo,
errors.New("mock error")).Once()
+ sourceErr := errors.New("mock error")
+ mockReport.On("GetAppMetadata").Return(metadataInfo,
sourceErr).Once()
_, err := GetMetadataFromMetadataReport("1", ins, "default")
require.Error(t, err)
+ assert.Contains(t, err.Error(), "metadata_report failed:")
+ assert.Contains(t, err.Error(), "operation=get")
+ assert.Contains(t, err.Error(), "app=dubbo-app")
+ assert.Contains(t, err.Error(), "revision=1")
+ assert.Contains(t, err.Error(), "registry_id=default")
+ assert.Contains(t, err.Error(), "storage_type=remote")
+ require.ErrorIs(t, err, sourceErr)
})
}
@@ -173,17 +195,31 @@ func TestGetMetadataFromRpc(t *testing.T) {
mockProtocol.On("Refer").Return(nil).Once()
_, err := GetMetadataFromRpc("111", ins)
require.Error(t, err)
+ assert.Contains(t, err.Error(), "rpc_metadata failed:")
+ assert.Contains(t, err.Error(), "app=dubbo-app")
+ assert.Contains(t, err.Error(), "revision=111")
+ assert.Contains(t, err.Error(), "instance_id=1")
+ assert.Contains(t, err.Error(), "host=dubbo.io")
+ assert.Contains(t, err.Error(), "storage_type=local")
})
t.Run("invoke timeout", func(t *testing.T) {
+ sourceErr := errors.New("timeout error")
mockProtocol.On("Refer").Return(mockInvoker).Once()
mockInvoker.On("Invoke").Return(&result.RPCResult{
Attrs: map[string]any{},
- Err: errors.New("timeout error"),
+ Err: sourceErr,
Rest: metadataInfo,
}).Once()
mockInvoker.On("Destroy").Once()
_, err := GetMetadataFromRpc("111", ins)
require.Error(t, err)
+ assert.Contains(t, err.Error(), "rpc_metadata failed:")
+ assert.Contains(t, err.Error(), "app=dubbo-app")
+ assert.Contains(t, err.Error(), "revision=111")
+ assert.Contains(t, err.Error(), "instance_id=1")
+ assert.Contains(t, err.Error(), "host=dubbo.io")
+ assert.Contains(t, err.Error(), "storage_type=local")
+ require.ErrorIs(t, err, sourceErr)
})
}
@@ -208,6 +244,21 @@ func TestGetMetadataFromRpcWithContext(t *testing.T) {
assert.Same(t, ctx, mockInvoker.invokedContext)
}
+func TestGetMetadataFromRpcWithCanceledContext(t *testing.T) {
+ ctx, cancel := context.WithCancel(context.Background())
+ cancel()
+
+ _, err := GetMetadataFromRpcWithContext(ctx, "111", ins)
+ require.Error(t, err)
+ assert.Contains(t, err.Error(), "rpc_metadata failed:")
+ assert.Contains(t, err.Error(), "app=dubbo-app")
+ assert.Contains(t, err.Error(), "revision=111")
+ assert.Contains(t, err.Error(), "instance_id=1")
+ assert.Contains(t, err.Error(), "host=dubbo.io")
+ assert.Contains(t, err.Error(), "storage_type=local")
+ require.ErrorIs(t, err, context.Canceled)
+}
+
func TestTriMetadataServiceWithContext(t *testing.T) {
mockInvoker := new(mockInvoker)
mockInvoker.url =
common.NewURLWithOptions(common.WithProtocol(constant.TriProtocol))
@@ -230,6 +281,12 @@ func TestGetMetadataFromRpc_MissingURLParams(t *testing.T)
{
}
_, err := GetMetadataFromRpc("1", insNoProto)
require.Error(t, err)
+ assert.Contains(t, err.Error(), "url_construction failed:")
+ assert.Contains(t, err.Error(), "app=dubbo-app")
+ assert.Contains(t, err.Error(), "revision=1")
+ assert.Contains(t, err.Error(), "instance_id=2")
+ assert.Contains(t, err.Error(), "host=dubbo.io")
+ assert.Contains(t, err.Error(), "storage_type=local")
assert.Contains(t, err.Error(), "protocol is empty")
})
@@ -244,6 +301,12 @@ func TestGetMetadataFromRpc_MissingURLParams(t *testing.T)
{
}
_, err := GetMetadataFromRpc("1", insNoPort)
require.Error(t, err)
+ assert.Contains(t, err.Error(), "url_construction failed:")
+ assert.Contains(t, err.Error(), "app=dubbo-app")
+ assert.Contains(t, err.Error(), "revision=1")
+ assert.Contains(t, err.Error(), "instance_id=3")
+ assert.Contains(t, err.Error(), "host=dubbo.io")
+ assert.Contains(t, err.Error(), "storage_type=local")
assert.Contains(t, err.Error(), "port is empty")
})
}
diff --git a/metadata/mapping/metadata/service_name_mapping.go
b/metadata/mapping/metadata/service_name_mapping.go
index 4202e9dd5..09d030728 100644
--- a/metadata/mapping/metadata/service_name_mapping.go
+++ b/metadata/mapping/metadata/service_name_mapping.go
@@ -19,6 +19,7 @@ package metadata
import (
"errors"
+ "fmt"
"sync"
"time"
)
@@ -74,6 +75,7 @@ type ServiceNameMapping struct {
// Map will map the service to this application-level service
func (d *ServiceNameMapping) Map(url *common.URL) (err error) {
serviceInterface := url.GetParam(constant.InterfaceKey, "")
+ serviceKey := url.ServiceKey()
appName := url.GetParam(constant.ApplicationKey, "")
event :=
metadataMetrics.NewMetadataMetricTimeEvent(metadataMetrics.MetadataMappingRegister)
@@ -90,13 +92,20 @@ func (d *ServiceNameMapping) Map(url *common.URL) (err
error) {
// if the mapping can hold a report instance, it can write once
metadataReports := metadata.GetMetadataReports()
if len(metadataReports) == 0 {
- err = errors.New("can not registering mapping to remote cause
no metadata report instance found")
+ err = fmt.Errorf("mapping_register failed: service_key=%s
interface=%s application=%s group=%s reports=0: no metadata report instance
found",
+ serviceKey, serviceInterface, appName, DefaultGroup)
logger.Errorf("[Metadata][Mapping] register failed interface=%s
application=%s group=%s reports=0 err=%v", serviceInterface, appName,
DefaultGroup, err)
return err
}
for _, metadataReport := range metadataReports {
- if err := registerWithRetry(metadataReport, serviceInterface,
DefaultGroup, appName); err != nil {
+ if registerErr := registerWithRetry(metadataReport,
serviceInterface, DefaultGroup, appName); registerErr != nil {
+ reportURL := ""
+ if u := metadataReport.URL(); u != nil {
+ reportURL = u.Protocol + "://" + u.Address()
+ }
+ err = fmt.Errorf("mapping_register failed:
service_key=%s interface=%s application=%s group=%s reports=%d report_url=%s:
%w",
+ serviceKey, serviceInterface, appName,
DefaultGroup, len(metadataReports), reportURL, registerErr)
logger.Errorf("[Metadata][Mapping] register failed
interface=%s application=%s group=%s reports=%d err=%v", serviceInterface,
appName, DefaultGroup, len(metadataReports), err)
return err
}
@@ -136,11 +145,14 @@ func backoff(attempt int) time.Duration {
// 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) (result *gxset.HashSet, err error) {
serviceInterface := url.GetParam(constant.InterfaceKey, "")
+ serviceKey := url.ServiceKey()
operation := "get"
+ errorCategory := "mapping_get"
eventName := metadataMetrics.MetadataMappingGet
if listener != nil {
operation = "listen"
+ errorCategory = "mapping_listen"
eventName = metadataMetrics.MetadataMappingListen
}
@@ -157,7 +169,8 @@ func (d *ServiceNameMapping) Get(url *common.URL, listener
mapping.MappingListen
metadataReports := metadata.GetMetadataReports()
if len(metadataReports) == 0 {
- err = errors.New("can not get mapping in remote cause no
metadata report instance found")
+ err = fmt.Errorf("%s failed: service_key=%s interface=%s
group=%s reports=0: no metadata report instance found",
+ errorCategory, serviceKey, serviceInterface,
DefaultGroup)
logger.Warnf("[Metadata][Mapping] get failed interface=%s
group=%s reports=0 err=%v", serviceInterface, DefaultGroup, err)
return nil, err
}
@@ -174,11 +187,13 @@ func (d *ServiceNameMapping) Get(url *common.URL,
listener mapping.MappingListen
}
set, getErr :=
metadataReport.GetServiceAppMapping(serviceInterface, DefaultGroup,
reportListener)
if getErr != nil {
- errs = append(errs, getErr)
reportURL := ""
if u := metadataReport.URL(); u != nil {
reportURL = u.Protocol + "://" + u.Address()
}
+ getErr = fmt.Errorf("%s failed: service_key=%s
interface=%s group=%s report=%d/%d report_url=%s: %w",
+ errorCategory, serviceKey, serviceInterface,
DefaultGroup, i+1, len(metadataReports), reportURL, getErr)
+ errs = append(errs, getErr)
logger.Warnf("[Metadata][Mapping] %s report %d/%d
failed interface=%s group=%s url=%s err=%v", operation, i+1,
len(metadataReports), serviceInterface, DefaultGroup, reportURL, getErr)
continue
}
@@ -207,6 +222,7 @@ func (d *ServiceNameMapping) Get(url *common.URL, listener
mapping.MappingListen
// error in one of the others.
func (d *ServiceNameMapping) Remove(url *common.URL) (err error) {
serviceInterface := url.GetParam(constant.InterfaceKey, "")
+ serviceKey := url.ServiceKey()
event :=
metadataMetrics.NewMetadataMetricTimeEvent(metadataMetrics.MetadataMappingRemove)
event.Attachment[constant.InterfaceKey] = serviceInterface
@@ -219,14 +235,21 @@ func (d *ServiceNameMapping) Remove(url *common.URL) (err
error) {
metadataReports := metadata.GetMetadataReports()
if len(metadataReports) == 0 {
- err = errors.New("can not remove mapping in remote cause no
metadata report instance found")
+ err = fmt.Errorf("mapping_remove failed: service_key=%s
interface=%s group=%s reports=0: no metadata report instance found",
+ serviceKey, serviceInterface, DefaultGroup)
logger.Warnf("[Metadata][Mapping] remove failed interface=%s
group=%s reports=0 err=%v", serviceInterface, DefaultGroup, err)
return err
}
var errs []error
- for _, metadataReport := range metadataReports {
+ for i, metadataReport := range metadataReports {
if removeErr :=
metadataReport.RemoveServiceAppMappingListener(serviceInterface, DefaultGroup);
removeErr != nil {
+ reportURL := ""
+ if u := metadataReport.URL(); u != nil {
+ reportURL = u.Protocol + "://" + u.Address()
+ }
+ removeErr = fmt.Errorf("mapping_remove failed:
service_key=%s interface=%s group=%s report=%d/%d report_url=%s: %w",
+ serviceKey, serviceInterface, DefaultGroup,
i+1, len(metadataReports), reportURL, removeErr)
errs = append(errs, removeErr)
}
}
diff --git a/metadata/mapping/metadata/service_name_mapping_test.go
b/metadata/mapping/metadata/service_name_mapping_test.go
index 7c5b384d7..882b4a23b 100644
--- a/metadata/mapping/metadata/service_name_mapping_test.go
+++ b/metadata/mapping/metadata/service_name_mapping_test.go
@@ -61,10 +61,17 @@ func TestNoReportInstance(t *testing.T) {
)
_, err := ins.Get(serviceUrl, lis)
require.Error(t, err, "test Get no report instance")
+ assert.Contains(t, err.Error(), "mapping_listen failed:")
+ assert.Contains(t, err.Error(),
"service_key=org.apache.dubbo.samples.proto.GreetService")
+ assert.Contains(t, err.Error(),
"interface=org.apache.dubbo.samples.proto.GreetService")
+ assert.Contains(t, err.Error(), "group=mapping")
err = ins.Map(serviceUrl)
require.Error(t, err, "test Map with no report instance")
+ assert.Contains(t, err.Error(), "mapping_register failed:")
+ assert.Contains(t, err.Error(), "application=dubbo")
err = ins.Remove(serviceUrl)
require.Error(t, err, "test Remove with no report instance")
+ assert.Contains(t, err.Error(), "mapping_remove failed:")
}
func TestServiceNameMappingNoReportMetersPerBusinessOperation(t *testing.T) {
@@ -85,6 +92,7 @@ func
TestServiceNameMappingNoReportMetersPerBusinessOperation(t *testing.T) {
err := ins.Map(serviceUrl)
require.Error(t, err)
+ assert.Contains(t, err.Error(), "mapping_register failed:")
wantAttachment := mappingAttachment("org.example.NoReportService")
wantAttachment[constant.ApplicationKey] = "no-report-app"
assertMappingMetricEvent(t, <-ch,
metricsMetadata.MetadataMappingRegister, false, false, wantAttachment)
@@ -92,16 +100,19 @@ func
TestServiceNameMappingNoReportMetersPerBusinessOperation(t *testing.T) {
_, err = ins.Get(serviceUrl, nil)
require.Error(t, err)
+ assert.Contains(t, err.Error(), "mapping_get failed:")
assertMappingMetricEvent(t, <-ch, metricsMetadata.MetadataMappingGet,
false, false, mappingAttachment("org.example.NoReportService"))
assert.Empty(t, ch)
_, err = ins.Get(serviceUrl, &listener{})
require.Error(t, err)
+ assert.Contains(t, err.Error(), "mapping_listen failed:")
assertMappingMetricEvent(t, <-ch,
metricsMetadata.MetadataMappingListen, false, false,
mappingAttachment("org.example.NoReportService"))
assert.Empty(t, ch)
err = ins.Remove(serviceUrl)
require.Error(t, err)
+ assert.Contains(t, err.Error(), "mapping_remove failed:")
assertMappingMetricEvent(t, <-ch,
metricsMetadata.MetadataMappingRemove, false, false,
mappingAttachment("org.example.NoReportService"))
assert.Empty(t, ch)
}
@@ -122,9 +133,16 @@ func TestServiceNameMappingGet(t *testing.T) {
assert.False(t, apps.Empty())
})
t.Run("test error", func(t *testing.T) {
- mockReport.On("GetServiceAppMapping").Return(gxset.NewSet(),
errors.New("mock error")).Once()
+ sourceErr := errors.New("mock error")
+ mockReport.On("GetServiceAppMapping").Return(gxset.NewSet(),
sourceErr).Once()
_, err = ins.Get(serviceUrl, lis)
require.Error(t, err)
+ assert.Contains(t, err.Error(), "mapping_listen failed:")
+ assert.Contains(t, err.Error(),
"service_key=org.apache.dubbo.samples.proto.GreetService")
+ assert.Contains(t, err.Error(),
"interface=org.apache.dubbo.samples.proto.GreetService")
+ assert.Contains(t, err.Error(), "group=mapping")
+ assert.Contains(t, err.Error(), "report_url=mock://127.0.0.1")
+ require.ErrorIs(t, err, sourceErr)
})
mockReport.AssertExpectations(t)
}
@@ -144,9 +162,15 @@ func TestServiceNameMappingMap(t *testing.T) {
})
t.Run("non-conflict error returns immediately", func(t *testing.T) {
// a generic error is not retriable, so
RegisterServiceAppMapping is called exactly once
-
mockReport.On("RegisterServiceAppMapping").Return(errors.New("mock
error")).Once()
+ sourceErr := errors.New("mock error")
+
mockReport.On("RegisterServiceAppMapping").Return(sourceErr).Once()
err = ins.Map(serviceUrl)
require.Error(t, err, "test mapping error")
+ assert.Contains(t, err.Error(), "mapping_register failed:")
+ assert.Contains(t, err.Error(),
"service_key=org.apache.dubbo.samples.proto.GreetService")
+ assert.Contains(t, err.Error(), "application=dubbo")
+ assert.Contains(t, err.Error(), "report_url=mock://127.0.0.1")
+ require.ErrorIs(t, err, sourceErr)
})
t.Run("CAS conflict retries up to retryTimes", func(t *testing.T) {
const conflictRetries = 3
@@ -154,6 +178,8 @@ func TestServiceNameMappingMap(t *testing.T) {
mockReport.On("RegisterServiceAppMapping").Return(report.ErrMappingCASConflict).Times(conflictRetries)
err = ins.Map(serviceUrl)
require.Error(t, err, "conflict exhausts the retry budget")
+ assert.Contains(t, err.Error(), "mapping_register failed:")
+ require.ErrorIs(t, err, report.ErrMappingCASConflict)
})
mockReport.AssertExpectations(t)
}
@@ -172,9 +198,14 @@ func TestServiceNameMappingRemove(t *testing.T) {
require.NoError(t, err)
})
t.Run("test error", func(t *testing.T) {
-
mockReport.On("RemoveServiceAppMappingListener").Return(errors.New("mock
error")).Once()
+ sourceErr := errors.New("mock error")
+
mockReport.On("RemoveServiceAppMappingListener").Return(sourceErr).Once()
err = ins.Remove(serviceUrl)
require.Error(t, err)
+ assert.Contains(t, err.Error(), "mapping_remove failed:")
+ assert.Contains(t, err.Error(),
"service_key=org.apache.dubbo.samples.proto.GreetService")
+ assert.Contains(t, err.Error(), "report_url=mock://127.0.0.1")
+ require.ErrorIs(t, err, sourceErr)
})
mockReport.AssertExpectations(t)
}
@@ -255,6 +286,9 @@ func TestServiceNameMappingRemoveCollectsAllErrors(t
*testing.T) {
err := ins.Remove(serviceUrl)
require.Error(t, err)
+ assert.Contains(t, err.Error(), "mapping_remove failed:")
+ assert.Contains(t, err.Error(), "service_key=org.example.BarService")
+ assert.Contains(t, err.Error(), "report_url=mock://127.0.0.1:8848")
// both individual errors must be present in the returned error
require.ErrorIs(t, err, err1)
require.ErrorIs(t, err, err2)
@@ -343,6 +377,8 @@ func TestServiceNameMappingGetMetersPerBusinessOperation(t
*testing.T) {
r2.On("GetServiceAppMapping").Return(gxset.NewSet(), errors.New("r2
failure")).Once()
_, err = ins.Get(serviceUrl, &listener{})
require.Error(t, err)
+ assert.Contains(t, err.Error(), "mapping_listen failed:")
+ require.ErrorIs(t, err, getErr)
assertMappingMetricEvent(t, <-ch,
metricsMetadata.MetadataMappingListen, false, false,
mappingAttachment("org.example.MeteredService"))
assert.Empty(t, ch)
diff --git a/metadata/report_instance.go b/metadata/report_instance.go
index 11dffa07b..19e973e6e 100644
--- a/metadata/report_instance.go
+++ b/metadata/report_instance.go
@@ -18,6 +18,7 @@
package metadata
import (
+ "fmt"
"sort"
"sync"
"time"
@@ -146,6 +147,10 @@ func (d *DelegateMetadataReport)
PublishAppMetadata(application, revision string
event.Succ = err == nil
event.End = time.Now()
metrics.Publish(event)
+ if err != nil {
+ return fmt.Errorf("metadata_report failed: operation=publish
app=%s revision=%s storage_type=%s: %w",
+ application, revision,
constant.RemoteMetadataStorageType, err)
+ }
return err
}
@@ -156,6 +161,10 @@ func (d *DelegateMetadataReport)
GetAppMetadata(application, revision string) (*
event.Succ = err == nil
event.End = time.Now()
metrics.Publish(event)
+ if err != nil {
+ return meta, fmt.Errorf("metadata_report failed: operation=get
app=%s revision=%s storage_type=%s: %w",
+ application, revision,
constant.RemoteMetadataStorageType, err)
+ }
return meta, err
}
@@ -173,10 +182,20 @@ func (d *DelegateMetadataReport)
RemoveServiceAppMappingListener(interfaceName,
// UnPublishAppMetadata delegate unpublish metadata info
func (d *DelegateMetadataReport) UnPublishAppMetadata(application, revision
string) error {
- return d.instance.UnPublishAppMetadata(application, revision)
+ err := d.instance.UnPublishAppMetadata(application, revision)
+ if err != nil {
+ return fmt.Errorf("metadata_report failed: operation=unpublish
app=%s revision=%s storage_type=%s: %w",
+ application, revision,
constant.RemoteMetadataStorageType, err)
+ }
+ return nil
}
// ListAppRevisions delegate list app revisions
func (d *DelegateMetadataReport) ListAppRevisions(application string)
([]report.AppRevision, error) {
- return d.instance.ListAppRevisions(application)
+ revisions, err := d.instance.ListAppRevisions(application)
+ if err != nil {
+ return revisions, fmt.Errorf("metadata_report failed:
operation=list_revisions app=%s storage_type=%s: %w",
+ application, constant.RemoteMetadataStorageType, err)
+ }
+ return revisions, nil
}
diff --git a/metadata/report_instance_test.go b/metadata/report_instance_test.go
index 87f311d49..5724b72e0 100644
--- a/metadata/report_instance_test.go
+++ b/metadata/report_instance_test.go
@@ -66,9 +66,16 @@ func TestDelegateMetadataReportGetAppMetadata(t *testing.T) {
assert.True(t, event.Succ)
})
t.Run("error", func(t *testing.T) {
-
mockReport.On("GetAppMetadata").Return(info.NewAppMetadataInfo("dubbo"),
errors.New("mock error")).Once()
+ sourceErr := errors.New("mock error")
+
mockReport.On("GetAppMetadata").Return(info.NewAppMetadataInfo("dubbo"),
sourceErr).Once()
_, err := delegate.GetAppMetadata("dubbo", "1111")
require.Error(t, err)
+ assert.Contains(t, err.Error(), "metadata_report failed:")
+ assert.Contains(t, err.Error(), "operation=get")
+ assert.Contains(t, err.Error(), "app=dubbo")
+ assert.Contains(t, err.Error(), "revision=1111")
+ assert.Contains(t, err.Error(), "storage_type=remote")
+ require.ErrorIs(t, err, sourceErr)
assert.Len(t, ch, 1)
metricEvent := <-ch
assert.Equal(t, constant.MetricsMetadata, metricEvent.Type())
@@ -104,9 +111,16 @@ func TestDelegateMetadataReportPublishAppMetadata(t
*testing.T) {
assert.True(t, event.Succ)
})
t.Run("error", func(t *testing.T) {
- mockReport.On("PublishAppMetadata").Return(errors.New("mock
error")).Once()
+ sourceErr := errors.New("mock error")
+ mockReport.On("PublishAppMetadata").Return(sourceErr).Once()
err := delegate.PublishAppMetadata("application", "revision",
metadataInfo)
require.Error(t, err)
+ assert.Contains(t, err.Error(), "metadata_report failed:")
+ assert.Contains(t, err.Error(), "operation=publish")
+ assert.Contains(t, err.Error(), "app=application")
+ assert.Contains(t, err.Error(), "revision=revision")
+ assert.Contains(t, err.Error(), "storage_type=remote")
+ require.ErrorIs(t, err, sourceErr)
assert.Len(t, ch, 1)
metricEvent := <-ch
assert.Equal(t, constant.MetricsMetadata, metricEvent.Type())
@@ -189,9 +203,16 @@ func TestDelegateMetadataReportUnPublishAppMetadata(t
*testing.T) {
require.NoError(t, err)
})
t.Run("error", func(t *testing.T) {
- mockReport.On("UnPublishAppMetadata").Return(errors.New("mock
error")).Once()
+ sourceErr := errors.New("mock error")
+ mockReport.On("UnPublishAppMetadata").Return(sourceErr).Once()
err := delegate.UnPublishAppMetadata("application", "revision")
require.Error(t, err)
+ assert.Contains(t, err.Error(), "metadata_report failed:")
+ assert.Contains(t, err.Error(), "operation=unpublish")
+ assert.Contains(t, err.Error(), "app=application")
+ assert.Contains(t, err.Error(), "revision=revision")
+ assert.Contains(t, err.Error(), "storage_type=remote")
+ require.ErrorIs(t, err, sourceErr)
})
}
@@ -210,9 +231,15 @@ func TestDelegateMetadataReportListAppRevisions(t
*testing.T) {
assert.Equal(t, expected, got)
})
t.Run("error", func(t *testing.T) {
-
mockReport.On("ListAppRevisions").Return([]report.AppRevision(nil),
errors.New("mock error")).Once()
+ sourceErr := errors.New("mock error")
+
mockReport.On("ListAppRevisions").Return([]report.AppRevision(nil),
sourceErr).Once()
_, err := delegate.ListAppRevisions("application")
require.Error(t, err)
+ assert.Contains(t, err.Error(), "metadata_report failed:")
+ assert.Contains(t, err.Error(), "operation=list_revisions")
+ assert.Contains(t, err.Error(), "app=application")
+ assert.Contains(t, err.Error(), "storage_type=remote")
+ require.ErrorIs(t, err, sourceErr)
})
}
diff --git a/registry/servicediscovery/service_discovery_registry.go
b/registry/servicediscovery/service_discovery_registry.go
index dec7edabb..df41e5453 100644
--- a/registry/servicediscovery/service_discovery_registry.go
+++ b/registry/servicediscovery/service_discovery_registry.go
@@ -145,7 +145,8 @@ func (s *serviceDiscoveryRegistry) RegisterService() error {
if metadata.GetMetadataType() == constant.RemoteMetadataStorageType {
if s.metadataReport == nil {
- return errors.New("can not publish app metadata cause
report instance not found")
+ return fmt.Errorf("metadata_report failed:
operation=publish app=%s revision=%s registry_id=%s storage_type=%s: no
metadata report instance found",
+ metaInfo.App, metaInfo.Revision, registryId,
constant.RemoteMetadataStorageType)
}
if err := s.metadataReport.PublishAppMetadata(metaInfo.App,
metaInfo.Revision, metaInfo); err != nil {
return err
@@ -306,7 +307,8 @@ func (s *serviceDiscoveryRegistry)
syncExportedMetadataAfterUnregister(targetURL
}
if metadata.GetMetadataType() == constant.RemoteMetadataStorageType {
if s.metadataReport == nil {
- return errors.New("can not publish app metadata cause
report instance not found")
+ return fmt.Errorf("metadata_report failed:
operation=publish app=%s revision=%s registry_id=%s storage_type=%s: no
metadata report instance found",
+ metadataInfo.App, revision, registryId,
constant.RemoteMetadataStorageType)
}
if err := s.metadataReport.PublishAppMetadata(metadataInfo.App,
revision, metadataInfo); err != nil {
return err
diff --git a/registry/servicediscovery/service_discovery_registry_test.go
b/registry/servicediscovery/service_discovery_registry_test.go
index 608e9b29f..bc6740558 100644
--- a/registry/servicediscovery/service_discovery_registry_test.go
+++ b/registry/servicediscovery/service_discovery_registry_test.go
@@ -329,7 +329,13 @@ func
TestServiceDiscoveryRegistryRegisterNilReportReturnsError(t *testing.T) {
err = sdReg.RegisterService()
require.Error(t, err)
- assert.Contains(t, err.Error(), "report instance not found")
+ assert.Contains(t, err.Error(), "metadata_report failed:")
+ assert.Contains(t, err.Error(), "operation=publish")
+ assert.Contains(t, err.Error(), "app="+testApp)
+ assert.Contains(t, err.Error(), "revision=")
+ assert.Contains(t, err.Error(), "registry_id="+regID)
+ assert.Contains(t, err.Error(), "storage_type=remote")
+ assert.Contains(t, err.Error(), "no metadata report instance found")
assert.False(t, mockSD.registerCalled, "no instance should be
registered when the metadata report is nil")
}
diff --git
a/registry/servicediscovery/service_instances_changed_listener_impl.go
b/registry/servicediscovery/service_instances_changed_listener_impl.go
index 18599d1c4..2c69cf6f2 100644
--- a/registry/servicediscovery/service_instances_changed_listener_impl.go
+++ b/registry/servicediscovery/service_instances_changed_listener_impl.go
@@ -384,12 +384,14 @@ func GetMetadataInfoWithContext(ctx context.Context, app
string, instance regist
initCache(app)
})
cacheKey := metadataCacheKey(app, registryId, revision)
- if metadataInfo, ok := metaCache.Get(cacheKey); ok {
+ if cachedValue, ok := metaCache.Get(cacheKey); ok {
logger.Debugf("[Metadata][Cache] app=%s registry=%s revision=%s
host=%s result=hit",
app, registryId, revision, instance.GetHost())
publishMetadataCacheEvent(app, true)
publishMetadataFetchEvent(app, metricsMetadata.SourceCache, "",
nil)
- return metadataInfo.(*info.MetadataInfo), nil
+ metadataInfo := cachedValue.(*info.MetadataInfo)
+ logMetadataRevisionMismatch(metadataInfo,
metricsMetadata.SourceCache, app, revision, registryId, instance)
+ return metadataInfo, nil
}
logger.Debugf("[Metadata][Cache] app=%s registry=%s revision=%s host=%s
result=miss",
app, registryId, revision, instance.GetHost())
@@ -414,11 +416,24 @@ func GetMetadataInfoWithContext(ctx context.Context, app
string, instance regist
publishMetadataFetchEvent(app, source, metricStorageType, err)
return nil, err
}
+ logMetadataRevisionMismatch(metadataInfo, source, app, revision,
registryId, instance)
metaCache.Set(cacheKey, metadataInfo)
publishMetadataFetchEvent(app, source, metricStorageType, nil)
return metadataInfo, nil
}
+func logMetadataRevisionMismatch(metadataInfo *info.MetadataInfo, source, app,
expectedRevision, registryId string, instance registry.ServiceInstance) {
+ if metadataInfo == nil || metadataInfo.Revision == expectedRevision {
+ return
+ }
+ storageType := constant.DefaultMetadataStorageType
+ if instanceMetadata := instance.GetMetadata(); instanceMetadata != nil
&& instanceMetadata[constant.MetadataStorageTypePropertyName] != "" {
+ storageType =
instanceMetadata[constant.MetadataStorageTypePropertyName]
+ }
+ logger.Warnf("[Metadata] revision_mismatch failed: source=%s app=%s
expected_revision=%q actual_revision=%q registry_id=%s storage_type=%s
instance_id=%s host=%s",
+ source, app, expectedRevision, metadataInfo.Revision,
registryId, storageType, instance.GetID(), instance.GetHost())
+}
+
func publishMetadataCacheEvent(app string, hit bool) {
event :=
metricsMetadata.NewMetadataMetricTimeEvent(metricsMetadata.MetadataCache)
event.Succ = hit
@@ -566,20 +581,25 @@ func getRemoteMetadataInfo(ctx context.Context, app
string, instance registry.Se
metadataInfo, rpcErr := metadata.GetMetadataFromRpcWithContext(ctx,
revision, instance)
if rpcErr != nil {
- return nil, metricsMetadata.SourceRpc,
wrapMetadataRPCFallbackError(rpcErr, reportErr)
+ rpcErr = wrapMetadataRPCFallbackError(rpcErr, reportErr)
+ rpcErr = fmt.Errorf("%w; registry_id=%s", rpcErr, registryId)
+ return nil, metricsMetadata.SourceRpc, rpcErr
}
metadataInfo, rpcErr = requireMetadataInfo(metadataInfo, app,
registryId, revision)
+ if rpcErr != nil {
+ return nil, metricsMetadata.SourceRpc,
wrapMetadataRPCFallbackError(rpcErr, reportErr)
+ }
return metadataInfo, metricsMetadata.SourceRpc, rpcErr
}
func logMetadataReportFallback(app, registryId, revision string, reportErr
error) {
if reportErr != nil {
- logger.Errorf("[Metadata][Fallback] report failed, fallback to
RPC app=%s registry=%s revision=%s err=%v",
- app, registryId, revision, reportErr)
+ logger.Errorf("[Metadata][Fallback] report failed, fallback to
RPC app=%s registry=%s revision=%s storage_type=%s err=%v",
+ app, registryId, revision,
constant.RemoteMetadataStorageType, reportErr)
return
}
- logger.Warnf("[Metadata][Fallback] report returned nil metadata,
fallback to RPC app=%s registry=%s revision=%s",
- app, registryId, revision)
+ logger.Warnf("[Metadata][Fallback] report returned nil metadata,
fallback to RPC app=%s registry=%s revision=%s storage_type=%s",
+ app, registryId, revision, constant.RemoteMetadataStorageType)
}
func wrapMetadataRPCFallbackError(rpcErr, reportErr error) error {
@@ -593,8 +613,8 @@ func wrapMetadataRPCFallbackError(rpcErr, reportErr error)
error {
func requireMetadataInfo(metadataInfo *info.MetadataInfo, app, registryId,
revision string) (*info.MetadataInfo, error) {
if metadataInfo == nil {
- return nil, fmt.Errorf("got nil metadata from RPC app=%s
registry=%s revision=%s",
- app, registryId, revision)
+ return nil, fmt.Errorf("rpc_metadata failed: app=%s revision=%s
registry_id=%s: metadata is nil",
+ app, revision, registryId)
}
return metadataInfo, nil
}
diff --git
a/registry/servicediscovery/service_instances_changed_listener_impl_test.go
b/registry/servicediscovery/service_instances_changed_listener_impl_test.go
index 232f6b4dc..ef88f9e1c 100644
--- a/registry/servicediscovery/service_instances_changed_listener_impl_test.go
+++ b/registry/servicediscovery/service_instances_changed_listener_impl_test.go
@@ -524,6 +524,11 @@ func TestGetMetadataInfo_LocalStorageGoesDirectlyToRPC(t
*testing.T) {
require.Error(t, err)
// Must be a URL/RPC error, not a report error, confirming the local
path
// skips the report entirely and goes straight to RPC.
+ assert.Contains(t, err.Error(), "url_construction failed:")
+ assert.Contains(t, err.Error(), "app=test-app")
+ assert.Contains(t, err.Error(), "revision=rev-local-rpc")
+ assert.Contains(t, err.Error(), "registry=default")
+ assert.Contains(t, err.Error(), "storage_type=local")
assert.Contains(t, err.Error(), "metadata service URL params missing",
"local storage path should go directly to RPC, not touch the
metadata report")
}
@@ -552,8 +557,13 @@ func TestGetMetadataInfo_FallbackToRPC(t *testing.T) {
require.Error(t, err)
// Both report and RPC fail: the combined error proves the fallback
path was taken
// and includes the RPC/URL failure as the wrapped cause.
- assert.Contains(t, err.Error(), "both paths failed",
- "fallback path should produce a combined error mentioning both
failures")
+ assert.Contains(t, err.Error(), "url_construction failed:")
+ assert.Contains(t, err.Error(), "both paths failed, reportErr:
metadata_report failed:",
+ "fallback path should retain the report failure")
+ assert.Contains(t, err.Error(), "app=test-app")
+ assert.Contains(t, err.Error(), "revision=rev-fallback-to-rpc")
+ assert.Contains(t, err.Error(), "registry_id=default")
+ assert.Contains(t, err.Error(), "storage_type=remote")
assert.Contains(t, err.Error(), "metadata service URL params missing",
"fallback error should include the RPC/URL failure cause")
}
@@ -607,8 +617,11 @@ func TestGetMetadataInfo_ReportReturnsNil_FallsBackToRPC(t
*testing.T) {
require.Error(t, err)
// The report returned nil (no error), so the fallback was triggered
and then RPC
// failed at URL construction. The error must reflect the
RPC-after-nil-report path.
+ assert.Contains(t, err.Error(), "url_construction failed:")
assert.Contains(t, err.Error(), "RPC fallback failed after report
returned nil metadata",
"nil report result should trigger fallback and surface an RPC
error")
+ assert.Contains(t, err.Error(), "registry_id="+regID)
+ assert.Contains(t, err.Error(), "storage_type=remote")
assert.Contains(t, err.Error(), "metadata service URL params missing",
"fallback error should include the RPC/URL failure cause")
mockReport.AssertExpectations(t)
