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: disable debug log in Makefile #1210

Merged
merged 1 commit into from
Oct 10, 2022
Merged

Conversation

Wondertan
Copy link
Member

@Wondertan Wondertan commented Oct 7, 2022

The motivation here is to solve debug logs overload on the CI. In practice, they are not useful in any way for debugging issues on CI, which are usually flakiness. Debug logs only clog the output requiring devs to download the log archive and analyze what test did fail locally, which is a lot of unnecessary friction.

Removing the debug log keeps logging failed tests and error logs, allowing devs to identify failed tests and analyze/resolve the failures locally. Furthermore, it reduces the execution time of each action run by 2 minutes!

Also, we can consider keeping existing Makefile commands untouched and introduce new ones that CI can depend on.

@Wondertan Wondertan added the kind:misc Attached to miscellaneous PRs label Oct 7, 2022
@Wondertan Wondertan self-assigned this Oct 7, 2022
@codecov-commenter
Copy link

Codecov Report

Merging #1210 (b0f7e0e) into main (23c77e8) will increase coverage by 0.24%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #1210      +/-   ##
==========================================
+ Coverage   55.51%   55.75%   +0.24%     
==========================================
  Files         161      161              
  Lines        9582     9582              
==========================================
+ Hits         5319     5342      +23     
+ Misses       3735     3713      -22     
+ Partials      528      527       -1     
Impacted Files Coverage Δ
das/coordinator.go 92.64% <0.00%> (+1.47%) ⬆️
fraud/pb/proof.pb.go 36.61% <0.00%> (+1.89%) ⬆️
ipld/get_namespaced_shares.go 90.51% <0.00%> (+2.18%) ⬆️
fraud/bad_encoding.go 66.00% <0.00%> (+3.00%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@MSevey
Copy link
Member

MSevey commented Oct 7, 2022

Has the team discussed moving the logs to log files? This is only really a problem because we are logging to stdout right?

In practice, they are not useful in any way for debugging issues on CI

I would disagree with this. They might not be useful now in the current form, but debug logs in general are very useful. Especially if we are seeing differences between local and online CI runs.

It seems that the current handling of debug logs is not helpful, which is different than saying debug logs in general are not useful.

If there isn't a plan already, I would recommend putting together a plan to have logfiles for the node so that we can have the debug logs for CI and in production while at the same time, improving the online CI experience.

If that means merging this for the short term and bring back the -v flag in the future, that is OK with me.

@Wondertan
Copy link
Member Author

Has the team discussed moving the logs to log files? This is only really a problem because we are logging to stdout right?

@MSevey, Do you mean writing logs into the file during CI runs? How we can access those through GitHub? Not sure it provides such features, but I can see it sending logs to some endpoint or so. This is essentially the same as downloading them from GH, but still allows to ignore them if they are not needed(which is the usual case), so I am in favour.

It seems that the current handling of debug logs is not helpful, which is different than saying debug logs in general are not useful

Yeah, my English is not perfect, but i didn't mean in general, but in particular.

I am happy to improve the strategy for debug logs on CI without cutting them off completely. Previously, we just focused on reducing verbosity of dependencies, but this also reduce verbosity locally, which potentially complicates debugging.

@MSevey
Copy link
Member

MSevey commented Oct 7, 2022

@MSevey, Do you mean writing logs into the file during CI runs? How we can access those through GitHub? Not sure it provides such features, but I can see it sending logs to some endpoint or so. This is essentially the same as downloading them from GH, but still allows to ignore them if they are not needed(which is the usual case), so I am in favour.

I believe what we want is to use artifacts. https://docs.github.com/en/actions/using-workflows/storing-workflow-data-as-artifacts

Then ideally, the workflow logs are just showing the tests running/failing and all log output is stored in logfiles that can be downloaded in the event of a failure for debug purposes.

@renaynay
Copy link
Member

@MSevey I've created an issue to track the above suggestion #1211

Copy link
Member

@renaynay renaynay left a comment

Choose a reason for hiding this comment

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

Fine with this.

Ref #1211

Copy link
Collaborator

@distractedm1nd distractedm1nd left a comment

Choose a reason for hiding this comment

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

Having debug logs the way they currently are prevent us in most cases from seeing what is actually failing, so this is necessary at least until we find a better solution

@Wondertan Wondertan merged commit 661e61b into main Oct 10, 2022
@Wondertan Wondertan deleted the hlib/disable-debug-logs branch October 10, 2022 09:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:misc Attached to miscellaneous PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants