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

Pass log level to executed script for more granular logging possibility #119

Merged
merged 1 commit into from
Apr 12, 2024

Conversation

andywaltlova
Copy link
Collaborator

@andywaltlova andywaltlova commented Mar 14, 2024

@andywaltlova andywaltlova force-pushed the feat/pass-loglevel-to-script branch from 0bbbba4 to 98787ac Compare March 14, 2024 15:33
@codecov-commenter
Copy link

codecov-commenter commented Mar 14, 2024

Codecov Report

Attention: Patch coverage is 72.72727% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 65.90%. Comparing base (4407b0e) to head (b96326d).

Files Patch % Lines
src/util.go 57.14% 1 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #119      +/-   ##
==========================================
+ Coverage   65.88%   65.90%   +0.02%     
==========================================
  Files           5        5              
  Lines         255      264       +9     
==========================================
+ Hits          168      174       +6     
- Misses         67       68       +1     
- Partials       20       22       +2     
Flag Coverage Δ
go-1.19 65.90% <72.72%> (+0.02%) ⬆️
go-1.20 65.90% <72.72%> (+0.02%) ⬆️

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.

@andywaltlova
Copy link
Collaborator Author

Tested with custom script executable agianst stage today, it works as expected:

  • This is log when rhc contains log-level = "trace" - 'INFO' value is passed to script
...
Apr 03 12:58:35 kvm-02-guest19.rhts.eng.brq.redhat.com rhcd[32119]: 2024/04/03 12:58:35 runner.go:123: Writing temporary script to /var/lib/rhc-worker-script
Apr 03 12:58:35 kvm-02-guest19.rhts.eng.brq.redhat.com rhcd[32119]: 2024/04/03 12:58:35 runner.go:128: Processing script ...
Apr 03 12:58:35 kvm-02-guest19.rhts.eng.brq.redhat.com rhcd[32119]: 2024/04/03 12:58:35 runner.go:131: Executing script...
Apr 03 12:58:35 kvm-02-guest19.rhts.eng.brq.redhat.com rhcd[32119]: 2024/04/03 12:58:35 runner.go:83: Successfully set env variable RHC_WORKER_SCRIPT_MODE
Apr 03 12:58:35 kvm-02-guest19.rhts.eng.brq.redhat.com rhcd[32119]: 2024/04/03 12:58:35 runner.go:90: Successfully set env variable LOG_LEVEL to INFO
Apr 03 12:58:35 kvm-02-guest19.rhts.eng.brq.redhat.com rhcd[32119]: 2024/04/03 12:58:35 runner.go:136: Command is being executed with following env variables:  [YGG_SOCKET_ADDR=unix:@yggd-dispatcher-lDOHNX PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin RHC_WORKER_SCRIPT_MODE=ANALYSIS LOG_LEVEL=INFO]
...

@andywaltlova andywaltlova marked this pull request as ready for review April 3, 2024 11:18
@andywaltlova
Copy link
Collaborator Author

andywaltlova commented Apr 3, 2024

And just note, that we have this in our code:

		// Yggdrasil < 3.0 does not share its configured log level with the
		// workers in any way
		log.SetLevel(log.LevelInfo)

So worker and script will be logging everything as info if rhc doesn't share the log level with us. If/When there is possibility to pass custom variables to rhc workers user will be able to overwrite it.

If desired we could instead of info level fallback on value in worker config file - would have to be new config option value.

@andywaltlova andywaltlova requested a review from r0x0d April 3, 2024 11:35
src/runner.go Outdated Show resolved Hide resolved
@r0x0d r0x0d force-pushed the feat/pass-loglevel-to-script branch 3 times, most recently from 928ed10 to 652776f Compare April 3, 2024 13:49
@r0x0d r0x0d force-pushed the feat/pass-loglevel-to-script branch from 652776f to c839109 Compare April 11, 2024 19:04
r0x0d
r0x0d previously approved these changes Apr 11, 2024
Improve logging format
Try to read the script log level from config file, if that fails, use
yggdrasil default log level, and if that's not set, use "INFO" as the
default one.
@andywaltlova andywaltlova force-pushed the feat/pass-loglevel-to-script branch from c839109 to b96326d Compare April 12, 2024 12:21
@r0x0d r0x0d dismissed their stale review April 12, 2024 12:43

re-approval

@r0x0d r0x0d merged commit 3eab527 into oamg:main Apr 12, 2024
6 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