-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Groom Tendermint Logs #5912
Comments
especially things like:
|
Executed a local network using simapp and looked for logs that seemed superfluous. This isn't by any means an exhaustive grooming, but should drastically help legibility of logs. ref: #5912
Should groom some logs outputted while syncing.
The above are logs for a block but when a node is syncing these values are not useful. It would be nice to see the highest height synced every second, if an error arises it will be displayed via an error log. Logging ends up looking like this Screen.Recording.2021-02-02.at.3.17.48.PM.mov |
Those logs are across multiple modules. It might be difficult to do that. Why is this bad? I don't think this is necessarily a bad thing. |
Not necessarily bad, just annoying and doesn't provide any value to the node operator. It could use the |
Neat! What's that? Could you point me to how/where that works? |
defined: https://github.com/tendermint/tendermint/blob/master/consensus/reactor.go#L47 used: https://github.com/tendermint/tendermint/blob/master/rpc/core/status.go#L64 It's currently used in RPC to tell node operators if they are syncing or not. It would be easier if we had start phases |
I'm confused. What does that have to do with logging and how does that translate across multiple reactors in terms of how they log? |
Please, merge this. It's very hard to develop, test and make operations with all this non-configurable logs in console. I'm using locally latest release patched to avoid all logs but it's not solution. Node operators were completely stunned when watched this. |
Stunned how? What logs do you believe should be downgraded or removed? Recall, previously logging had zero application logs -- it was all Tendermint. Now, all logs are present, it's just that Tendermint logs are naturally more verbose and this issue is more or less a meta-issue to investigate what can be removed or level-downgraded. Please post same example logs for us to look into. |
Executed a local network using simapp and looked for logs that seemed superfluous. This isn't by any means an exhaustive grooming, but should drastically help legibility of logs. ref: #5912
Executed a local network using simapp and looked for logs that seemed superfluous. This isn't by any means an exhaustive grooming, but should drastically help legibility of logs. ref: #5912
Executed a local network using simapp and looked for logs that seemed superfluous. This isn't by any means an exhaustive grooming, but should drastically help legibility of logs. ref: #5912
Next up, we need to groom logs in I think once we groom these, we should cut an 0.34.x release for the SDK to use @tessr . Logs should be pretty minimal then. |
Do we need to change any of the logs in the mempool? I remember them being a bit noisy in the e2e tests but maybe they've already been modified. |
Wanted to chime into why I'd also like to see less logging In my case I ship logs to a remote service, where it combs thru the logs looking for trouble and alerts via pushover when it finds strings that indicate trouble. So having a few MB per day is fine. But when it grows to nearly a hundreds of MBs per day, the remote service costs increase drastically. For example, this is a log line, which is surely not intended for humans
|
Yes! I've updated the issue body. |
@gaia that doesn't look like a log, but rather some sort of encoding. Can you past the full log in pastebin please? It's probably something from consensus which we've already addressed and is slated for 0.34.x |
yep, i looked for it in the raw logs, can't find it. this is how they should up at the external log facility it would be SUPER to be able to have logs NOT be colored (for exporting to 3rd party collectors) |
Not much we can do unless you have the actual log.
format: text -> human readable and meant to be colored You should be using json format always when sending logs to an external log facility. |
thx alex.
i have |
If that doesn't work a temporary solution is the use the Another log that would be great to move from Line 36 in 27eb10a
Line 1979 in 27eb10a
At least in the context of the following error message:
I cross-referenced the pubkey for that validator between the tendermint validator set and cosmos-sdk validators and turns out that it belongs to Line 1976 in 27eb10a
Bad peers might warrant an error message but in this particular case I think it would be better surfaced |
No, if you're sending logs to a log facility, use |
Thanks @RyanHendricks, sure, I can move this to info. Thanks for the rationale as well 👍 done in #6152 |
This is mainly completed, The last step would be an audit of what interfaces we include in logging, and ideally removing any of them. |
Summary
I introduced a PR into the Cosmos-SDK that changed the underlying logger and logging behavior -- cosmos/cosmos-sdk#8072. Please see the PR, specifically
log_level
section for the rationale, which I believe is strongly justified.Problem Definition
The problem that was introduced via cosmos/cosmos-sdk#8072 is that it exposed how verbose and superfluous many of Tendermint's logs are. The logs now are very very verbose, many of which are completely useless and uninformative or should be moved down a level (i.e. DEBUG) at the very least.
Because of the now exposed verbosity of Tendermint's log, it will becomes difficult for node operators to grok logs and make sense of them for useful diagnostic info. Of course, operators should be sending their logs to a logging service that provides rich query functionality, but still...
Proposal
Go through all of Tendermint's logs (a global search for
log.Info(...)
) and evaluate (1) does this log make sense/should we have it? And (2), if so, does this log make sense at INFO or should it be downgraded to DEBUG?I do not think it should take that much effort, but the review might be a bit lengthy. Perhaps instead of doing this in one fell swoop, we evaluate the noisy logs in Gaia and see what can be removed/downgraded and then incrementally go from there.
Groom/Investigate
/cc @marbar3778 @alessio
For Admin Use
The text was updated successfully, but these errors were encountered: