-
Notifications
You must be signed in to change notification settings - Fork 2k
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: Make agent syslog log level inherit from Nomad agent log #15625
Conversation
Signed-off-by: dttung2905 <[email protected]>
Signed-off-by: dttung2905 <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great @dttung2905! I agree that it makes the best sense to have the syslog logger inherit the log level from the agent. I've left a suggestion for how to treat the "OFF"
level, but I think what you've done for "TRACE"
is fine.
Thanks for the PR!
Signed-off-by: dttung2905 <[email protected]>
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good! I found one minor issue around case handling that became obvious once we checked for OFF
. Once that's fixed I think this is ready for merge.
Signed-off-by: dttung2905 <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks!
I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions. |
Fix issue : #15087
Hi Nomad Core Dev team,
Below is my attempt to fix the issue mentioned above. Imo, we can either make an extra argument for syslog log level or make it inherit from Nomad Agent existing one. This PR is using the latter approach because there is not need to use the former for a rarely used feature.
Would appreciate getting your feedback on this PR, especially on a few issues:
OFF
andTRACE
in Nomad log_level maps to gsyslog log levelui.ErrorWriter
orui.OutputWriter
. Please let me know if a unit test is needed or there are better ways to structure the unit test