From 53f36f11cb78350dc8f0e66985989a638780023a Mon Sep 17 00:00:00 2001 From: Kanan Bayramov Date: Fri, 12 Apr 2024 23:03:30 +0900 Subject: [PATCH 01/25] TraceLog changes --- jsonrpc/server.go | 13 +++++++++++++ p2p/sync.go | 8 ++++++-- upgrader/upgrader_test.go | 5 +++++ utils/log.go | 29 ++++++++++++++++++++++------- 4 files changed, 46 insertions(+), 9 deletions(-) diff --git a/jsonrpc/server.go b/jsonrpc/server.go index 1a2339b156..fbf5cfacd9 100644 --- a/jsonrpc/server.go +++ b/jsonrpc/server.go @@ -397,9 +397,14 @@ func isNil(i any) bool { } func (s *Server) handleRequest(ctx context.Context, req *Request) (*response, error) { + reqJSON, _ := json.Marshal(req) // Serialize the request early for use in logging + s.log.Tracew("Received request", "req", string(reqJSON)) + if err := req.isSane(); err != nil { + s.log.Tracew("Request sanity check failed", "error", err.Error()) return nil, err } + res := &response{ Version: "2.0", @@ -409,14 +414,18 @@ func (s *Server) handleRequest(ctx context.Context, req *Request) (*response, er calledMethod, found := s.methods[req.Method] if !found { res.Error = Err(MethodNotFound, nil) + s.log.Tracew("Method not found in request", "method", req.Method) return res, nil } handlerTimer := time.Now() s.listener.OnNewRequest(req.Method) + s.log.Tracew("Handling new RPC request", "method", req.Method) + args, err := s.buildArguments(ctx, req.Params, calledMethod) if err != nil { res.Error = Err(InvalidParams, err.Error()) + s.log.Tracew("Error building arguments for RPC call", "error", err.Error()) return res, nil } defer func() { @@ -425,6 +434,7 @@ func (s *Server) handleRequest(ctx context.Context, req *Request) (*response, er tuple := reflect.ValueOf(calledMethod.Handler).Call(args) if res.ID == nil { // notification + s.log.Tracew("Notification received, no response expected") return nil, nil } @@ -435,10 +445,13 @@ func (s *Server) handleRequest(ctx context.Context, req *Request) (*response, er reqJSON, _ := json.Marshal(req) errJSON, _ := json.Marshal(res.Error) s.log.Debugw("Failed handing RPC request", "req", string(reqJSON), "res", string(errJSON)) + s.log.Tracew("Internal error during RPC handling", "req", string(reqJSON), "error", string(errJSON)) } return res, nil } res.Result = tuple[0].Interface() + resJSON, _ := json.Marshal(res) // Serialize response for logging + s.log.Tracew("Successfully handled RPC request", "res", string(resJSON)) return res, nil } diff --git a/p2p/sync.go b/p2p/sync.go index 635673891d..a2bd8389fa 100644 --- a/p2p/sync.go +++ b/p2p/sync.go @@ -195,15 +195,19 @@ func (s *syncService) logError(msg string, err error) { if !errors.Is(err, context.Canceled) { var log utils.SimpleLogger if v, ok := s.log.(*utils.ZapLogger); ok { - log = v.WithOptions(zap.AddCallerSkip(1)) + enhancedLogger := v.SugaredLogger.Desugar().WithOptions(zap.AddCallerSkip(1)).Sugar() + log = &utils.ZapLogger{SugaredLogger: enhancedLogger} } else { log = s.log } - log.Errorw(msg, "err", err) + log.Tracew("Logging error", "msg", msg, "error", err.Error()) + // Log the error + log.Errorw(msg, "err", err) } } + // blockBody is used to mange all the different parts of the blocks require to store the block in the blockchain.Store() type blockBody struct { block *core.Block diff --git a/upgrader/upgrader_test.go b/upgrader/upgrader_test.go index 4b26342d77..213eed0b5e 100644 --- a/upgrader/upgrader_test.go +++ b/upgrader/upgrader_test.go @@ -19,6 +19,7 @@ type upgradeLogger struct { infoMsg string warnMsg string errorMsg string + traceMsg string } func (l *upgradeLogger) Debugw(msg string, keysAndValues ...any) {} @@ -35,6 +36,10 @@ func (l *upgradeLogger) Errorw(msg string, keysAndValues ...any) { l.errorMsg = msg } +func (l *upgradeLogger) Tracew(msg string, keysAndValues ...any) { + l.traceMsg = msg +} + func newVersion(t *testing.T, v string) semver.Version { version, err := semver.StrictNewVersion(v) require.NoError(t, err) diff --git a/utils/log.go b/utils/log.go index f9f4b837c8..9b4b4bbb9d 100644 --- a/utils/log.go +++ b/utils/log.go @@ -2,19 +2,16 @@ package utils import ( "encoding" - "fmt" "time" - + "errors" "github.com/cockroachdb/pebble" "github.com/spf13/pflag" "go.uber.org/zap" "go.uber.org/zap/zapcore" ) -var ErrUnknownLogLevel = fmt.Errorf( - "unknown log level (known: %s, %s, %s, %s)", - DEBUG, INFO, WARN, ERROR, -) +var ErrUnknownLogLevel = errors.New("unknown log level (known: debug, info, warn, error, trace)") + type LogLevel int @@ -30,6 +27,7 @@ const ( INFO WARN ERROR + TRACE ) func (l LogLevel) String() string { @@ -42,6 +40,8 @@ func (l LogLevel) String() string { return "warn" case ERROR: return "error" + case TRACE: + return "trace" default: // Should not happen. panic(ErrUnknownLogLevel) @@ -62,6 +62,8 @@ func (l *LogLevel) Set(s string) error { *l = WARN case "ERROR", "error": *l = ERROR + case "TRACE", "trace": // Adding the TRACE case + *l = TRACE default: return ErrUnknownLogLevel } @@ -90,13 +92,20 @@ type SimpleLogger interface { Infow(msg string, keysAndValues ...any) Warnw(msg string, keysAndValues ...any) Errorw(msg string, keysAndValues ...any) + Tracew(msg string, keysAndValues ...any) } type ZapLogger struct { *zap.SugaredLogger } -var _ Logger = (*ZapLogger)(nil) +func (l *ZapLogger) Tracew(msg string, keysAndValues ...interface{}) { + l.Debugw("[TRACE] "+msg, keysAndValues...) +} + + + +var _ SimpleLogger = (*ZapLogger)(nil) func NewNopZapLogger() *ZapLogger { return &ZapLogger{zap.NewNop().Sugar()} @@ -117,7 +126,12 @@ func NewZapLogger(logLevel LogLevel, colour bool) (*ZapLogger, error) { if err != nil { return nil, err } + if logLevel == TRACE { + config.Level.SetLevel(zapcore.DebugLevel) // Adjust according to how you define TRACE + } else { config.Level.SetLevel(level) + } + log, err := config.Build() if err != nil { return nil, err @@ -129,3 +143,4 @@ func NewZapLogger(logLevel LogLevel, colour bool) (*ZapLogger, error) { func (l *ZapLogger) Warningf(msg string, args ...any) { l.Warnf(msg, args) } + From 29adb1f2f291c677186e7b4aa8ee5bbed2f1f80c Mon Sep 17 00:00:00 2001 From: Kanan Bayramov Date: Sat, 13 Apr 2024 00:09:20 +0900 Subject: [PATCH 02/25] New changes --- jsonrpc/server.go | 11 +++++------ p2p/sync.go | 7 +++---- utils/log.go | 17 +++++++---------- utils/log_test.go | 2 +- utils/network_test.go | 2 +- 5 files changed, 17 insertions(+), 22 deletions(-) diff --git a/jsonrpc/server.go b/jsonrpc/server.go index fbf5cfacd9..d18474aa24 100644 --- a/jsonrpc/server.go +++ b/jsonrpc/server.go @@ -397,14 +397,13 @@ func isNil(i any) bool { } func (s *Server) handleRequest(ctx context.Context, req *Request) (*response, error) { - reqJSON, _ := json.Marshal(req) // Serialize the request early for use in logging - s.log.Tracew("Received request", "req", string(reqJSON)) - + reqJSON, _ := json.Marshal(req) // Serialise the request early for use in logging + s.log.Tracew("Received request", "req", string(reqJSON)) + if err := req.isSane(); err != nil { s.log.Tracew("Request sanity check failed", "error", err.Error()) return nil, err } - res := &response{ Version: "2.0", @@ -450,8 +449,8 @@ func (s *Server) handleRequest(ctx context.Context, req *Request) (*response, er return res, nil } res.Result = tuple[0].Interface() - resJSON, _ := json.Marshal(res) // Serialize response for logging - s.log.Tracew("Successfully handled RPC request", "res", string(resJSON)) + resJSON, _ := json.Marshal(res) // Serialise response for logging + s.log.Tracew("Successfully handled RPC request", "res", string(resJSON)) return res, nil } diff --git a/p2p/sync.go b/p2p/sync.go index a2bd8389fa..c835de9b61 100644 --- a/p2p/sync.go +++ b/p2p/sync.go @@ -196,18 +196,17 @@ func (s *syncService) logError(msg string, err error) { var log utils.SimpleLogger if v, ok := s.log.(*utils.ZapLogger); ok { enhancedLogger := v.SugaredLogger.Desugar().WithOptions(zap.AddCallerSkip(1)).Sugar() - log = &utils.ZapLogger{SugaredLogger: enhancedLogger} + log = &utils.ZapLogger{SugaredLogger: enhancedLogger} } else { log = s.log } log.Tracew("Logging error", "msg", msg, "error", err.Error()) - // Log the error - log.Errorw(msg, "err", err) + // Log the error + log.Errorw(msg, "err", err) } } - // blockBody is used to mange all the different parts of the blocks require to store the block in the blockchain.Store() type blockBody struct { block *core.Block diff --git a/utils/log.go b/utils/log.go index 9b4b4bbb9d..0d782db37b 100644 --- a/utils/log.go +++ b/utils/log.go @@ -62,8 +62,8 @@ func (l *LogLevel) Set(s string) error { *l = WARN case "ERROR", "error": *l = ERROR - case "TRACE", "trace": // Adding the TRACE case - *l = TRACE + case "TRACE", "trace": // Adding the TRACE case + *l = TRACE default: return ErrUnknownLogLevel } @@ -100,11 +100,9 @@ type ZapLogger struct { } func (l *ZapLogger) Tracew(msg string, keysAndValues ...interface{}) { - l.Debugw("[TRACE] "+msg, keysAndValues...) + l.Debugw("[TRACE] "+msg, keysAndValues...) } - - var _ SimpleLogger = (*ZapLogger)(nil) func NewNopZapLogger() *ZapLogger { @@ -127,11 +125,11 @@ func NewZapLogger(logLevel LogLevel, colour bool) (*ZapLogger, error) { return nil, err } if logLevel == TRACE { - config.Level.SetLevel(zapcore.DebugLevel) // Adjust according to how you define TRACE - } else { - config.Level.SetLevel(level) + config.Level.SetLevel(zapcore.DebugLevel) // Adjust according to how you define TRACE + } else { + config.Level.SetLevel(level) } - + log, err := config.Build() if err != nil { return nil, err @@ -143,4 +141,3 @@ func NewZapLogger(logLevel LogLevel, colour bool) (*ZapLogger, error) { func (l *ZapLogger) Warningf(msg string, args ...any) { l.Warnf(msg, args) } - diff --git a/utils/log_test.go b/utils/log_test.go index a3f782a1db..1dfbb116d8 100644 --- a/utils/log_test.go +++ b/utils/log_test.go @@ -19,7 +19,7 @@ var levelStrings = map[utils.LogLevel]string{ func TestLogLevelString(t *testing.T) { for level, str := range levelStrings { - t.Run("level "+str, func(t *testing.T) { //nolint:goconst + t.Run("level "+str, func(t *testing.T) { assert.Equal(t, str, level.String()) }) } diff --git a/utils/network_test.go b/utils/network_test.go index 9df032881f..dc522f0927 100644 --- a/utils/network_test.go +++ b/utils/network_test.go @@ -89,7 +89,7 @@ func TestNetwork(t *testing.T) { //nolint:dupl // see comment in utils/log_test.go func TestNetworkSet(t *testing.T) { for network, str := range networkStrings { - t.Run("network "+str, func(t *testing.T) { //nolint:goconst + t.Run("network "+str, func(t *testing.T) { n := new(utils.Network) require.NoError(t, n.Set(str)) assert.Equal(t, network, *n) From d1f664cfea4e0a169bca12c58bcaf92d09da3229 Mon Sep 17 00:00:00 2001 From: Kanan Bayramov Date: Sat, 13 Apr 2024 01:02:57 +0900 Subject: [PATCH 03/25] new updates --- .golangci.yaml | 18 ++---------------- p2p/sync.go | 2 +- 2 files changed, 3 insertions(+), 17 deletions(-) diff --git a/.golangci.yaml b/.golangci.yaml index 068f99cd04..7f272ff49c 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -46,15 +46,6 @@ linters-settings: ignored-functions: - strings.SplitN - govet: - check-shadowing: true - settings: - printf: - funcs: - - (github.com/golangci/golangci-lint/pkg/logutils.Log).Infof - - (github.com/golangci/golangci-lint/pkg/logutils.Log).Warnf - - (github.com/golangci/golangci-lint/pkg/logutils.Log).Errorf - - (github.com/golangci/golangci-lint/pkg/logutils.Log).Fatalf lll: line-length: 140 misspell: @@ -99,6 +90,7 @@ linters: - whitespace - tparallel - gci + # don't enable: # - asciicheck @@ -146,10 +138,4 @@ issues: - path: pkg/golinters/unused.go text: "rangeValCopy: each iteration copies 160 bytes \\(consider pointers or indexing\\)" -run: - timeout: 5m - skip-dirs: - - test/testdata_etc # test files - - internal/cache # extracted from Go code - - internal/renameio # extracted from Go code - - internal/robustio # extracted from Go code + diff --git a/p2p/sync.go b/p2p/sync.go index c835de9b61..a7c256a69e 100644 --- a/p2p/sync.go +++ b/p2p/sync.go @@ -101,7 +101,7 @@ func (s *syncService) start(ctx context.Context) { } var nextHeight int - if curHeight, err := s.blockchain.Height(); err == nil { //nolint:govet + if curHeight, err := s.blockchain.Height(); err == nil { nextHeight = int(curHeight) + 1 } else if !errors.Is(db.ErrKeyNotFound, err) { s.log.Errorw("Failed to get current height", "err", err) From 1df84c2474e94e2d7d4f186a5a956df54de97c42 Mon Sep 17 00:00:00 2001 From: Kanan Bayramov Date: Sat, 13 Apr 2024 01:42:39 +0900 Subject: [PATCH 04/25] new update --- db/pebble/transaction.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/db/pebble/transaction.go b/db/pebble/transaction.go index c07b499144..2bef0c5bb2 100644 --- a/db/pebble/transaction.go +++ b/db/pebble/transaction.go @@ -94,7 +94,7 @@ func (t *Transaction) Get(key []byte, cb func([]byte) error) error { return ErrDiscardedTransaction } - defer t.listener.OnIO(false, time.Since(start)) //nolint:govet + t.listener.OnIO(false, time.Since(start)) if err != nil { if errors.Is(err, pebble.ErrNotFound) { return db.ErrKeyNotFound From 50b5318be67fc3e962128efdd29dcf648b4b9156 Mon Sep 17 00:00:00 2001 From: Kanan Bayramov Date: Mon, 15 Apr 2024 20:11:10 +0900 Subject: [PATCH 05/25] Updates --- .golangci.yaml | 12 +++++++++++- p2p/sync.go | 3 ++- utils/log.go | 2 +- 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/.golangci.yaml b/.golangci.yaml index 7f272ff49c..2fe21fe191 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -46,6 +46,15 @@ linters-settings: ignored-functions: - strings.SplitN + govet: + check-shadowing: true + settings: + printf: + funcs: + - (github.com/golangci/golangci-lint/pkg/logutils.Log).Infof + - (github.com/golangci/golangci-lint/pkg/logutils.Log).Warnf + - (github.com/golangci/golangci-lint/pkg/logutils.Log).Errorf + - (github.com/golangci/golangci-lint/pkg/logutils.Log).Fatalf lll: line-length: 140 misspell: @@ -90,7 +99,7 @@ linters: - whitespace - tparallel - gci - + # don't enable: # - asciicheck @@ -139,3 +148,4 @@ issues: text: "rangeValCopy: each iteration copies 160 bytes \\(consider pointers or indexing\\)" + \ No newline at end of file diff --git a/p2p/sync.go b/p2p/sync.go index a7c256a69e..df94a408e4 100644 --- a/p2p/sync.go +++ b/p2p/sync.go @@ -101,7 +101,8 @@ func (s *syncService) start(ctx context.Context) { } var nextHeight int - if curHeight, err := s.blockchain.Height(); err == nil { + curHeight, err := s.blockchain.Height() + if err == nil { nextHeight = int(curHeight) + 1 } else if !errors.Is(db.ErrKeyNotFound, err) { s.log.Errorw("Failed to get current height", "err", err) diff --git a/utils/log.go b/utils/log.go index 0d782db37b..becb96c05c 100644 --- a/utils/log.go +++ b/utils/log.go @@ -62,7 +62,7 @@ func (l *LogLevel) Set(s string) error { *l = WARN case "ERROR", "error": *l = ERROR - case "TRACE", "trace": // Adding the TRACE case + case "TRACE", "trace": *l = TRACE default: return ErrUnknownLogLevel From e6bc8c445bf351510d39ea78b6821343d3cc5664 Mon Sep 17 00:00:00 2001 From: Kanan Bayramov Date: Mon, 15 Apr 2024 20:24:19 +0900 Subject: [PATCH 06/25] update --- .golangci.yaml | 9 --------- 1 file changed, 9 deletions(-) diff --git a/.golangci.yaml b/.golangci.yaml index 2fe21fe191..d94e79328f 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -46,15 +46,6 @@ linters-settings: ignored-functions: - strings.SplitN - govet: - check-shadowing: true - settings: - printf: - funcs: - - (github.com/golangci/golangci-lint/pkg/logutils.Log).Infof - - (github.com/golangci/golangci-lint/pkg/logutils.Log).Warnf - - (github.com/golangci/golangci-lint/pkg/logutils.Log).Errorf - - (github.com/golangci/golangci-lint/pkg/logutils.Log).Fatalf lll: line-length: 140 misspell: From 515a9639a43249aba24afdc34ee166e9b16ac9e5 Mon Sep 17 00:00:00 2001 From: Kanan Bayramov Date: Mon, 15 Apr 2024 20:53:57 +0900 Subject: [PATCH 07/25] update --- .golangci.yaml | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/.golangci.yaml b/.golangci.yaml index d94e79328f..96b40d87cd 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -46,6 +46,15 @@ linters-settings: ignored-functions: - strings.SplitN + govet: + check-shadowing: true + settings: + printf: + funcs: + - (github.com/golangci/golangci-lint/pkg/logutils.Log).Infof + - (github.com/golangci/golangci-lint/pkg/logutils.Log).Warnf + - (github.com/golangci/golangci-lint/pkg/logutils.Log).Errorf + - (github.com/golangci/golangci-lint/pkg/logutils.Log).Fatalf lll: line-length: 140 misspell: @@ -91,7 +100,6 @@ linters: - tparallel - gci - # don't enable: # - asciicheck # - scopelint @@ -138,5 +146,10 @@ issues: - path: pkg/golinters/unused.go text: "rangeValCopy: each iteration copies 160 bytes \\(consider pointers or indexing\\)" - - \ No newline at end of file +run: + timeout: 5m + skip-dirs: + - test/testdata_etc # test files + - internal/cache # extracted from Go code + - internal/renameio # extracted from Go code + - internal/robustio # extracted from Go code \ No newline at end of file From 328b4a6f3e628c3a7735bce41e1dfbb37e49fdef Mon Sep 17 00:00:00 2001 From: Kanan Bayramov Date: Mon, 15 Apr 2024 22:14:43 +0900 Subject: [PATCH 08/25] latest update --- utils/log_test.go | 2 +- utils/network_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/utils/log_test.go b/utils/log_test.go index 1dfbb116d8..1df287daf6 100644 --- a/utils/log_test.go +++ b/utils/log_test.go @@ -19,7 +19,7 @@ var levelStrings = map[utils.LogLevel]string{ func TestLogLevelString(t *testing.T) { for level, str := range levelStrings { - t.Run("level "+str, func(t *testing.T) { + t.Run("level "+str, func(t *testing.T) { // goconst assert.Equal(t, str, level.String()) }) } diff --git a/utils/network_test.go b/utils/network_test.go index dc522f0927..aac791fd84 100644 --- a/utils/network_test.go +++ b/utils/network_test.go @@ -89,7 +89,7 @@ func TestNetwork(t *testing.T) { //nolint:dupl // see comment in utils/log_test.go func TestNetworkSet(t *testing.T) { for network, str := range networkStrings { - t.Run("network "+str, func(t *testing.T) { + t.Run("network "+str, func(t *testing.T) { // goconst n := new(utils.Network) require.NoError(t, n.Set(str)) assert.Equal(t, network, *n) From 062d74ce89277ae59ff1c75ddfdc84ecdb3cbb6b Mon Sep 17 00:00:00 2001 From: Kanan Bayramov Date: Mon, 15 Apr 2024 22:54:03 +0900 Subject: [PATCH 09/25] new commit --- utils/log_test.go | 2 +- utils/network_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/utils/log_test.go b/utils/log_test.go index 1df287daf6..507c59eb64 100644 --- a/utils/log_test.go +++ b/utils/log_test.go @@ -19,7 +19,7 @@ var levelStrings = map[utils.LogLevel]string{ func TestLogLevelString(t *testing.T) { for level, str := range levelStrings { - t.Run("level "+str, func(t *testing.T) { // goconst + t.Run("level "+str, func(t *testing.T) { //golint:goconst assert.Equal(t, str, level.String()) }) } diff --git a/utils/network_test.go b/utils/network_test.go index aac791fd84..d5fc9e3165 100644 --- a/utils/network_test.go +++ b/utils/network_test.go @@ -89,7 +89,7 @@ func TestNetwork(t *testing.T) { //nolint:dupl // see comment in utils/log_test.go func TestNetworkSet(t *testing.T) { for network, str := range networkStrings { - t.Run("network "+str, func(t *testing.T) { // goconst + t.Run("network "+str, func(t *testing.T) { //golint:goconst n := new(utils.Network) require.NoError(t, n.Set(str)) assert.Equal(t, network, *n) From 828c1469f3851132267c346155b95eb2a586a458 Mon Sep 17 00:00:00 2001 From: Kanan Bayramov Date: Mon, 15 Apr 2024 23:26:40 +0900 Subject: [PATCH 10/25] changes --- utils/log_test.go | 2 +- utils/network_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/utils/log_test.go b/utils/log_test.go index 507c59eb64..1dfbb116d8 100644 --- a/utils/log_test.go +++ b/utils/log_test.go @@ -19,7 +19,7 @@ var levelStrings = map[utils.LogLevel]string{ func TestLogLevelString(t *testing.T) { for level, str := range levelStrings { - t.Run("level "+str, func(t *testing.T) { //golint:goconst + t.Run("level "+str, func(t *testing.T) { assert.Equal(t, str, level.String()) }) } diff --git a/utils/network_test.go b/utils/network_test.go index d5fc9e3165..dc522f0927 100644 --- a/utils/network_test.go +++ b/utils/network_test.go @@ -89,7 +89,7 @@ func TestNetwork(t *testing.T) { //nolint:dupl // see comment in utils/log_test.go func TestNetworkSet(t *testing.T) { for network, str := range networkStrings { - t.Run("network "+str, func(t *testing.T) { //golint:goconst + t.Run("network "+str, func(t *testing.T) { n := new(utils.Network) require.NoError(t, n.Set(str)) assert.Equal(t, network, *n) From 8969a9c09461685614fd1ac0678b85009512ed41 Mon Sep 17 00:00:00 2001 From: Kanan Bayramov Date: Mon, 15 Apr 2024 23:44:52 +0900 Subject: [PATCH 11/25] networklabel --- utils/network_test.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/utils/network_test.go b/utils/network_test.go index dc522f0927..94f5524cd9 100644 --- a/utils/network_test.go +++ b/utils/network_test.go @@ -13,6 +13,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) +const networkLabel = "network " var networkStrings = map[utils.Network]string{ utils.Mainnet: "mainnet", @@ -89,19 +90,19 @@ func TestNetwork(t *testing.T) { //nolint:dupl // see comment in utils/log_test.go func TestNetworkSet(t *testing.T) { for network, str := range networkStrings { - t.Run("network "+str, func(t *testing.T) { + t.Run(networkLabel + str, func(t *testing.T) { n := new(utils.Network) require.NoError(t, n.Set(str)) assert.Equal(t, network, *n) }) uppercase := strings.ToUpper(str) - t.Run("network "+uppercase, func(t *testing.T) { + t.Run(networkLabel + uppercase, func(t *testing.T) { n := new(utils.Network) require.NoError(t, n.Set(uppercase)) assert.Equal(t, network, *n) }) } - t.Run("unknown network", func(t *testing.T) { + t.Run("unknown " + networkLabel, func(t *testing.T) { n := utils.Network{} require.Error(t, n.Set("blah")) }) @@ -109,13 +110,13 @@ func TestNetworkSet(t *testing.T) { func TestNetworkUnmarshalText(t *testing.T) { for network, str := range networkStrings { - t.Run("network "+str, func(t *testing.T) { + t.Run(networkLabel +str, func(t *testing.T) { n := new(utils.Network) require.NoError(t, n.UnmarshalText([]byte(str))) assert.Equal(t, network, *n) }) uppercase := strings.ToUpper(str) - t.Run("network "+uppercase, func(t *testing.T) { + t.Run(networkLabel + uppercase, func(t *testing.T) { n := new(utils.Network) require.NoError(t, n.UnmarshalText([]byte(uppercase))) assert.Equal(t, network, *n) From f3ef97199b6f2ee63a313dfb07f0f845d50d2920 Mon Sep 17 00:00:00 2001 From: Kanan Bayramov Date: Mon, 15 Apr 2024 23:50:29 +0900 Subject: [PATCH 12/25] networklabel2 --- utils/log_test.go | 2 -- utils/network_test.go | 12 ++++++------ 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/utils/log_test.go b/utils/log_test.go index 1dfbb116d8..b87eedae28 100644 --- a/utils/log_test.go +++ b/utils/log_test.go @@ -29,8 +29,6 @@ func TestLogLevelString(t *testing.T) { // both implement the pflag.Value and encoding.TextUnmarshaller interfaces. // We can open a PR on github.com/thediveo/enumflag to add TextUnmarshaller // support. -// -//nolint:dupl func TestLogLevelSet(t *testing.T) { for level, str := range levelStrings { t.Run("level "+str, func(t *testing.T) { diff --git a/utils/network_test.go b/utils/network_test.go index 94f5524cd9..348b8c9a81 100644 --- a/utils/network_test.go +++ b/utils/network_test.go @@ -13,6 +13,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) + const networkLabel = "network " var networkStrings = map[utils.Network]string{ @@ -87,22 +88,21 @@ func TestNetwork(t *testing.T) { }) } -//nolint:dupl // see comment in utils/log_test.go func TestNetworkSet(t *testing.T) { for network, str := range networkStrings { - t.Run(networkLabel + str, func(t *testing.T) { + t.Run(networkLabel+str, func(t *testing.T) { n := new(utils.Network) require.NoError(t, n.Set(str)) assert.Equal(t, network, *n) }) uppercase := strings.ToUpper(str) - t.Run(networkLabel + uppercase, func(t *testing.T) { + t.Run(networkLabel+uppercase, func(t *testing.T) { n := new(utils.Network) require.NoError(t, n.Set(uppercase)) assert.Equal(t, network, *n) }) } - t.Run("unknown " + networkLabel, func(t *testing.T) { + t.Run("unknown "+networkLabel, func(t *testing.T) { n := utils.Network{} require.Error(t, n.Set("blah")) }) @@ -110,13 +110,13 @@ func TestNetworkSet(t *testing.T) { func TestNetworkUnmarshalText(t *testing.T) { for network, str := range networkStrings { - t.Run(networkLabel +str, func(t *testing.T) { + t.Run(networkLabel+str, func(t *testing.T) { n := new(utils.Network) require.NoError(t, n.UnmarshalText([]byte(str))) assert.Equal(t, network, *n) }) uppercase := strings.ToUpper(str) - t.Run(networkLabel + uppercase, func(t *testing.T) { + t.Run(networkLabel+uppercase, func(t *testing.T) { n := new(utils.Network) require.NoError(t, n.UnmarshalText([]byte(uppercase))) assert.Equal(t, network, *n) From 1b43e75f413cbccdfea2f10f466b33090370f511 Mon Sep 17 00:00:00 2001 From: Kanan Bayramov Date: Tue, 16 Apr 2024 00:11:33 +0900 Subject: [PATCH 13/25] levelLabel --- utils/log_test.go | 10 +++++++--- utils/network_test.go | 1 + 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/utils/log_test.go b/utils/log_test.go index b87eedae28..ee52c0fc0c 100644 --- a/utils/log_test.go +++ b/utils/log_test.go @@ -10,6 +10,8 @@ import ( "github.com/stretchr/testify/require" ) +const levelLabel = "level " + var levelStrings = map[utils.LogLevel]string{ utils.DEBUG: "debug", utils.INFO: "info", @@ -19,7 +21,7 @@ var levelStrings = map[utils.LogLevel]string{ func TestLogLevelString(t *testing.T) { for level, str := range levelStrings { - t.Run("level "+str, func(t *testing.T) { + t.Run(levelLabel+str, func(t *testing.T) { assert.Equal(t, str, level.String()) }) } @@ -29,15 +31,17 @@ func TestLogLevelString(t *testing.T) { // both implement the pflag.Value and encoding.TextUnmarshaller interfaces. // We can open a PR on github.com/thediveo/enumflag to add TextUnmarshaller // support. +// +//nolint:dupl func TestLogLevelSet(t *testing.T) { for level, str := range levelStrings { - t.Run("level "+str, func(t *testing.T) { + t.Run(levelLabel+str, func(t *testing.T) { l := new(utils.LogLevel) require.NoError(t, l.Set(str)) assert.Equal(t, level, *l) }) uppercase := strings.ToUpper(str) - t.Run("level "+uppercase, func(t *testing.T) { + t.Run(levelLabel+uppercase, func(t *testing.T) { l := new(utils.LogLevel) require.NoError(t, l.Set(uppercase)) assert.Equal(t, level, *l) diff --git a/utils/network_test.go b/utils/network_test.go index 348b8c9a81..b9ea80f78e 100644 --- a/utils/network_test.go +++ b/utils/network_test.go @@ -88,6 +88,7 @@ func TestNetwork(t *testing.T) { }) } +//nolint:dupl // see comment in utils/log_test.go func TestNetworkSet(t *testing.T) { for network, str := range networkStrings { t.Run(networkLabel+str, func(t *testing.T) { From 5a902508cfba10785d848518dadd8139c0dd37cd Mon Sep 17 00:00:00 2001 From: Kanan Bayramov Date: Tue, 16 Apr 2024 00:20:13 +0900 Subject: [PATCH 14/25] levelLabel2 --- utils/log_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/utils/log_test.go b/utils/log_test.go index ee52c0fc0c..5a8f18cd12 100644 --- a/utils/log_test.go +++ b/utils/log_test.go @@ -56,13 +56,13 @@ func TestLogLevelSet(t *testing.T) { func TestLogLevelUnmarshalText(t *testing.T) { for level, str := range levelStrings { - t.Run("level "+str, func(t *testing.T) { + t.Run(levelLabel+str, func(t *testing.T) { l := new(utils.LogLevel) require.NoError(t, l.UnmarshalText([]byte(str))) assert.Equal(t, level, *l) }) uppercase := strings.ToUpper(str) - t.Run("level "+uppercase, func(t *testing.T) { + t.Run(levelLabel+uppercase, func(t *testing.T) { l := new(utils.LogLevel) require.NoError(t, l.UnmarshalText([]byte(uppercase))) assert.Equal(t, level, *l) @@ -93,7 +93,7 @@ func TestLogLevelType(t *testing.T) { func TestZapWithColour(t *testing.T) { for level, str := range levelStrings { - t.Run("level: "+str, func(t *testing.T) { + t.Run(levelLabel+": "+str, func(t *testing.T) { _, err := utils.NewZapLogger(level, true) assert.NoError(t, err) }) @@ -102,7 +102,7 @@ func TestZapWithColour(t *testing.T) { func TestZapWithoutColour(t *testing.T) { for level, str := range levelStrings { - t.Run("level: "+str, func(t *testing.T) { + t.Run(levelLabel+": "+str, func(t *testing.T) { _, err := utils.NewZapLogger(level, false) assert.NoError(t, err) }) From e9dea42d1a7d9e57119dc076e8a388c1516cd12a Mon Sep 17 00:00:00 2001 From: rian Date: Fri, 19 Apr 2024 15:40:22 +0300 Subject: [PATCH 15/25] revert nolint changes --- utils/log_test.go | 16 +++++++--------- utils/network_test.go | 12 +++++------- 2 files changed, 12 insertions(+), 16 deletions(-) diff --git a/utils/log_test.go b/utils/log_test.go index 5a8f18cd12..a3f782a1db 100644 --- a/utils/log_test.go +++ b/utils/log_test.go @@ -10,8 +10,6 @@ import ( "github.com/stretchr/testify/require" ) -const levelLabel = "level " - var levelStrings = map[utils.LogLevel]string{ utils.DEBUG: "debug", utils.INFO: "info", @@ -21,7 +19,7 @@ var levelStrings = map[utils.LogLevel]string{ func TestLogLevelString(t *testing.T) { for level, str := range levelStrings { - t.Run(levelLabel+str, func(t *testing.T) { + t.Run("level "+str, func(t *testing.T) { //nolint:goconst assert.Equal(t, str, level.String()) }) } @@ -35,13 +33,13 @@ func TestLogLevelString(t *testing.T) { //nolint:dupl func TestLogLevelSet(t *testing.T) { for level, str := range levelStrings { - t.Run(levelLabel+str, func(t *testing.T) { + t.Run("level "+str, func(t *testing.T) { l := new(utils.LogLevel) require.NoError(t, l.Set(str)) assert.Equal(t, level, *l) }) uppercase := strings.ToUpper(str) - t.Run(levelLabel+uppercase, func(t *testing.T) { + t.Run("level "+uppercase, func(t *testing.T) { l := new(utils.LogLevel) require.NoError(t, l.Set(uppercase)) assert.Equal(t, level, *l) @@ -56,13 +54,13 @@ func TestLogLevelSet(t *testing.T) { func TestLogLevelUnmarshalText(t *testing.T) { for level, str := range levelStrings { - t.Run(levelLabel+str, func(t *testing.T) { + t.Run("level "+str, func(t *testing.T) { l := new(utils.LogLevel) require.NoError(t, l.UnmarshalText([]byte(str))) assert.Equal(t, level, *l) }) uppercase := strings.ToUpper(str) - t.Run(levelLabel+uppercase, func(t *testing.T) { + t.Run("level "+uppercase, func(t *testing.T) { l := new(utils.LogLevel) require.NoError(t, l.UnmarshalText([]byte(uppercase))) assert.Equal(t, level, *l) @@ -93,7 +91,7 @@ func TestLogLevelType(t *testing.T) { func TestZapWithColour(t *testing.T) { for level, str := range levelStrings { - t.Run(levelLabel+": "+str, func(t *testing.T) { + t.Run("level: "+str, func(t *testing.T) { _, err := utils.NewZapLogger(level, true) assert.NoError(t, err) }) @@ -102,7 +100,7 @@ func TestZapWithColour(t *testing.T) { func TestZapWithoutColour(t *testing.T) { for level, str := range levelStrings { - t.Run(levelLabel+": "+str, func(t *testing.T) { + t.Run("level: "+str, func(t *testing.T) { _, err := utils.NewZapLogger(level, false) assert.NoError(t, err) }) diff --git a/utils/network_test.go b/utils/network_test.go index b9ea80f78e..9df032881f 100644 --- a/utils/network_test.go +++ b/utils/network_test.go @@ -14,8 +14,6 @@ import ( "github.com/stretchr/testify/require" ) -const networkLabel = "network " - var networkStrings = map[utils.Network]string{ utils.Mainnet: "mainnet", utils.Goerli: "goerli", @@ -91,19 +89,19 @@ func TestNetwork(t *testing.T) { //nolint:dupl // see comment in utils/log_test.go func TestNetworkSet(t *testing.T) { for network, str := range networkStrings { - t.Run(networkLabel+str, func(t *testing.T) { + t.Run("network "+str, func(t *testing.T) { //nolint:goconst n := new(utils.Network) require.NoError(t, n.Set(str)) assert.Equal(t, network, *n) }) uppercase := strings.ToUpper(str) - t.Run(networkLabel+uppercase, func(t *testing.T) { + t.Run("network "+uppercase, func(t *testing.T) { n := new(utils.Network) require.NoError(t, n.Set(uppercase)) assert.Equal(t, network, *n) }) } - t.Run("unknown "+networkLabel, func(t *testing.T) { + t.Run("unknown network", func(t *testing.T) { n := utils.Network{} require.Error(t, n.Set("blah")) }) @@ -111,13 +109,13 @@ func TestNetworkSet(t *testing.T) { func TestNetworkUnmarshalText(t *testing.T) { for network, str := range networkStrings { - t.Run(networkLabel+str, func(t *testing.T) { + t.Run("network "+str, func(t *testing.T) { n := new(utils.Network) require.NoError(t, n.UnmarshalText([]byte(str))) assert.Equal(t, network, *n) }) uppercase := strings.ToUpper(str) - t.Run(networkLabel+uppercase, func(t *testing.T) { + t.Run("network "+uppercase, func(t *testing.T) { n := new(utils.Network) require.NoError(t, n.UnmarshalText([]byte(uppercase))) assert.Equal(t, network, *n) From 613430984947d2a49e1e361a1ba11b844ebb1474 Mon Sep 17 00:00:00 2001 From: Kanan Bayramov Date: Sun, 21 Apr 2024 21:23:59 +0900 Subject: [PATCH 16/25] Update on Marshal --- jsonrpc/server.go | 50 +++++++++++++++++++++++++++++++++++-------- utils/log.go | 35 ++++++++++++++++++++++-------- utils/log_test.go | 3 ++- utils/network_test.go | 2 +- 4 files changed, 70 insertions(+), 20 deletions(-) diff --git a/jsonrpc/server.go b/jsonrpc/server.go index d18474aa24..4cf87de96d 100644 --- a/jsonrpc/server.go +++ b/jsonrpc/server.go @@ -16,6 +16,7 @@ import ( "github.com/NethermindEth/juno/utils" "github.com/sourcegraph/conc/pool" + "go.uber.org/zap/zapcore" ) const ( @@ -396,15 +397,35 @@ func isNil(i any) bool { return i == nil || reflect.ValueOf(i).IsNil() } +//nolint:govet func (s *Server) handleRequest(ctx context.Context, req *Request) (*response, error) { - reqJSON, _ := json.Marshal(req) // Serialise the request early for use in logging - s.log.Tracew("Received request", "req", string(reqJSON)) + // Declare necessary variables at the beginning + var reqJSON, resJSON, errJSON []byte + var err error + + // Check if s.log is initialised and call IsLogLevel + if s.log == nil { + return nil, fmt.Errorf("logger not initialised") + } + + // Check log level for conditional logging + if utils.IsLogLevel(s.log, zapcore.DebugLevel) { + reqJSON, err = json.Marshal(req) // Only serialise if the level is Debug or above + if err != nil { + s.log.Errorw("Error marshalling request", "error", err) // Log error + return nil, fmt.Errorf("failed to marshal request: %w", err) // Handle error properly + } + s.log.Debugw("Received request", "req", string(reqJSON)) // Log at Debug level + } + + s.log.Tracew("Received request", "req", req) // Continue with trace-level logging if err := req.isSane(); err != nil { s.log.Tracew("Request sanity check failed", "error", err.Error()) return nil, err } + // Initialise response object res := &response{ Version: "2.0", ID: req.ID, @@ -427,12 +448,14 @@ func (s *Server) handleRequest(ctx context.Context, req *Request) (*response, er s.log.Tracew("Error building arguments for RPC call", "error", err.Error()) return res, nil } + defer func() { s.listener.OnRequestHandled(req.Method, time.Since(handlerTimer)) }() tuple := reflect.ValueOf(calledMethod.Handler).Call(args) - if res.ID == nil { // notification + + if res.ID == nil { // Notification, no response expected s.log.Tracew("Notification received, no response expected") return nil, nil } @@ -440,17 +463,26 @@ func (s *Server) handleRequest(ctx context.Context, req *Request) (*response, er if errAny := tuple[1].Interface(); !isNil(errAny) { res.Error = errAny.(*Error) if res.Error.Code == InternalError { + errJSON, err = json.Marshal(res.Error) // Serialise error + if err != nil { + s.log.Errorw("Error marshalling error response", "error", err) // Handle marshalling error + } s.listener.OnRequestFailed(req.Method, res.Error) - reqJSON, _ := json.Marshal(req) - errJSON, _ := json.Marshal(res.Error) - s.log.Debugw("Failed handing RPC request", "req", string(reqJSON), "res", string(errJSON)) - s.log.Tracew("Internal error during RPC handling", "req", string(reqJSON), "error", string(errJSON)) + s.log.Debugw("Failed handling RPC request", "req", string(reqJSON), "res", string(errJSON)) } return res, nil } + res.Result = tuple[0].Interface() - resJSON, _ := json.Marshal(res) // Serialise response for logging - s.log.Tracew("Successfully handled RPC request", "res", string(resJSON)) + + // Serialise the response for logging + resJSON, err = json.Marshal(res) + if err != nil { + s.log.Errorw("Error marshalling response", "error", err) // Log marshalling error + return nil, fmt.Errorf("failed to marshal response: %w", err) // Return error + } + + s.log.Tracew("Successfully handled RPC request", "res", string(resJSON)) // Log successful handling return res, nil } diff --git a/utils/log.go b/utils/log.go index becb96c05c..98bede786f 100644 --- a/utils/log.go +++ b/utils/log.go @@ -2,8 +2,10 @@ package utils import ( "encoding" + "errors" + "fmt" "time" - "errors" + "github.com/cockroachdb/pebble" "github.com/spf13/pflag" "go.uber.org/zap" @@ -11,8 +13,6 @@ import ( ) var ErrUnknownLogLevel = errors.New("unknown log level (known: debug, info, warn, error, trace)") - - type LogLevel int // The following are necessary for Cobra and Viper, respectively, to unmarshal log level @@ -30,6 +30,7 @@ const ( TRACE ) +//nolint:goconst func (l LogLevel) String() string { switch l { case DEBUG: @@ -120,13 +121,14 @@ func NewZapLogger(logLevel LogLevel, colour bool) (*ZapLogger, error) { config.EncoderConfig.EncodeTime = func(t time.Time, enc zapcore.PrimitiveArrayEncoder) { enc.AppendString(t.Local().Format("15:04:05.000 02/01/2006 -07:00")) } - level, err := zapcore.ParseLevel(logLevel.String()) - if err != nil { - return nil, err - } - if logLevel == TRACE { - config.Level.SetLevel(zapcore.DebugLevel) // Adjust according to how you define TRACE + levelStr := logLevel.String() + if levelStr == "trace" { + config.Level.SetLevel(zapcore.DebugLevel) // Custom handling for TRACE, maps to Debug level } else { + level, err := zapcore.ParseLevel(levelStr) + if err != nil { + return nil, fmt.Errorf("failed to parse log level '%s': %w", levelStr, err) // More descriptive error + } config.Level.SetLevel(level) } @@ -141,3 +143,18 @@ func NewZapLogger(logLevel LogLevel, colour bool) (*ZapLogger, error) { func (l *ZapLogger) Warningf(msg string, args ...any) { l.Warnf(msg, args) } + +// IsLogLevel checks if a logger has a specific log level enabled +func IsLogLevel(logger interface{}, targetLevel zapcore.Level) bool { + switch l := logger.(type) { + case *zap.SugaredLogger: + // Check the underlying logger's core + return l.Desugar().Core().Enabled(targetLevel) // Handle SugaredLogger + case *zap.Logger: + // Check if the core has the specified level + return l.Core().Enabled(targetLevel) // Handle basic Zap Logger + default: + // If logger type is unknown, return false + return false + } +} diff --git a/utils/log_test.go b/utils/log_test.go index a3f782a1db..1953972ea8 100644 --- a/utils/log_test.go +++ b/utils/log_test.go @@ -15,11 +15,12 @@ var levelStrings = map[utils.LogLevel]string{ utils.INFO: "info", utils.WARN: "warn", utils.ERROR: "error", + utils.TRACE: "trace", } func TestLogLevelString(t *testing.T) { for level, str := range levelStrings { - t.Run("level "+str, func(t *testing.T) { //nolint:goconst + t.Run("level "+str, func(t *testing.T) { assert.Equal(t, str, level.String()) }) } diff --git a/utils/network_test.go b/utils/network_test.go index 9df032881f..6388e91861 100644 --- a/utils/network_test.go +++ b/utils/network_test.go @@ -89,7 +89,7 @@ func TestNetwork(t *testing.T) { //nolint:dupl // see comment in utils/log_test.go func TestNetworkSet(t *testing.T) { for network, str := range networkStrings { - t.Run("network "+str, func(t *testing.T) { //nolint:goconst + t.Run("network "+str, func(t *testing.T) { n := new(utils.Network) require.NoError(t, n.Set(str)) assert.Equal(t, network, *n) From 589778613f31f28aa843eeded663d45bbbf69b55 Mon Sep 17 00:00:00 2001 From: Kanan Bayramov Date: Tue, 23 Apr 2024 20:10:37 +0900 Subject: [PATCH 17/25] change on conflict --- utils/log.go | 1 + 1 file changed, 1 insertion(+) diff --git a/utils/log.go b/utils/log.go index 98bede786f..df5fca7725 100644 --- a/utils/log.go +++ b/utils/log.go @@ -13,6 +13,7 @@ import ( ) var ErrUnknownLogLevel = errors.New("unknown log level (known: debug, info, warn, error, trace)") + type LogLevel int // The following are necessary for Cobra and Viper, respectively, to unmarshal log level From c42b7c04e1fc6f9f98262946774448df6497711b Mon Sep 17 00:00:00 2001 From: Kanan Bayramov Date: Tue, 23 Apr 2024 23:02:52 +0900 Subject: [PATCH 18/25] Update gci --- utils/log_test.go | 2 +- utils/network_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/utils/log_test.go b/utils/log_test.go index 1953972ea8..8a5954b872 100644 --- a/utils/log_test.go +++ b/utils/log_test.go @@ -20,7 +20,7 @@ var levelStrings = map[utils.LogLevel]string{ func TestLogLevelString(t *testing.T) { for level, str := range levelStrings { - t.Run("level "+str, func(t *testing.T) { + t.Run("level "+str, func(t *testing.T) { assert.Equal(t, str, level.String()) }) } diff --git a/utils/network_test.go b/utils/network_test.go index 6388e91861..dc522f0927 100644 --- a/utils/network_test.go +++ b/utils/network_test.go @@ -89,7 +89,7 @@ func TestNetwork(t *testing.T) { //nolint:dupl // see comment in utils/log_test.go func TestNetworkSet(t *testing.T) { for network, str := range networkStrings { - t.Run("network "+str, func(t *testing.T) { + t.Run("network "+str, func(t *testing.T) { n := new(utils.Network) require.NoError(t, n.Set(str)) assert.Equal(t, network, *n) From fc0d1528bd4c88c0cde711766b43b1fcf321a583 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Nowosielski?= Date: Wed, 24 Apr 2024 14:48:59 +0200 Subject: [PATCH 19/25] fix linter constant suggestions --- utils/log_test.go | 1 + utils/network_test.go | 1 + 2 files changed, 2 insertions(+) diff --git a/utils/log_test.go b/utils/log_test.go index 8a5954b872..a451d8d5d4 100644 --- a/utils/log_test.go +++ b/utils/log_test.go @@ -20,6 +20,7 @@ var levelStrings = map[utils.LogLevel]string{ func TestLogLevelString(t *testing.T) { for level, str := range levelStrings { + //nolint:goconst t.Run("level "+str, func(t *testing.T) { assert.Equal(t, str, level.String()) }) diff --git a/utils/network_test.go b/utils/network_test.go index dc522f0927..9b78a26168 100644 --- a/utils/network_test.go +++ b/utils/network_test.go @@ -89,6 +89,7 @@ func TestNetwork(t *testing.T) { //nolint:dupl // see comment in utils/log_test.go func TestNetworkSet(t *testing.T) { for network, str := range networkStrings { + //nolint:goconst t.Run("network "+str, func(t *testing.T) { n := new(utils.Network) require.NoError(t, n.Set(str)) From e9af0dfacc53e23df227020f5586cf5095ca7156 Mon Sep 17 00:00:00 2001 From: rian Date: Thu, 9 May 2024 11:59:18 +0300 Subject: [PATCH 20/25] simplify changes --- .golangci.yaml | 2 +- db/pebble/transaction.go | 2 +- jsonrpc/server.go | 50 +++++----------------------------------- p2p/sync.go | 5 +--- utils/log.go | 40 ++++++++++---------------------- utils/log_test.go | 1 - utils/network_test.go | 3 +-- 7 files changed, 22 insertions(+), 81 deletions(-) diff --git a/.golangci.yaml b/.golangci.yaml index 96b40d87cd..068f99cd04 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -152,4 +152,4 @@ run: - test/testdata_etc # test files - internal/cache # extracted from Go code - internal/renameio # extracted from Go code - - internal/robustio # extracted from Go code \ No newline at end of file + - internal/robustio # extracted from Go code diff --git a/db/pebble/transaction.go b/db/pebble/transaction.go index 2bef0c5bb2..c07b499144 100644 --- a/db/pebble/transaction.go +++ b/db/pebble/transaction.go @@ -94,7 +94,7 @@ func (t *Transaction) Get(key []byte, cb func([]byte) error) error { return ErrDiscardedTransaction } - t.listener.OnIO(false, time.Since(start)) + defer t.listener.OnIO(false, time.Since(start)) //nolint:govet if err != nil { if errors.Is(err, pebble.ErrNotFound) { return db.ErrKeyNotFound diff --git a/jsonrpc/server.go b/jsonrpc/server.go index 4cf87de96d..fceb69dc93 100644 --- a/jsonrpc/server.go +++ b/jsonrpc/server.go @@ -16,7 +16,6 @@ import ( "github.com/NethermindEth/juno/utils" "github.com/sourcegraph/conc/pool" - "go.uber.org/zap/zapcore" ) const ( @@ -397,35 +396,13 @@ func isNil(i any) bool { return i == nil || reflect.ValueOf(i).IsNil() } -//nolint:govet func (s *Server) handleRequest(ctx context.Context, req *Request) (*response, error) { - // Declare necessary variables at the beginning - var reqJSON, resJSON, errJSON []byte - var err error - - // Check if s.log is initialised and call IsLogLevel - if s.log == nil { - return nil, fmt.Errorf("logger not initialised") - } - - // Check log level for conditional logging - if utils.IsLogLevel(s.log, zapcore.DebugLevel) { - reqJSON, err = json.Marshal(req) // Only serialise if the level is Debug or above - if err != nil { - s.log.Errorw("Error marshalling request", "error", err) // Log error - return nil, fmt.Errorf("failed to marshal request: %w", err) // Handle error properly - } - s.log.Debugw("Received request", "req", string(reqJSON)) // Log at Debug level - } - - s.log.Tracew("Received request", "req", req) // Continue with trace-level logging - + s.log.Tracew("Received request", "req", req) if err := req.isSane(); err != nil { s.log.Tracew("Request sanity check failed", "error", err.Error()) return nil, err } - // Initialise response object res := &response{ Version: "2.0", ID: req.ID, @@ -440,22 +417,18 @@ func (s *Server) handleRequest(ctx context.Context, req *Request) (*response, er handlerTimer := time.Now() s.listener.OnNewRequest(req.Method) - s.log.Tracew("Handling new RPC request", "method", req.Method) - args, err := s.buildArguments(ctx, req.Params, calledMethod) if err != nil { res.Error = Err(InvalidParams, err.Error()) s.log.Tracew("Error building arguments for RPC call", "error", err.Error()) return res, nil } - defer func() { s.listener.OnRequestHandled(req.Method, time.Since(handlerTimer)) }() tuple := reflect.ValueOf(calledMethod.Handler).Call(args) - - if res.ID == nil { // Notification, no response expected + if res.ID == nil { // notification s.log.Tracew("Notification received, no response expected") return nil, nil } @@ -463,26 +436,15 @@ func (s *Server) handleRequest(ctx context.Context, req *Request) (*response, er if errAny := tuple[1].Interface(); !isNil(errAny) { res.Error = errAny.(*Error) if res.Error.Code == InternalError { - errJSON, err = json.Marshal(res.Error) // Serialise error - if err != nil { - s.log.Errorw("Error marshalling error response", "error", err) // Handle marshalling error - } s.listener.OnRequestFailed(req.Method, res.Error) - s.log.Debugw("Failed handling RPC request", "req", string(reqJSON), "res", string(errJSON)) + reqJSON, _ := json.Marshal(req) + errJSON, _ := json.Marshal(res.Error) + s.log.Debugw("Failed handing RPC request", "req", string(reqJSON), "res", string(errJSON)) } return res, nil } - res.Result = tuple[0].Interface() - - // Serialise the response for logging - resJSON, err = json.Marshal(res) - if err != nil { - s.log.Errorw("Error marshalling response", "error", err) // Log marshalling error - return nil, fmt.Errorf("failed to marshal response: %w", err) // Return error - } - - s.log.Tracew("Successfully handled RPC request", "res", string(resJSON)) // Log successful handling + s.log.Tracew("Successfully handled RPC request", "req", req) return res, nil } diff --git a/p2p/sync.go b/p2p/sync.go index df94a408e4..53ceaeff55 100644 --- a/p2p/sync.go +++ b/p2p/sync.go @@ -101,8 +101,7 @@ func (s *syncService) start(ctx context.Context) { } var nextHeight int - curHeight, err := s.blockchain.Height() - if err == nil { + if curHeight, err := s.blockchain.Height(); err == nil { //nolint:govet nextHeight = int(curHeight) + 1 } else if !errors.Is(db.ErrKeyNotFound, err) { s.log.Errorw("Failed to get current height", "err", err) @@ -202,8 +201,6 @@ func (s *syncService) logError(msg string, err error) { log = s.log } - log.Tracew("Logging error", "msg", msg, "error", err.Error()) - // Log the error log.Errorw(msg, "err", err) } } diff --git a/utils/log.go b/utils/log.go index df5fca7725..88fe55f1e4 100644 --- a/utils/log.go +++ b/utils/log.go @@ -2,7 +2,6 @@ package utils import ( "encoding" - "errors" "fmt" "time" @@ -12,7 +11,10 @@ import ( "go.uber.org/zap/zapcore" ) -var ErrUnknownLogLevel = errors.New("unknown log level (known: debug, info, warn, error, trace)") +var ErrUnknownLogLevel = fmt.Errorf( + "unknown log level (known: %s, %s, %s, %s, %s)", + DEBUG, INFO, WARN, ERROR, TRACE, +) type LogLevel int @@ -31,7 +33,6 @@ const ( TRACE ) -//nolint:goconst func (l LogLevel) String() string { switch l { case DEBUG: @@ -105,7 +106,7 @@ func (l *ZapLogger) Tracew(msg string, keysAndValues ...interface{}) { l.Debugw("[TRACE] "+msg, keysAndValues...) } -var _ SimpleLogger = (*ZapLogger)(nil) +var _ Logger = (*ZapLogger)(nil) func NewNopZapLogger() *ZapLogger { return &ZapLogger{zap.NewNop().Sugar()} @@ -123,16 +124,14 @@ func NewZapLogger(logLevel LogLevel, colour bool) (*ZapLogger, error) { enc.AppendString(t.Local().Format("15:04:05.000 02/01/2006 -07:00")) } levelStr := logLevel.String() - if levelStr == "trace" { - config.Level.SetLevel(zapcore.DebugLevel) // Custom handling for TRACE, maps to Debug level - } else { - level, err := zapcore.ParseLevel(levelStr) - if err != nil { - return nil, fmt.Errorf("failed to parse log level '%s': %w", levelStr, err) // More descriptive error - } - config.Level.SetLevel(level) + if levelStr == TRACE.String() { + levelStr = DEBUG.String() // Todo: zapcore has no trace level, and we are using debug for both.. } - + level, err := zapcore.ParseLevel(levelStr) + if err != nil { + return nil, err + } + config.Level.SetLevel(level) log, err := config.Build() if err != nil { return nil, err @@ -144,18 +143,3 @@ func NewZapLogger(logLevel LogLevel, colour bool) (*ZapLogger, error) { func (l *ZapLogger) Warningf(msg string, args ...any) { l.Warnf(msg, args) } - -// IsLogLevel checks if a logger has a specific log level enabled -func IsLogLevel(logger interface{}, targetLevel zapcore.Level) bool { - switch l := logger.(type) { - case *zap.SugaredLogger: - // Check the underlying logger's core - return l.Desugar().Core().Enabled(targetLevel) // Handle SugaredLogger - case *zap.Logger: - // Check if the core has the specified level - return l.Core().Enabled(targetLevel) // Handle basic Zap Logger - default: - // If logger type is unknown, return false - return false - } -} diff --git a/utils/log_test.go b/utils/log_test.go index a451d8d5d4..8a5954b872 100644 --- a/utils/log_test.go +++ b/utils/log_test.go @@ -20,7 +20,6 @@ var levelStrings = map[utils.LogLevel]string{ func TestLogLevelString(t *testing.T) { for level, str := range levelStrings { - //nolint:goconst t.Run("level "+str, func(t *testing.T) { assert.Equal(t, str, level.String()) }) diff --git a/utils/network_test.go b/utils/network_test.go index 961be1340d..6922969858 100644 --- a/utils/network_test.go +++ b/utils/network_test.go @@ -86,8 +86,7 @@ func TestNetwork(t *testing.T) { //nolint:dupl // see comment in utils/log_test.go func TestNetworkSet(t *testing.T) { for network, str := range networkStrings { - //nolint:goconst - t.Run("network "+str, func(t *testing.T) { + t.Run("network "+str, func(t *testing.T) { //nolint:goconst n := new(utils.Network) require.NoError(t, n.Set(str)) assert.Equal(t, network, *n) From 77c84734101fb580420b6cf7cf684cf194252ad3 Mon Sep 17 00:00:00 2001 From: rian Date: Thu, 9 May 2024 12:01:21 +0300 Subject: [PATCH 21/25] lint error --- utils/log_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utils/log_test.go b/utils/log_test.go index 8a5954b872..5f77b2735d 100644 --- a/utils/log_test.go +++ b/utils/log_test.go @@ -20,7 +20,7 @@ var levelStrings = map[utils.LogLevel]string{ func TestLogLevelString(t *testing.T) { for level, str := range levelStrings { - t.Run("level "+str, func(t *testing.T) { + t.Run("level "+str, func(t *testing.T) { //nolint:goconst assert.Equal(t, str, level.String()) }) } From f9be592508e9d242708ccf35d3989428124f32d3 Mon Sep 17 00:00:00 2001 From: rian Date: Thu, 9 May 2024 12:33:44 +0300 Subject: [PATCH 22/25] create and set trace level that uses debug --- utils/log.go | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/utils/log.go b/utils/log.go index 88fe55f1e4..de0f05ea9a 100644 --- a/utils/log.go +++ b/utils/log.go @@ -102,8 +102,18 @@ type ZapLogger struct { *zap.SugaredLogger } +const ( + TraceLevel = zapcore.Level(-2) +) + +func (l *ZapLogger) IsTraceEnabled() bool { + return l.Desugar().Core().Enabled(TraceLevel) +} + func (l *ZapLogger) Tracew(msg string, keysAndValues ...interface{}) { - l.Debugw("[TRACE] "+msg, keysAndValues...) + if l.IsTraceEnabled() { + l.Debugw("[TRACE] "+msg, keysAndValues...) // Hack: we use Debug for traces + } } var _ Logger = (*ZapLogger)(nil) @@ -123,13 +133,17 @@ func NewZapLogger(logLevel LogLevel, colour bool) (*ZapLogger, error) { config.EncoderConfig.EncodeTime = func(t time.Time, enc zapcore.PrimitiveArrayEncoder) { enc.AppendString(t.Local().Format("15:04:05.000 02/01/2006 -07:00")) } + + var level zapcore.Level + var err error levelStr := logLevel.String() if levelStr == TRACE.String() { - levelStr = DEBUG.String() // Todo: zapcore has no trace level, and we are using debug for both.. - } - level, err := zapcore.ParseLevel(levelStr) - if err != nil { - return nil, err + level = TraceLevel + } else { + level, err = zapcore.ParseLevel(levelStr) + if err != nil { + return nil, err + } } config.Level.SetLevel(level) log, err := config.Build() From 3d4654d6a0885886700a29015f4f7e65a3e1f381 Mon Sep 17 00:00:00 2001 From: rian Date: Thu, 16 May 2024 10:57:30 +0300 Subject: [PATCH 23/25] addresss comments --- jsonrpc/server.go | 2 +- utils/log.go | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/jsonrpc/server.go b/jsonrpc/server.go index fceb69dc93..f397c9822c 100644 --- a/jsonrpc/server.go +++ b/jsonrpc/server.go @@ -399,7 +399,7 @@ func isNil(i any) bool { func (s *Server) handleRequest(ctx context.Context, req *Request) (*response, error) { s.log.Tracew("Received request", "req", req) if err := req.isSane(); err != nil { - s.log.Tracew("Request sanity check failed", "error", err.Error()) + s.log.Tracew("Request sanity check failed", "err", err) return nil, err } diff --git a/utils/log.go b/utils/log.go index de0f05ea9a..7a6d93e614 100644 --- a/utils/log.go +++ b/utils/log.go @@ -103,11 +103,11 @@ type ZapLogger struct { } const ( - TraceLevel = zapcore.Level(-2) + traceLevel = zapcore.Level(-2) ) func (l *ZapLogger) IsTraceEnabled() bool { - return l.Desugar().Core().Enabled(TraceLevel) + return l.Desugar().Core().Enabled(traceLevel) } func (l *ZapLogger) Tracew(msg string, keysAndValues ...interface{}) { @@ -137,8 +137,8 @@ func NewZapLogger(logLevel LogLevel, colour bool) (*ZapLogger, error) { var level zapcore.Level var err error levelStr := logLevel.String() - if levelStr == TRACE.String() { - level = TraceLevel + if logLevel == TRACE { + level = traceLevel } else { level, err = zapcore.ParseLevel(levelStr) if err != nil { From fb0d9fd78daadc46e395f8e961dd303101c59445 Mon Sep 17 00:00:00 2001 From: rian Date: Thu, 16 May 2024 14:39:03 +0300 Subject: [PATCH 24/25] err.Error(0 --- jsonrpc/server.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jsonrpc/server.go b/jsonrpc/server.go index f397c9822c..b0bc7dc2b9 100644 --- a/jsonrpc/server.go +++ b/jsonrpc/server.go @@ -420,7 +420,7 @@ func (s *Server) handleRequest(ctx context.Context, req *Request) (*response, er args, err := s.buildArguments(ctx, req.Params, calledMethod) if err != nil { res.Error = Err(InvalidParams, err.Error()) - s.log.Tracew("Error building arguments for RPC call", "error", err.Error()) + s.log.Tracew("Error building arguments for RPC call", "err", err) return res, nil } defer func() { From e6010b2fa83ddec69f6f78aa55ea4030f9feb265 Mon Sep 17 00:00:00 2001 From: rian Date: Thu, 16 May 2024 14:55:55 +0300 Subject: [PATCH 25/25] remove succesful log --- jsonrpc/server.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jsonrpc/server.go b/jsonrpc/server.go index b0bc7dc2b9..b75aeae6d0 100644 --- a/jsonrpc/server.go +++ b/jsonrpc/server.go @@ -444,7 +444,7 @@ func (s *Server) handleRequest(ctx context.Context, req *Request) (*response, er return res, nil } res.Result = tuple[0].Interface() - s.log.Tracew("Successfully handled RPC request", "req", req) + return res, nil }