feat(metadata): classify errors and add useful context (#3707)
* feat: classify errors, enrich them with useful context, and add failure-path tests
* fix test
diff --git a/metadata/client.go b/metadata/client.go
index 15169d6..791ce0a 100644
--- a/metadata/client.go
+++ b/metadata/client.go
@@ -45,11 +45,12 @@
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 @@
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 @@
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 @@
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 c334134..1c15368 100644
--- a/metadata/client_test.go
+++ b/metadata/client_test.go
@@ -91,6 +91,20 @@
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 @@
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 @@
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 @@
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 @@
}
_, 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 @@
}
_, 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 4202e9d..09d0307 100644
--- a/metadata/mapping/metadata/service_name_mapping.go
+++ b/metadata/mapping/metadata/service_name_mapping.go
@@ -19,6 +19,7 @@
import (
"errors"
+ "fmt"
"sync"
"time"
)
@@ -74,6 +75,7 @@
// 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 @@
// 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 @@
// 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 @@
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 @@
}
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 @@
// 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 @@
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 7c5b384..882b4a2 100644
--- a/metadata/mapping/metadata/service_name_mapping_test.go
+++ b/metadata/mapping/metadata/service_name_mapping_test.go
@@ -61,10 +61,17 @@
)
_, 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 @@
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 @@
_, 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 @@
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 @@
})
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 @@
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 @@
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 @@
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 @@
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 11dffa0..19e973e 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 @@
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 @@
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 @@
// 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 87f311d..5724b72 100644
--- a/metadata/report_instance_test.go
+++ b/metadata/report_instance_test.go
@@ -66,9 +66,16 @@
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 @@
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 @@
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 @@
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 dec7eda..df41e54 100644
--- a/registry/servicediscovery/service_discovery_registry.go
+++ b/registry/servicediscovery/service_discovery_registry.go
@@ -145,7 +145,8 @@
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 @@
}
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 608e9b2..bc67405 100644
--- a/registry/servicediscovery/service_discovery_registry_test.go
+++ b/registry/servicediscovery/service_discovery_registry_test.go
@@ -329,7 +329,13 @@
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 18599d1..2c69cf6 100644
--- a/registry/servicediscovery/service_instances_changed_listener_impl.go
+++ b/registry/servicediscovery/service_instances_changed_listener_impl.go
@@ -384,12 +384,14 @@
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 @@
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 @@
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 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 232f6b4..ef88f9e 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 @@
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 @@
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 @@
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)
