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

chore(raiko-lib): auto logging #219

Merged
merged 19 commits into from
Jun 3, 2024
Merged

chore(raiko-lib): auto logging #219

merged 19 commits into from
Jun 3, 2024

Conversation

35359595
Copy link
Contributor

  • automatic logging if RUST_LOG env not present is set to info level;
  • println logs replaced with tracing::debug;
  • logging levels demoted to debug on most chatty outputs;
  • install script sources sp1 not only on CI but also if not found in current $PATH;

@35359595 35359595 temporarily deployed to test-environment May 17, 2024 00:40 — with GitHub Actions Inactive
@35359595 35359595 requested a review from Brechtpd May 17, 2024 00:40
@dantaik
Copy link

dantaik commented May 17, 2024

you have to fix your PR title first, something like "chore(project_name): verb ..."

Copy link
Contributor

@Brechtpd Brechtpd left a comment

Choose a reason for hiding this comment

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

Some good changes!

But feels like info now prints very little, and debug prints so much stuff it's hard to find anything back, so I think we need to tweak things some more so that the default log level still prints out a sufficient amount of useful information. Also seems like

debug!(" Ok: {result:?}");
needs to changed to trace level now if we want to use the debug level more because that one is very spammy and normally not useful.

I think it's okay to print some stuff using println for stuff that is mostly just important while you're waiting on the block, like the transaction progress or the data fetching progress. That stuff is not so important for the logs, but very useful while you're staring at it and wondering how long something will still take to finish.

Currently the block execution though is very messy because of the optimistic execution still printing a lot of stuff, I think to solve that we could not print/log anything when is_optimistic is true in execute_transactions. Or something like that. We could also have a custom logger trait that people could set but that seems overkill.

host/src/main.rs Outdated Show resolved Hide resolved
lib/src/lib.rs Outdated Show resolved Hide resolved
@Brechtpd Brechtpd requested a review from petarvujovic98 May 17, 2024 04:28
@petarvujovic98
Copy link
Contributor

Good stuff here @35359595. Some of the println usage for progress/waiting feedback could get hidden behind a #[cfg(debug_assertions] so it's only visible when running in dev mode (e.g. people looking at the screen for progress), not suitable for tracing as that data doesn't help with either metrics or debugging after the fact.a

@35359595 35359595 changed the title Chore/auto logging chore(raiko-lib): auto logging May 17, 2024
@35359595 35359595 had a problem deploying to test-environment May 17, 2024 21:07 — with GitHub Actions Failure
@35359595 35359595 temporarily deployed to test-environment May 17, 2024 21:31 — with GitHub Actions Inactive
@35359595 35359595 temporarily deployed to test-environment May 21, 2024 20:01 — with GitHub Actions Inactive
@35359595 35359595 requested a review from Brechtpd May 21, 2024 22:03
@johntaiko
Copy link
Contributor

How is the progress going here?
image
It has some negative impacts on our production environment, and we hope it can be resolved ASAP :)

@Brechtpd Brechtpd had a problem deploying to test-environment May 24, 2024 02:29 — with GitHub Actions Failure
@Brechtpd Brechtpd temporarily deployed to test-environment May 24, 2024 02:35 — with GitHub Actions Inactive
@johntaiko johntaiko temporarily deployed to test-environment May 24, 2024 09:22 — with GitHub Actions Inactive
lib/src/lib.rs Outdated Show resolved Hide resolved
@johntaiko johntaiko force-pushed the chore/auto_logging branch from c028930 to f13e527 Compare May 24, 2024 11:27
@35359595 35359595 had a problem deploying to test-environment May 28, 2024 01:05 — with GitHub Actions Failure
@35359595 35359595 temporarily deployed to test-environment May 28, 2024 01:43 — with GitHub Actions Inactive
@35359595 35359595 requested a review from johntaiko May 28, 2024 03:13
@35359595 35359595 temporarily deployed to test-environment May 28, 2024 17:48 — with GitHub Actions Inactive
lib/Cargo.toml Outdated Show resolved Hide resolved
lib/src/builder/execute.rs Outdated Show resolved Hide resolved
core/src/lib.rs Show resolved Hide resolved
lib/src/builder/execute.rs Outdated Show resolved Hide resolved
@35359595 35359595 temporarily deployed to test-environment May 31, 2024 16:34 — with GitHub Actions Inactive
@35359595 35359595 requested a review from petarvujovic98 May 31, 2024 16:35
@35359595 35359595 added this pull request to the merge queue Jun 3, 2024
Merged via the queue into main with commit 18bbfc9 Jun 3, 2024
14 checks passed
@35359595 35359595 deleted the chore/auto_logging branch June 3, 2024 16:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants