Skip to content

Conversation

ackintosh
Copy link
Member

@ackintosh ackintosh commented May 3, 2025

Issue Addressed

Beacon logs in the simulator are printed only to stdout. The logs are usually large, so persisting them would be helpful for debugging.

Proposed Changes

Added --log-dir parameter to the simulators and a step to upload the logs to Artifacts.

(Update)
Added --disable-stdout-logging to disable stdout logging, making the CI page cleaner.

Additional Info

I have checked that the CI works and the beacon logs are available for download.

image

@ackintosh ackintosh added the test improvement Improve tests label May 3, 2025
@ackintosh ackintosh marked this pull request as ready for review May 4, 2025 11:02
@ackintosh ackintosh added the ready-for-review The code is ready for review label May 4, 2025
Copy link
Member

@macladson macladson left a comment

Choose a reason for hiding this comment

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

This is a great change! One thing we could consider is turning off stdout logging for CI as well, since the logs get truncated anyway.

@ackintosh
Copy link
Member Author

ackintosh commented May 7, 2025

Thanks for your feedback! That makes sense. I'll go ahead and add --disable-stdout-logging.

@ackintosh
Copy link
Member Author

Done. It looks much cleaner. ✨

https://github.com/sigp/lighthouse/actions/runs/14894201749/job/41833311270?pr=7394#step:5:995

Copy link

mergify bot commented May 8, 2025

This pull request has merge conflicts. Could you please resolve them @ackintosh? 🙏

# Conflicts:
#	.github/workflows/test-suite.yml
#	testing/simulator/src/basic_sim.rs
Copy link
Member

@macladson macladson left a comment

Choose a reason for hiding this comment

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

LGTM!

@macladson macladson added ready-for-merge This PR is ready to merge. and removed ready-for-review The code is ready for review labels May 9, 2025
mergify bot added a commit that referenced this pull request May 9, 2025
@mergify mergify bot merged commit e0c1f27 into sigp:unstable May 9, 2025
31 checks passed
@ackintosh ackintosh deleted the sim-log branch May 9, 2025 20:40
mergify bot pushed a commit that referenced this pull request May 15, 2025
This PR relates to:
- #7199
- -> workspace_filter has been enabled (dependency logging has been disabled)
- #7394
- -> file logging has been optionally enabled

Building on these, this PR enables dependency logging for the simulators. The logs are written to separate files.

The libp2p/discv5 logs:
- are saved to the directory  specified with `--log-dir`
- respects the `RUST_LOG` environment variable for log level configuration
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-merge This PR is ready to merge. test improvement Improve tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants