Skip to content

feat(consume): enable logging, propagate logs to hive results #1361

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

Merged
merged 22 commits into from
Apr 2, 2025

Conversation

danceratopz
Copy link
Member

@danceratopz danceratopz commented Mar 27, 2025

🗒️ Description

The Besu Team reported some flaky looking eest/consume-rlp results in the Pectra dashboard, without any logging from the simulator side, we're essentially blind as to the timing of the simulator setup with regards to client startup. This PR adds logging to help debug this.

Changes

  1. Use pytest's --tb short to only show relevant tracebacks for unexpected exceptions in test execution (e.g., timeout on rpc call; requests exceptions are notoriously noisy).
  2. Add a new logging plugin. This is pretty ugly stuff, but the complexity seems necessary to achieve:
    a. Getting timestamps in pytest's capd output (pytest has amazing logging, but this seemed unachievable with the built-in plugin, would be happy to be wrong).
    b. Clean logging to file per logger with xdist.
    c. Adding log lines to the file for start and end of test.
  3. Propagates stdout and stderr capd from pytest to the pytest test result so it's available in the test result in hiveview.

Example of Hiveview test case fail

Still with a bug; setup stdout is duplicated in call stdout.
image

Example of Hiveview test case pass

With bug fix (setup stdout no longer duplicated to call stdout).
image

Example of file logging

2025-03-27 09:42:00.502+00:00 [INFO] pytest_plugins.logging: ℹ️  - START TEST: src/pytest_plugins/consume/hive_simulators/rlp/test_via_rlp.py::test_via_rlp[tests/istanbul/eip1344_chainid/test_chainid.py::test_chainid[fork_Cancun-blockchain_test_from_state_test]-go-ethereum_default]
2025-03-27 09:42:00.531+00:00 [INFO] pytest_plugins.consume.hive_simulators.conftest: Starting client (go-ethereum_default)...
2025-03-27 09:42:01.409+00:00 [INFO] pytest_plugins.consume.hive_simulators.conftest: Client (go-ethereum_default) ready!
2025-03-27 09:42:01.411+00:00 [INFO] pytest_plugins.consume.hive_simulators.rlp.test_via_rlp: Calling getBlockByNumber to get genesis block...
2025-03-27 09:42:01.416+00:00 [INFO] pytest_plugins.consume.hive_simulators.rlp.test_via_rlp: Calling getBlockByNumber to get latest block...
2025-03-27 09:42:01.421+00:00 [INFO] pytest_plugins.logging: ✅ - PASSED in 0.01s: src/pytest_plugins/consume/hive_simulators/rlp/test_via_rlp.py::test_via_rlp[tests/istanbul/eip1344_chainid/test_chainid.py::test_chainid[fork_Cancun-blockchain_test_from_state_test]-go-ethereum_default]
2025-03-27 09:42:01.421+00:00 [INFO] pytest_plugins.consume.hive_simulators.conftest: Stopping client (go-ethereum_default)...
2025-03-27 09:42:01.564+00:00 [INFO] pytest_plugins.consume.hive_simulators.conftest: Client (go-ethereum_default) stopped!
2025-03-27 09:42:01.569+00:00 [INFO] pytest_plugins.logging: ℹ️  - END TEST: src/pytest_plugins/consume/hive_simulators/rlp/test_via_rlp.py::test_via_rlp[tests/istanbul/eip1344_chainid/test_chainid.py::test_chainid[fork_Cancun-blockchain_test_from_state_test]-go-ethereum_default]
2025-03-27 09:42:01.569+00:00 [INFO] pytest_plugins.logging: ℹ️  - START TEST: src/pytest_plugins/consume/hive_simulators/rlp/test_via_rlp.py::test_via_rlp[tests/istanbul/eip1344_chainid/test_chainid.py::test_chainid[fork_Shanghai-blockchain_test_from_state_test]-go-ethereum_default]
2025-03-27 09:42:01.575+00:00 [INFO] pytest_plugins.consume.hive_simulators.conftest: Starting client (go-ethereum_default)...
2025-03-27 09:42:02.500+00:00 [INFO] pytest_plugins.consume.hive_simulators.conftest: Client (go-ethereum_default) ready!
2025-03-27 09:42:02.502+00:00 [INFO] pytest_plugins.consume.hive_simulators.rlp.test_via_rlp: Calling getBlockByNumber to get genesis block...
2025-03-27 09:42:02.507+00:00 [INFO] pytest_plugins.consume.hive_simulators.rlp.test_via_rlp: Calling getBlockByNumber to get latest block...
2025-03-27 09:42:02.512+00:00 [INFO] pytest_plugins.logging: ✅ - PASSED in 0.01s: src/pytest_plugins/consume/hive_simulators/rlp/test_via_rlp.py::test_via_rlp[tests/istanbul/eip1344_chainid/test_chainid.py::test_chainid[fork_Shanghai-blockchain_test_from_state_test]-go-ethereum_default]
2025-03-27 09:42:02.513+00:00 [INFO] pytest_plugins.consume.hive_simulators.conftest: Stopping client (go-ethereum_default)...
2025-03-27 09:42:02.677+00:00 [INFO] pytest_plugins.consume.hive_simulators.conftest: Client (go-ethereum_default) stopped!
2025-03-27 09:42:02.688+00:00 [INFO] pytest_plugins.logging: ℹ️  - END TEST: src/pytest_plugins/consume/hive_simulators/rlp/test_via_rlp.py::test_via_rlp[tests/istanbul/eip1344_chainid/test_chainid.py::test_chainid[fork_Shanghai-blockchain_test_from_state_test]-go-ethereum_default]

