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.

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

[CI] Disable runtime logging for benchmarks #1463

Merged
merged 7 commits into from
Sep 12, 2023
Merged

[CI] Disable runtime logging for benchmarks #1463

merged 7 commits into from
Sep 12, 2023

Conversation

ggwpez
Copy link
Member

@ggwpez ggwpez commented Sep 8, 2023

Changes:

  • Disable runtime logging in benchmarks by building with a specific profile

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
@ggwpez ggwpez added T0-node This PR/Issue is related to the topic “node”. T4-runtime_API This PR/Issue is related to runtime APIs. labels Sep 8, 2023
@paritytech-ci paritytech-ci requested review from a team September 8, 2023 12:38
@paritytech-ci paritytech-ci requested a review from a team September 8, 2023 13:28
@ggwpez ggwpez requested a review from bkchr September 8, 2023 19:45
Copy link
Member

@bkchr bkchr left a comment

Choose a reason for hiding this comment

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

Could you please give more information on why? I may assume that there are benchmarks that spamming the execution. This sounds more like that they should not spam info or whatever with debug information?

@ggwpez
Copy link
Member Author

ggwpez commented Sep 10, 2023

Could you please give more information on why? I may assume that there are benchmarks that spamming the execution. This sounds more like that they should not spam info or whatever with debug information?

They are not just spamming debug, but also warn and error.
That makes it difficult to see the actual benchmark output from the CLI. Eg:

Maybe i can also intercept the runtime-interface in the benchmarking CLI and redirect the logs there.

@bkchr
Copy link
Member

bkchr commented Sep 11, 2023

Benchmarks should probably just be build like the production binary with on-chain-release-build enabled, which disables any kind of logging.

@ggwpez
Copy link
Member Author

ggwpez commented Sep 11, 2023

Benchmarks should probably just be build like the production binary with on-chain-release-build enabled, which disables any kind of logging.

Oh yea, did not think about this. Thanks!

@ggwpez ggwpez closed this Sep 11, 2023
@ggwpez
Copy link
Member Author

ggwpez commented Sep 11, 2023

(Actually going to try it out here)

@ggwpez ggwpez reopened this Sep 11, 2023
@ggwpez ggwpez changed the title Runtime: env var for log level [CI] Disable runtime logging for benchmarks Sep 11, 2023
@ggwpez ggwpez marked this pull request as draft September 11, 2023 10:50
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
@ggwpez
Copy link
Member Author

ggwpez commented Sep 11, 2023

Okay seems to work. Westend benchmark log file was ~1.25 MB on a recent MR and is now just ~600KB here.
Searching for string Failed to convert BEEFY PublicKey to ETH address also yields no results in the new log.

@ggwpez ggwpez marked this pull request as ready for review September 11, 2023 16:50
@ggwpez ggwpez requested a review from bkchr September 11, 2023 16:51
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T0-node This PR/Issue is related to the topic “node”. T4-runtime_API This PR/Issue is related to runtime APIs.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants