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

add eve_log_level parameter to GH test workflow #1033

Merged
merged 2 commits into from
Nov 11, 2024

Conversation

europaul
Copy link
Contributor

It would be useful to be able to set the log level for EVE when running tests, as e.g. "debug" log level would provide more information for debugging failed tests.

It would be good to include this change in the next release of Eden, so that it can be used workflows started from EVE repository.

@uncleDecart
Copy link
Member

But how would you execute a workflow with debug log level in EVE? You want to introduce manual eden test run action?

@europaul
Copy link
Contributor Author

But how would you execute a workflow with debug log level in EVE? You want to introduce manual eden test run action?

No, I would just set it for the existing eden test workflow for every PR. Like this lf-edge/eve#4381

eve_log_level:
type: string
required: false
default: 'info'
Copy link
Contributor

Choose a reason for hiding this comment

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

Before using this from the eve repo, we should try to set 'debug' first here.
We need to check how large the output artifacts will be and whether they are not exceeding some github limits.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I couldn't find any official documentation on the topic, but there is a rumor that the limit is 5 GB per file.

Our artifacts still stay under 50 MB https://github.com/lf-edge/eden/actions/runs/11437464614?pr=1033

Copy link
Contributor Author

Choose a reason for hiding this comment

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

before
after
Here are the before and after screenshots. I'd say the sizes stay comparable.

Copy link
Contributor

Choose a reason for hiding this comment

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

Does it actually work?
When I look for example into https://github.com/lf-edge/eden/actions/runs/11437464614/artifacts/2081755978,
I do not see debug logs inside eve-info.tar.gz
debug.default.loglevel is configured to info in persist-status/zedagent/ConfigItemValueMap/global.json

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I just noticed that as well 🤦
Will fix very soon

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, now I think I got it right
image
the sizes have increased but still not dramatic

Copy link
Contributor Author

Choose a reason for hiding this comment

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

maybe we can try running it like this for a while and if we reach some limit we can still revert it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

btw github copilot can provide some interesting insights that cannot be found in the github documentation
image
I'm just wondering if it's true or hallucinations 😄

@europaul europaul force-pushed the eve-log-level-workflow-parameter branch 2 times, most recently from 726ea06 to c1481dd Compare October 21, 2024 19:17
@europaul
Copy link
Contributor Author

folks, could you give this PR another look? I think I answered all the comments and fixed the issues.
let me know if it's good to merge and I'll remove the last two commits, which were here for demonstration purposes only.

Copy link
Contributor

@milan-zededa milan-zededa left a comment

Choose a reason for hiding this comment

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

LGTM and I would suggest to keep set log-level to debug for all tests so that we have more logs also here in the eden repo. Just revert the last commit.

It would be useful to be able to set the log level for EVE when running
tests, as e.g. "debug" log level would provide more information for
debugging failed tests.

It would be good to include this change in the next release of Eden, so
that it can be used workflows started from EVE repository.

Signed-off-by: Paul Gaiduk <[email protected]>
This commit is used for testing the workflow parameter for the log-level
It will be reverted before merging

Signed-off-by: Paul Gaiduk <[email protected]>
@europaul europaul force-pushed the eve-log-level-workflow-parameter branch from a44c42b to aa55191 Compare October 31, 2024 14:46
@uncleDecart uncleDecart merged commit 39bc844 into lf-edge:master Nov 11, 2024
16 of 19 checks passed
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.

4 participants