status-im / status-desktop

Status Desktop client made in Nim & QML
https://status.app
Mozilla Public License 2.0
292 stars 78 forks source link

Replace `geth.Logger` usage with `zap.Logger` across status-go repo #16536

Open osmaczko opened 5 days ago

osmaczko commented 5 days ago

Description

In status-go, the usage of geth.Logger should be replaced with zap.Logger, and a custom handler should be created for the root geth.Logger to proxy logs to a zap.Logger instance named "geth." This will allow us to filter logs originating from there.

Context:

Logs in geth.log are incomplete; they do not include the logger name. This issue arises because we are propagating zap logs to the root geth.Logger, which results in the loss of some information. Additionally, this logging flow seems incorrect. Instead of zap.Logger->geth.Logger, it should likely be geth.Logger->zap.Logger (i.e., from a specific logger to a more generic one).

Notes

go-ethereum has updated its logging utility, and when we rebase our fork, we will need to use the SetDefault function instead of SetHandler.

igor-sirotin commented 4 days ago

@osmaczko This should probably be in status-go repo?

osmaczko commented 4 days ago

@osmaczko This should probably be in status-go repo?

Yes, ideally it should. However, since the parent resides in status-desktop, I would prefer to keep it as is. By the way, I am aware that the parent should also belong to status-go, and it did previously, but I moved it because the goal labels in status-desktop aren't accessible in status-go 🙈

igor-sirotin commented 4 days ago

@osmaczko I guess we should copy the goals labels to status-go then.