From 06a3a29792f3abdddc40d406d2bca753b7e238c2 Mon Sep 17 00:00:00 2001 From: Denis Kirillov Date: Fri, 2 Aug 2024 16:33:39 +0300 Subject: [PATCH] [#452] tree: Fix logging Don't log parsing tags error in listing Signed-off-by: Denis Kirillov --- api/handler/handlers_test.go | 16 +++++++++------- api/handler/object_list_test.go | 30 ++++++++++++++++++++++++++++-- pkg/service/tree/tree.go | 8 ++++++-- 3 files changed, 43 insertions(+), 11 deletions(-) diff --git a/api/handler/handlers_test.go b/api/handler/handlers_test.go index 53752d6..c0669df 100644 --- a/api/handler/handlers_test.go +++ b/api/handler/handlers_test.go @@ -33,6 +33,7 @@ import ( "github.com/nspcc-dev/neo-go/pkg/crypto/keys" "github.com/stretchr/testify/require" "go.uber.org/zap" + "go.uber.org/zap/zaptest" "golang.org/x/exp/slices" ) @@ -141,18 +142,19 @@ func (c *configMock) Domains() []string { } func prepareHandlerContext(t *testing.T) *handlerContext { - return prepareHandlerContextBase(t, layer.DefaultCachesConfigs(zap.NewExample())) + log := zaptest.NewLogger(t) + return prepareHandlerContextBase(t, layer.DefaultCachesConfigs(log), log) } func prepareHandlerContextWithMinCache(t *testing.T) *handlerContext { - return prepareHandlerContextBase(t, getMinCacheConfig(zap.NewExample())) + log := zaptest.NewLogger(t) + return prepareHandlerContextBase(t, getMinCacheConfig(log), log) } -func prepareHandlerContextBase(t *testing.T, cacheCfg *layer.CachesConfig) *handlerContext { +func prepareHandlerContextBase(t *testing.T, cacheCfg *layer.CachesConfig, log *zap.Logger) *handlerContext { key, err := keys.NewPrivateKey() require.NoError(t, err) - l := zap.NewExample() tp := layer.NewTestFrostFS(key) testResolver := &resolver.Resolver{Name: "test_resolver"} @@ -166,7 +168,7 @@ func prepareHandlerContextBase(t *testing.T, cacheCfg *layer.CachesConfig) *hand memCli, err := tree.NewTreeServiceClientMemory() require.NoError(t, err) - treeMock := tree.NewTree(memCli, zap.NewExample()) + treeMock := tree.NewTree(memCli, log) features := &layer.FeatureSettingsMock{} @@ -187,8 +189,8 @@ func prepareHandlerContextBase(t *testing.T, cacheCfg *layer.CachesConfig) *hand defaultPolicy: pp, } h := &handler{ - log: l, - obj: layer.NewLayer(l, tp, layerCfg), + log: log, + obj: layer.NewLayer(log, tp, layerCfg), cfg: cfg, ape: newAPEMock(), frostfsid: newFrostfsIDMock(), diff --git a/api/handler/object_list_test.go b/api/handler/object_list_test.go index 4065d3e..ffde199 100644 --- a/api/handler/object_list_test.go +++ b/api/handler/object_list_test.go @@ -7,6 +7,7 @@ import ( "net/url" "sort" "strconv" + "strings" "testing" "time" @@ -15,8 +16,11 @@ import ( "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/data" "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/layer" "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/layer/encryption" + "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/internal/logs" "github.com/stretchr/testify/require" + "go.uber.org/zap" "go.uber.org/zap/zaptest" + "go.uber.org/zap/zaptest/observer" ) func TestParseContinuationToken(t *testing.T) { @@ -93,6 +97,27 @@ func TestListObjectsWithOldTreeNodes(t *testing.T) { checkListVersionsOldNodes(hc, listVers.Version, objInfos) } +func TestListObjectsVersionsSkipLogTaggingNodesError(t *testing.T) { + loggerCore, observedLog := observer.New(zap.DebugLevel) + log := zap.New(loggerCore) + hc := prepareHandlerContextBase(t, layer.DefaultCachesConfigs(log), log) + + bktName, objName := "bucket-versioning-enabled", "versions/object" + bktInfo := createTestBucket(hc, bktName) + + createTestObject(hc, bktInfo, objName, encryption.Params{}) + createTestObject(hc, bktInfo, objName, encryption.Params{}) + + putObjectTagging(hc.t, hc, bktName, objName, map[string]string{"tag1": "val1"}) + + listObjectsVersions(hc, bktName, "", "", "", "", -1) + + filtered := observedLog.Filter(func(entry observer.LoggedEntry) bool { + return strings.Contains(entry.Message, logs.ParseTreeNode) + }) + require.Empty(t, filtered) +} + func makeAllTreeObjectsOld(hc *handlerContext, bktInfo *data.BucketInfo) { nodes, err := hc.treeMock.GetSubTree(hc.Context(), bktInfo, "version", []uint64{0}, 0) require.NoError(hc.t, err) @@ -138,11 +163,12 @@ func checkListVersionsOldNodes(hc *handlerContext, list []ObjectVersionResponse, } func TestListObjectsContextCanceled(t *testing.T) { - layerCfg := layer.DefaultCachesConfigs(zaptest.NewLogger(t)) + log := zaptest.NewLogger(t) + layerCfg := layer.DefaultCachesConfigs(log) layerCfg.SessionList.Lifetime = time.Hour layerCfg.SessionList.Size = 1 - hc := prepareHandlerContextBase(t, layerCfg) + hc := prepareHandlerContextBase(t, layerCfg, log) bktName := "bucket-versioning-enabled" bktInfo := createTestBucket(hc, bktName) diff --git a/pkg/service/tree/tree.go b/pkg/service/tree/tree.go index a9afc91..79a4fe1 100644 --- a/pkg/service/tree/tree.go +++ b/pkg/service/tree/tree.go @@ -82,6 +82,8 @@ var ( // ErrGatewayTimeout is returned from ServiceClient service in case of timeout error. ErrGatewayTimeout = layer.ErrGatewayTimeout + + errNodeDoesntContainFileName = fmt.Errorf("node doesn't contain FileName") ) const ( @@ -1019,7 +1021,9 @@ func (s *VersionsByPrefixStreamImpl) getNodeVersionFromInnerStream() (*data.Node func (s *VersionsByPrefixStreamImpl) parseNodeResponse(node NodeResponse) (res *data.NodeVersion, skip bool, err error) { trNode, fileName, err := parseTreeNode(node) if err != nil { - s.log.Debug(logs.ParseTreeNode, zap.Error(err)) + if !errors.Is(err, errNodeDoesntContainFileName) { + s.log.Debug(logs.ParseTreeNode, zap.Error(err)) + } return nil, true, nil } @@ -1230,7 +1234,7 @@ func parseTreeNode(node NodeResponse) (*treeNode, string, error) { fileName, ok := tNode.FileName() if !ok { - return nil, "", fmt.Errorf("doesn't contain FileName") + return nil, "", errNodeDoesntContainFileName } return tNode, fileName, nil