Skip to content

log: stderr offending large log messages#4930

Merged
algorandskiy merged 1 commit intoalgorand:masterfrom
algorandskiy:pavel/large-log-line
Dec 22, 2022
Merged

log: stderr offending large log messages#4930
algorandskiy merged 1 commit intoalgorand:masterfrom
algorandskiy:pavel/large-log-line

Conversation

@algorandskiy
Copy link
Copy Markdown
Contributor

Summary

There is a possibility someone might log very large error messages, and they are lost if exceed the log rotation file size.
To prevent this, dump part of the message into stderr as well as callstack to see the caller.

Test Plan

Existing tests

Comment thread logging/cyclicWriter.go
Comment on lines +132 to +133
buf := make([]byte, 16*1024)
stlen := runtime.Stack(buf, false)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: debug.Stack() manages making a buffer for runtime.Stack() on your behalf, but 16K seems like it should be enough

10MB for min debug log line size is a lot!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we get into this branch if exceeding cyclic.limit that is 1GB by default. and I only want to print really big messages even if someone sets smaller log file size for rotation.

@codecov
Copy link
Copy Markdown

codecov Bot commented Dec 21, 2022

Codecov Report

Merging #4930 (34a5005) into master (2ab5e27) will increase coverage by 0.01%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master    #4930      +/-   ##
==========================================
+ Coverage   53.48%   53.50%   +0.01%     
==========================================
  Files         432      432              
  Lines       53616    53622       +6     
==========================================
+ Hits        28678    28691      +13     
+ Misses      22705    22701       -4     
+ Partials     2233     2230       -3     
Impacted Files Coverage Δ
logging/cyclicWriter.go 46.46% <0.00%> (-3.00%) ⬇️
cmd/tealdbg/debugger.go 72.69% <0.00%> (-0.81%) ⬇️
ledger/testing/randomAccounts.go 56.26% <0.00%> (ø)
network/wsNetwork.go 64.92% <0.00%> (+0.17%) ⬆️
network/wsPeer.go 69.45% <0.00%> (+2.38%) ⬆️
ledger/roundlru.go 96.22% <0.00%> (+5.66%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@algorandskiy algorandskiy merged commit 1313a7b into algorand:master Dec 22, 2022
algobarb pushed a commit to algobarb/go-algorand that referenced this pull request Dec 22, 2022
@algorandskiy algorandskiy deleted the pavel/large-log-line branch March 16, 2026 20:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants