Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add logging guideline #158

Merged
merged 1 commit into from
Oct 28, 2021
Merged

Conversation

njgheorghita
Copy link
Collaborator

Quick, initial logging guideline - very open to change if anybody has thoughts / formatting guidelines they'd like to change / add.

@pipermerriam
Copy link
Member

I would suggest we also go for the following display structure with fixed width "columns":

LEVEL TIMESTAMP  WHAT_HAPPENED   FIELD_A=VALUE_A FIELD_B=VALUE_B

This is largely a copy of what geth does.

@jacobkaufmann
Copy link
Collaborator

In my experience, it's very useful to have conventions for logging relevant data structures/fields so that querying/searching logs is less of a treasure hunt. It looks like what @pipermerriam suggested would be good. I think this would require some conventions around how we implement fmt::Display for data types as we did for Ping and Pong in #157.

@jacobkaufmann
Copy link
Collaborator

I also forgot to add a note about logging request IDs, but it looks like discv5 doesn't expose the request ID of a TalkRequest so I opened a PR.

Copy link
Contributor

@lithp lithp left a comment

Choose a reason for hiding this comment

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

This sounds good to me, no matter what log format we end up standardizing on these three bullets will be relevant advice.

@lithp
Copy link
Contributor

lithp commented Oct 26, 2021

Agree with @jacobkaufmann, I think the primary value of a standardized format is that it makes logs easier to grep. e.g. if you're looking for a record of all interactions with some peer you should be able to find it by grepping for node_id=$NODEID.

This is enabled by both the Ping(node_id=xxx) format and by the structured logging format @pipermerriam suggests, I'm happy with either!

@njgheorghita
Copy link
Collaborator Author

Merging this for now and tracking a more general log format standardization convo over here (#168) - would love to hear any input.

@njgheorghita njgheorghita merged commit 5bb16f2 into ethereum:master Oct 28, 2021
@njgheorghita njgheorghita deleted the logging-guidelines branch October 28, 2021 19:38
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants