fix: honor klog -stderrthreshold even when -logtostderr is true#168
fix: honor klog -stderrthreshold even when -logtostderr is true#168pierluigilenoci wants to merge 4 commits into
Conversation
klog v2 defaults -logtostderr to true, which silently ignores -stderrthreshold — all log levels go to stderr unconditionally. This has been an open issue since 2020 (kubernetes/klog#212). klog v2.140.0 introduced a fix behind an opt-in flag (legacy_stderr_threshold_behavior). This commit bumps klog to v2.140.0 and enables the fix in both entry points so that -stderrthreshold is honored, while preserving the current default behavior (stderrthreshold=INFO means all logs still go to stderr unless the user overrides it on the command line). Ref: kubernetes/klog#212 Ref: kubernetes/klog#432 Signed-off-by: Pierluigi Lenoci <[email protected]>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #168 +/- ##
=========================================
- Coverage 10.51% 9.89% -0.63%
=========================================
Files 49 49
Lines 7029 6501 -528
=========================================
- Hits 739 643 -96
+ Misses 6238 5806 -432
Partials 52 52
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Cover the new klog flag overrides (legacy_stderr_threshold_behavior and stderrthreshold) introduced in the previous commit to satisfy the codecov patch coverage threshold (70%). Signed-off-by: Pierluigi Lenoci <[email protected]> Signed-off-by: Pierluigi Lenoci <[email protected]>
|
Added a unit test for Regarding the |
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="pkg/config/args_log_test.go">
<violation number="1" location="pkg/config/args_log_test.go:36">
P2: Non-fatal `assert.NotNil` is followed by unconditional dereference, which can panic and obscure the real assertion failure.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
|
||
| // legacy_stderr_threshold_behavior should be opted out | ||
| legacy := set.Lookup("legacy_stderr_threshold_behavior") | ||
| assert.NotNil(t, legacy, "expected legacy_stderr_threshold_behavior flag") |
There was a problem hiding this comment.
P2: Non-fatal assert.NotNil is followed by unconditional dereference, which can panic and obscure the real assertion failure.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At pkg/config/args_log_test.go, line 36:
<comment>Non-fatal `assert.NotNil` is followed by unconditional dereference, which can panic and obscure the real assertion failure.</comment>
<file context>
@@ -0,0 +1,44 @@
+
+ // legacy_stderr_threshold_behavior should be opted out
+ legacy := set.Lookup("legacy_stderr_threshold_behavior")
+ assert.NotNil(t, legacy, "expected legacy_stderr_threshold_behavior flag")
+ assert.Equal(t, "false", legacy.Value.String())
+
</file context>
|
@yue9944882 @Somefive — friendly ping, could one of you review this when you get a chance? The |
Signed-off-by: Pierluigi Lenoci <[email protected]>
Signed-off-by: Pierluigi Lenoci <[email protected]>
|
Hi — friendly follow-up. CI is green, merge state is clean, and all checks are passing. Would you be able to review when you get a chance? Thank you! |
What changed
klog v2 defaults
-logtostderrtotrue, which silently ignores-stderrthreshold— all log levels go to stderr unconditionally. This has been an open issue since 2020.klog v2.140.0 introduced a fix behind an opt-in flag (
legacy_stderr_threshold_behavior). This PR bumps klog to v2.140.0 and enables the fix in both entry points (cmd/addon-manager/main.goandpkg/config/args_log.go) so that-stderrthresholdis honored, while preserving the current default behavior.References
Summary by cubic
Honor
-stderrthresholdwhen-logtostderr=trueby enablingklog’s new behavior. Default stderr level stays at INFO unless overridden.Bug Fixes
legacy_stderr_threshold_behavior=falseand defaultstderrthreshold=INFOincmd/addon-manager/main.goandpkg/config/args_log.go; add unit test forAddLogFlags.Dependencies
k8s.io/klog/v2tov2.140.0.Written for commit a378e5d. Summary will update on new commits.