From c792f622fbcf03f5d5d88a326e45570bd2d60f1e Mon Sep 17 00:00:00 2001 From: Emmanuel T Odeke Date: Wed, 2 Mar 2022 11:35:05 +0300 Subject: [PATCH 1/4] eth/tracers/logger: use JSON struct tag ",omitempty" to reduce log bloat from blank fields Noticed while debugging Tharsis/EVMOS logs that the logs were extraneous with empty fields such as ```json {"pc":4716,"op":80,"gas":"0x688e5","gasCost":"0x2","memory":"0x","memSize":352,"stack":["0xfa31de01","0x29a","0x7d0","0xabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcd","0x80","0x534","0xc0","0x5f646b5592c4159d4963fbf901f81144ef53cc51a7082435e3a6120a45002ff7","0x15dc","0x34","0x5f646b5592c4159d4963fbf901f81144ef53cc51a7082435e3a6120a45002ff7"],"returnData":"0x","depth":2,"refund":0,"opName":"POP","error":""} {"pc":4717,"op":80,"gas":"0x688e3","gasCost":"0x2","memory":"0x","memSize":352,"stack":["0xfa31de01","0x29a","0x7d0","0xabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcd","0x80","0x534","0xc0","0x5f646b5592c4159d4963fbf901f81144ef53cc51a7082435e3a6120a45002ff7","0x15dc","0x34"],"returnData":"0x","depth":2,"refund":0,"opName":"POP","error":""} ``` with empty fields like "error", "refund", "memory", "returnData". This change adds the JSON tag ",omitempty" to remove the bloat from the above empty fields which shows immediate impact of reducing the logs to ```json {"pc":4716,"op":80,"gas":"0x688e5","gasCost":"0x2","memSize":352,"stack":["0xfa31de01","0x29a","0x7d0","0xabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcd","0x80","0x534","0xc0","0x5f646b5592c4159d4963fbf901f81144ef53cc51a7082435e3a6120a45002ff7","0x15dc","0x34","0x5f646b5592c4159d4963fbf901f81144ef53cc51a7082435e3a6120a45002ff7"],"depth":2,"opName":"POP"} {"pc":4717,"op":80,"gas":"0x688e3","gasCost":"0x2","memSize":352,"stack":["0xfa31de01","0x29a","0x7d0","0xabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcd","0x80","0x534","0xc0","0x5f646b5592c4159d4963fbf901f81144ef53cc51a7082435e3a6120a45002ff7","0x15dc","0x34"],"depth":2,"opName":"POP"} ``` and not only does that reduce the size of logs but it also makes it much easier to quickly debug and grep for errors, which will only be present if erroring. Fixes #24487 --- eth/tracers/logger/gen_structlog.go | 4 ++-- eth/tracers/logger/logger.go | 12 ++++++------ eth/tracers/logger/logger_test.go | 27 +++++++++++++++++++++++++++ 3 files changed, 35 insertions(+), 8 deletions(-) diff --git a/eth/tracers/logger/gen_structlog.go b/eth/tracers/logger/gen_structlog.go index 9e71b555cb63..7ce8cc50d635 100644 --- a/eth/tracers/logger/gen_structlog.go +++ b/eth/tracers/logger/gen_structlog.go @@ -29,8 +29,8 @@ func (s StructLog) MarshalJSON() ([]byte, error) { Depth int `json:"depth"` RefundCounter uint64 `json:"refund"` Err error `json:"-"` - OpName string `json:"opName"` - ErrorString string `json:"error"` + OpName string `json:"opName,omitempty"` + ErrorString string `json:"error,omitempty"` } var enc StructLog enc.Pc = s.Pc diff --git a/eth/tracers/logger/logger.go b/eth/tracers/logger/logger.go index 8461935822d8..bbee21e64c57 100644 --- a/eth/tracers/logger/logger.go +++ b/eth/tracers/logger/logger.go @@ -78,12 +78,12 @@ type StructLog struct { // overrides for gencodec type structLogMarshaling struct { - Gas math.HexOrDecimal64 - GasCost math.HexOrDecimal64 - Memory hexutil.Bytes - ReturnData hexutil.Bytes - OpName string `json:"opName"` // adds call to OpName() in MarshalJSON - ErrorString string `json:"error"` // adds call to ErrorString() in MarshalJSON + Gas math.HexOrDecimal64 `json:",omitempty"` + GasCost math.HexOrDecimal64 `json:",omitempty"` + Memory hexutil.Bytes `json:",omitempty"` + ReturnData hexutil.Bytes `json:",omitempty"` + OpName string `json:"opName,omitempty"` // adds call to OpName() in MarshalJSON + ErrorString string `json:"error,omitempty"` // adds call to ErrorString() in MarshalJSON } // OpName formats the operand name in a human-readable format. diff --git a/eth/tracers/logger/logger_test.go b/eth/tracers/logger/logger_test.go index 205ee311201a..e44fbff36b5f 100644 --- a/eth/tracers/logger/logger_test.go +++ b/eth/tracers/logger/logger_test.go @@ -17,6 +17,7 @@ package logger import ( + "encoding/json" "math/big" "testing" @@ -72,3 +73,29 @@ func TestStoreCapture(t *testing.T) { t.Errorf("expected %x, got %x", exp, logger.storage[contract.Address()][index]) } } + +// Tests that blank fields don't appear in logs when JSON marshalled, to reduce +// logs bloat and confusion. See https://github.com/ethereum/go-ethereum/issues/24487 +func TestStructLogMarshalingOmitEmpty(t *testing.T) { + tests := []struct { + name string + log *structLogMarshaling + want string + }{ + {"empty err and no fields", &structLogMarshaling{ErrorString: ""}, `{}`}, + {"with Gas cost only", &structLogMarshaling{GasCost: 10}, `{"GasCost":"0xa"}`}, + {"with err", &structLogMarshaling{ErrorString: "this failed"}, `{"error":"this failed"}`}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + blob, err := json.Marshal(tt.log) + if err != nil { + t.Fatal(err) + } + if g, w := string(blob), tt.want; g != w { + t.Fatalf("Mismatched results\n\tGot: %q\n\tWant: %q", g, w) + } + }) + } +} From 84e4daa01c2c9afa9da84335c84e7a57376a82c5 Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Wed, 16 Mar 2022 11:27:36 +0100 Subject: [PATCH 2/4] eth/tracers: fix review concerns --- eth/tracers/logger/gen_structlog.go | 2 +- eth/tracers/logger/logger.go | 12 ++++++------ eth/tracers/logger/logger_test.go | 9 ++++----- 3 files changed, 11 insertions(+), 12 deletions(-) diff --git a/eth/tracers/logger/gen_structlog.go b/eth/tracers/logger/gen_structlog.go index 7ce8cc50d635..7818fbe70b26 100644 --- a/eth/tracers/logger/gen_structlog.go +++ b/eth/tracers/logger/gen_structlog.go @@ -29,7 +29,7 @@ func (s StructLog) MarshalJSON() ([]byte, error) { Depth int `json:"depth"` RefundCounter uint64 `json:"refund"` Err error `json:"-"` - OpName string `json:"opName,omitempty"` + OpName string `json:"opName"` ErrorString string `json:"error,omitempty"` } var enc StructLog diff --git a/eth/tracers/logger/logger.go b/eth/tracers/logger/logger.go index bbee21e64c57..46ba0271dc56 100644 --- a/eth/tracers/logger/logger.go +++ b/eth/tracers/logger/logger.go @@ -78,12 +78,12 @@ type StructLog struct { // overrides for gencodec type structLogMarshaling struct { - Gas math.HexOrDecimal64 `json:",omitempty"` - GasCost math.HexOrDecimal64 `json:",omitempty"` - Memory hexutil.Bytes `json:",omitempty"` - ReturnData hexutil.Bytes `json:",omitempty"` - OpName string `json:"opName,omitempty"` // adds call to OpName() in MarshalJSON - ErrorString string `json:"error,omitempty"` // adds call to ErrorString() in MarshalJSON + Gas math.HexOrDecimal64 + GasCost math.HexOrDecimal64 + Memory hexutil.Bytes `json:",omitempty"` + ReturnData hexutil.Bytes `json:",omitempty"` + OpName string `json:"opName"` // adds call to OpName() in MarshalJSON + ErrorString string `json:"error,omitempty"` // adds call to ErrorString() in MarshalJSON } // OpName formats the operand name in a human-readable format. diff --git a/eth/tracers/logger/logger_test.go b/eth/tracers/logger/logger_test.go index e44fbff36b5f..3b7d852d91c9 100644 --- a/eth/tracers/logger/logger_test.go +++ b/eth/tracers/logger/logger_test.go @@ -82,9 +82,8 @@ func TestStructLogMarshalingOmitEmpty(t *testing.T) { log *structLogMarshaling want string }{ - {"empty err and no fields", &structLogMarshaling{ErrorString: ""}, `{}`}, - {"with Gas cost only", &structLogMarshaling{GasCost: 10}, `{"GasCost":"0xa"}`}, - {"with err", &structLogMarshaling{ErrorString: "this failed"}, `{"error":"this failed"}`}, + {"empty err and no fields", &structLogMarshaling{ErrorString: ""}, `{"Gas":"0x0","GasCost":"0x0","opName":""}`}, + {"with err", &structLogMarshaling{ErrorString: "this failed"}, `{"Gas":"0x0","GasCost":"0x0","opName":"","error":"this failed"}`}, } for _, tt := range tests { @@ -93,8 +92,8 @@ func TestStructLogMarshalingOmitEmpty(t *testing.T) { if err != nil { t.Fatal(err) } - if g, w := string(blob), tt.want; g != w { - t.Fatalf("Mismatched results\n\tGot: %q\n\tWant: %q", g, w) + if have, want := string(blob), tt.want; have != want { + t.Fatalf("mismatched results\n\thave: %q\n\twant: %q", have, want) } }) } From b749723d4a456753fea3e754f047fb38801c3136 Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Wed, 16 Mar 2022 14:48:32 +0100 Subject: [PATCH 3/4] eth/tracers/logger: apply changes at the right places --- eth/tracers/logger/gen_structlog.go | 8 ++++---- eth/tracers/logger/logger.go | 12 ++++++------ eth/tracers/logger/logger_test.go | 15 +++++++++++---- 3 files changed, 21 insertions(+), 14 deletions(-) diff --git a/eth/tracers/logger/gen_structlog.go b/eth/tracers/logger/gen_structlog.go index 7818fbe70b26..df06a9ee6b66 100644 --- a/eth/tracers/logger/gen_structlog.go +++ b/eth/tracers/logger/gen_structlog.go @@ -21,10 +21,10 @@ func (s StructLog) MarshalJSON() ([]byte, error) { Op vm.OpCode `json:"op"` Gas math.HexOrDecimal64 `json:"gas"` GasCost math.HexOrDecimal64 `json:"gasCost"` - Memory hexutil.Bytes `json:"memory"` + Memory hexutil.Bytes `json:"memory,omitempty"` MemorySize int `json:"memSize"` Stack []uint256.Int `json:"stack"` - ReturnData hexutil.Bytes `json:"returnData"` + ReturnData hexutil.Bytes `json:"returnData,omitempty"` Storage map[common.Hash]common.Hash `json:"-"` Depth int `json:"depth"` RefundCounter uint64 `json:"refund"` @@ -57,10 +57,10 @@ func (s *StructLog) UnmarshalJSON(input []byte) error { Op *vm.OpCode `json:"op"` Gas *math.HexOrDecimal64 `json:"gas"` GasCost *math.HexOrDecimal64 `json:"gasCost"` - Memory *hexutil.Bytes `json:"memory"` + Memory *hexutil.Bytes `json:"memory,omitempty"` MemorySize *int `json:"memSize"` Stack []uint256.Int `json:"stack"` - ReturnData *hexutil.Bytes `json:"returnData"` + ReturnData *hexutil.Bytes `json:"returnData,omitempty"` Storage map[common.Hash]common.Hash `json:"-"` Depth *int `json:"depth"` RefundCounter *uint64 `json:"refund"` diff --git a/eth/tracers/logger/logger.go b/eth/tracers/logger/logger.go index 46ba0271dc56..d0c7bff89368 100644 --- a/eth/tracers/logger/logger.go +++ b/eth/tracers/logger/logger.go @@ -66,10 +66,10 @@ type StructLog struct { Op vm.OpCode `json:"op"` Gas uint64 `json:"gas"` GasCost uint64 `json:"gasCost"` - Memory []byte `json:"memory"` + Memory []byte `json:"memory,omitempty"` MemorySize int `json:"memSize"` Stack []uint256.Int `json:"stack"` - ReturnData []byte `json:"returnData"` + ReturnData []byte `json:"returnData,omitempty"` Storage map[common.Hash]common.Hash `json:"-"` Depth int `json:"depth"` RefundCounter uint64 `json:"refund"` @@ -80,10 +80,10 @@ type StructLog struct { type structLogMarshaling struct { Gas math.HexOrDecimal64 GasCost math.HexOrDecimal64 - Memory hexutil.Bytes `json:",omitempty"` - ReturnData hexutil.Bytes `json:",omitempty"` - OpName string `json:"opName"` // adds call to OpName() in MarshalJSON - ErrorString string `json:"error,omitempty"` // adds call to ErrorString() in MarshalJSON + Memory hexutil.Bytes + ReturnData hexutil.Bytes + OpName string `json:"opName"` // adds call to OpName() in MarshalJSON + ErrorString string `json:"error,omitempty"` // adds call to ErrorString() in MarshalJSON } // OpName formats the operand name in a human-readable format. diff --git a/eth/tracers/logger/logger_test.go b/eth/tracers/logger/logger_test.go index 3b7d852d91c9..b501862e116b 100644 --- a/eth/tracers/logger/logger_test.go +++ b/eth/tracers/logger/logger_test.go @@ -21,6 +21,7 @@ import ( "math/big" "testing" + "fmt" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/core/state" "github.com/ethereum/go-ethereum/core/vm" @@ -79,11 +80,17 @@ func TestStoreCapture(t *testing.T) { func TestStructLogMarshalingOmitEmpty(t *testing.T) { tests := []struct { name string - log *structLogMarshaling + log *StructLog want string }{ - {"empty err and no fields", &structLogMarshaling{ErrorString: ""}, `{"Gas":"0x0","GasCost":"0x0","opName":""}`}, - {"with err", &structLogMarshaling{ErrorString: "this failed"}, `{"Gas":"0x0","GasCost":"0x0","opName":"","error":"this failed"}`}, + {"empty err and no fields", &StructLog{}, + `{"pc":0,"op":0,"gas":"0x0","gasCost":"0x0","memSize":0,"stack":null,"depth":0,"refund":0,"opName":"STOP"}`}, + {"with err", &StructLog{Err: fmt.Errorf("this failed")}, + `{"pc":0,"op":0,"gas":"0x0","gasCost":"0x0","memSize":0,"stack":null,"depth":0,"refund":0,"opName":"STOP","error":"this failed"}`}, + {"with mem", &StructLog{Memory: make([]byte, 2), MemorySize: 2}, + `{"pc":0,"op":0,"gas":"0x0","gasCost":"0x0","memory":"0x0000","memSize":2,"stack":null,"depth":0,"refund":0,"opName":"STOP"}`}, + {"with 0-size mem", &StructLog{Memory: make([]byte, 0)}, + `{"pc":0,"op":0,"gas":"0x0","gasCost":"0x0","memSize":0,"stack":null,"depth":0,"refund":0,"opName":"STOP"}`}, } for _, tt := range tests { @@ -93,7 +100,7 @@ func TestStructLogMarshalingOmitEmpty(t *testing.T) { t.Fatal(err) } if have, want := string(blob), tt.want; have != want { - t.Fatalf("mismatched results\n\thave: %q\n\twant: %q", have, want) + t.Fatalf("mismatched results\n\thave: %v\n\twant: %v", have, want) } }) } From 660c1c4bacdfc0b4e9762d9e7238822298d44a19 Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Thu, 17 Mar 2022 10:02:42 +0100 Subject: [PATCH 4/4] eth/tracers/logger: import fix --- eth/tracers/logger/logger_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eth/tracers/logger/logger_test.go b/eth/tracers/logger/logger_test.go index b501862e116b..6b1e74081454 100644 --- a/eth/tracers/logger/logger_test.go +++ b/eth/tracers/logger/logger_test.go @@ -18,10 +18,10 @@ package logger import ( "encoding/json" + "fmt" "math/big" "testing" - "fmt" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/core/state" "github.com/ethereum/go-ethereum/core/vm"