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

fix: propagate --log-level and --log-file to child processes #224

Merged
merged 1 commit into from
Aug 16, 2024

Conversation

nsrip-dd
Copy link
Contributor

What does this PR do?

If the log level is set via --log-level, set the corresponding environment
variables so that the child processes will get it. This was done for
--log-file already, but only if it was originally a relative path. Propagate
it always.

Motivation

I noticed this when investigating #223. I was setting the log level to "trace"
via the command line and didn't see much output. It took me a bit to realize
that the setting wasn't picked up by all the workers. I set the env var
instead, but ideally the command line argument works the
same way :)

Reviewer's Checklist

  • Changed code has unit tests for its functionality.

If the log level is set via `--log-level`, set the corresponding
environment variables so that the child processes will get it. This was
done for `--log-file` already, but only if it was originally a relative
path. Propagate it always.

I noticed this when investigating #223. I was setting the log level to
"trace" via the command line and didn't see much output. It took me a
bit to realize that the setting wasn't picked up by all the workers. I
set the env var instead, but ideally the command line argument works the
same way :)
@nsrip-dd nsrip-dd requested a review from a team as a code owner August 15, 2024 20:26
Copy link

codecov bot commented Aug 15, 2024

Codecov Report

Attention: Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.

Project coverage is 61.50%. Comparing base (6bf164e) to head (03d085e).
Report is 2 commits behind head on main.

Files Patch % Lines
main.go 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #224      +/-   ##
==========================================
- Coverage   61.51%   61.50%   -0.02%     
==========================================
  Files          90       90              
  Lines        4914     4915       +1     
==========================================
  Hits         3023     3023              
- Misses       1568     1569       +1     
  Partials      323      323              
Flag Coverage Δ
ARM64 44.68% <0.00%> (-0.02%) ⬇️
Linux 66.13% <0.00%> (-0.02%) ⬇️
Windows 43.56% <0.00%> (-0.02%) ⬇️
X64 61.50% <0.00%> (-0.02%) ⬇️
generator 42.71% <ø> (ø)
go1.22 50.96% <0.00%> (-0.02%) ⬇️
go1.23 49.74% <0.00%> (-0.02%) ⬇️
integration 46.19% <0.00%> (-0.02%) ⬇️
macOS 44.68% <0.00%> (-0.02%) ⬇️
unit 40.00% <0.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@RomainMuller RomainMuller added this pull request to the merge queue Aug 16, 2024
Merged via the queue into main with commit 5da4513 Aug 16, 2024
20 checks passed
@RomainMuller RomainMuller deleted the nick.ripley/propagate-log-info branch August 16, 2024 08:56
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.

2 participants