Refine logging to avoid printing expensive objects on hot path#3066
Refine logging to avoid printing expensive objects on hot path#3066
Conversation
Fix logging of objects to: 1) avoid printing expensive encodings, and 2) use hex encoded string for bytes.
|
The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3066 +/- ##
=======================================
Coverage 58.58% 58.59%
=======================================
Files 2080 2080
Lines 171656 171659 +3
=======================================
+ Hits 100567 100580 +13
+ Misses 62166 62159 -7
+ Partials 8923 8920 -3
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
| logger.Info("sent a request", "method", request.Method, "id", request.ID()) | ||
| // c.mtx.Lock() | ||
| // c.sentIDs[request.ID.(types.JSONRPCIntID)] = true | ||
| // c.mtx.Unlock() |
There was a problem hiding this comment.
[nit]: unrelated to the PR, but im curious why we keep commented code in the codebase here.
There was a problem hiding this comment.
Great question. This one goes back a few years. We should separately review cases like this
| _, _, err := parseTx(tx) | ||
| if err != nil { | ||
| logger.Error("malformed transaction in ProcessProposal", "tx", tx, "err", err) | ||
| logger.Error("malformed transaction in ProcessProposal", "len-bytes", len(tx), "err", err) |
There was a problem hiding this comment.
[nit]: wouldn't this be useful to have the full tx in a debug log as well?
There was a problem hiding this comment.
We could but for malformed proposals off the wire that could be large at Error level would come with security concerns.
An alternative approach is to log the proposal at debug level disabled by default. I will add that 🙌
* main: fix(giga): match v2 correctness checks (#3071) Added clone method to canned random (#3076) Helper files for the flatKV cache implementation (#3072) fix: restore PRs inadvertently reverted by #3039 squash-merge (#3070) Refine logging to avoid printing expensive objects on hot path (#3066) Fix flaky tendermint syncer test (#3065) Add runtime log level control via gRPC admin service (#3062) chore: dcoument run RPC suite on legacy vs giga (#3041) chore: self-contained revert tests, contract reorg, and failure analysis (#3033)
3 participants
Fix logging of objects to: 1) avoid printing expensive encodings, and 2) use hex encoded string for bytes.