Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Reduce the verbosity of eth2network #1597

Merged
merged 2 commits into from
Oct 12, 2023
Merged

Reduce the verbosity of eth2network #1597

merged 2 commits into from
Oct 12, 2023

Conversation

otherview
Copy link
Contributor

Why this change is needed

https://github.com/obscuronet/obscuro-internal/issues/2281

Reduces the eth2 network logs to error

What changes were made as part of this PR

Please provide a high level list of the changes made

PR checks pre-merging

Please indicate below by ticking the checkbox that you have read and performed the required
PR checks

  • PR checks reviewed and performed

@coderabbitai
Copy link

coderabbitai bot commented Oct 12, 2023 •
edited
Loading

Walkthrough

The changes primarily focus on adjusting the verbosity level of log outputs in various functions across the eth2network and obscuroscan integration tests. This is achieved by modifying the --verbosity flag in the gethStartNode, prysmStartBeaconNode, and prysmStartValidator functions. Additionally, a delay has been introduced in the obscuroscan_test.go file to allow for certain operations to complete.

Changes

File Path Summary
.../eth2network/eth2_network.go The --verbosity flag is adjusted in three functions: gethStartNode (set to "1"), prysmStartBeaconNode, and prysmStartValidator (both set to "error"). This aims to control the log output level for better error handling and debugging.
.../obscuroscan/obscuroscan_test.go A delay of 5 minutes is introduced with time.Sleep(5 * time.Minute) to allow for certain operations to complete.
Tips

Chat with CodeRabbit Bot (@coderabbitai)

  • Mention @coderabbitai in any review comment for bot assistance.
  • Note: Review comments are made on code diffs or files, not on the PR overview.

Pause Incremental Reviews

  • Insert @coderabbitai: ignore in the PR description to halt the bot's ongoing reviews. Remove the line to resume.
  • Additionally, enter @coderabbitai pause to halt the bot's ongoing reviews as any issue or review comment. To
    resume reviews, enter @coderabbitai resume as any issue or review comment.

@otherview otherview force-pushed the pedro/mute_eth2network_logs branch from e897e27 to db2f4c9 Compare October 12, 2023 11:29
BedrockSquirrel
BedrockSquirrel approved these changes Oct 12, 2023
View reviewed changes
Copy link
Collaborator

@BedrockSquirrel BedrockSquirrel left a comment

Choose a reason for hiding this comment

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

lgtm, comment suggestion

integration/eth2network/eth2_network.go Outdated
@@ -390,6 +390,7 @@ func (n *Impl) gethStartNode(executionPort, networkPort, httpPort, wsPort int, d
"--allow-insecure-unlock", // allows to use personal accounts over http/ws
"--nodiscover", // don't try and discover peers
"--ipcdisable", // avoid geth erroring bc the ipc path is too long
"--verbosity", "1",
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm guessing this is ERROR level too but worth adding comment next to it to say what '1' means probably

coderabbitai[bot]
coderabbitai bot reviewed Oct 12, 2023
View reviewed changes
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 0

Commits Files that changed from the base of the PR and between 7dfd9ab and db2f4c9.
Files selected for processing (1)
  • integration/eth2network/eth2_network.go (3 hunks)
Additional comments (Suppressed): 3
integration/eth2network/eth2_network.go (3)

  • 393-393: The verbosity level for the gethStartNode function has been reduced to "1". This will limit the logs to only show important messages. Ensure that this does not hide any crucial information needed for debugging or monitoring.

  • 442-442: The verbosity level for the prysmStartBeaconNode function has been changed from "debug" to "error". This will significantly reduce the amount of log output, but it may also hide useful debugging information. Verify that this change does not impact the ability to troubleshoot issues.

  • 467-467: A new verbosity level "error" has been added to the prysmStartValidator function. This will limit the logs to only show error messages. Ensure that this does not hide any crucial information needed for debugging or monitoring.

coderabbitai[bot]
coderabbitai bot reviewed Oct 12, 2023
View reviewed changes
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 0

Commits Files that changed from the base of the PR and between db2f4c9 and 4a91386.
Files selected for processing (1)
  • integration/eth2network/eth2_network.go (3 hunks)
Files skipped from review due to trivial changes (1)
  • integration/eth2network/eth2_network.go

@otherview otherview merged commit b5620b1 into main Oct 12, 2023
2 checks passed
@otherview otherview deleted the pedro/mute_eth2network_logs branch October 12, 2023 12:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments

@BedrockSquirrel BedrockSquirrel BedrockSquirrel approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@otherview @BedrockSquirrel