Example of console logging with fail

Pytest provides this out the box, but without timestamps 😭
image

🔗 Related Issues

✅ Checklist

  • All: Set appropriate labels for the changes.
  • All: Considered squashing commits to improve commit history.
  • All: Added an entry to CHANGELOG.md.

@danceratopz danceratopz added type:feat type: Feature scope:consume Scope: Consume command suite labels Mar 27, 2025
@danceratopz danceratopz requested a review from Copilot March 27, 2025 10:06
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds logging capabilities to help debug timing issues in simulator and client interactions by propagating logs to hive test results and configuring more concise tracebacks.

  • Use pytest’s --tb short for cleaner tracebacks.
  • Introduce a new logging plugin to add timestamps and per-worker log files.
  • Propagate captured stdout/stderr into test result details for hiveview.

Reviewed Changes

Copilot reviewed 4 out of 6 changed files in this pull request and generated 2 comments.

File Description
src/pytest_plugins/pytest_hive/pytest_hive.py Adjusts test reporting to include captured output and modifies how test details are aggregated.
src/pytest_plugins/logging.py Introduces a logging plugin that creates per-worker log files with UTC timestamps and colorful terminal output.
src/pytest_plugins/consume/hive_simulators/rlp/test_via_rlp.py Adds logging statements before RPC calls and improves assertion messages for clarity.
src/pytest_plugins/consume/hive_simulators/conftest.py Inserts logging calls to mark the lifecycle events of clients.
Files not reviewed (2)
  • pytest-consume.ini: Language not supported
  • whitelist.txt: Language not supported

Copy link
Contributor

@spencer-tb spencer-tb left a comment

Choose a reason for hiding this comment

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

This must have been hard to get working! Really happy to have this. Minor comments below :)

Assuming consume engine additions in another PR?

@danceratopz
Copy link
Member Author

danceratopz commented Mar 27, 2025

Assuming consume engine additions in another PR?

@spencer-tb We should probably add a couple here for good measure. I was hoping to do a fast follow-up PR with rpc call retries, in which case the logging will require re-writing anyway. But let's get them in here, it's only a few mins.

Copy link
Member

@marioevz marioevz left a comment

Choose a reason for hiding this comment

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

Some comments, or just like questions in general, but overall looks pretty good.

Copy link
Member

@marioevz marioevz left a comment

Choose a reason for hiding this comment

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

LGTM, I think we could improve this in the future, by adding fill for example, but as of now seems good to go. Thanks!

@marioevz
Copy link
Member

marioevz commented Apr 2, 2025

I've debugged the unit test fail and funnily enough, it does not fail when you select the failing test only:

uv run pytest -c ./pytest-framework.ini src/pytest_plugins/consume/tests/test_consume_args.py

passes all tests, but doing:

uv run pytest -c ./pytest-framework.ini src/pytest_plugins/consume/tests/test_consume_args.py src/pytest_plugins/logging/tests/test_logging.py

makes them fail.

@danceratopz
Copy link
Member Author

I've debugged the unit test fail and funnily enough, it does not fail when you select the failing test only:

uv run pytest -c ./pytest-framework.ini src/pytest_plugins/consume/tests/test_consume_args.py

passes all tests, but doing:

uv run pytest -c ./pytest-framework.ini src/pytest_plugins/consume/tests/test_consume_args.py src/pytest_plugins/logging/tests/test_logging.py

makes them fail.

Thanks so much for starting to debug this, I was confused about the CI fail as I was sure that I fixed it locally.

Should be sorted now.

@danceratopz danceratopz merged commit 3eddd28 into main Apr 2, 2025
21 checks passed
@danceratopz danceratopz deleted the consume-add-logging branch April 2, 2025 21:19
felix314159 pushed a commit to felix314159/execution-spec-tests that referenced this pull request May 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope:consume Scope: Consume command suite type:feat type: Feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants