Add klog-verbosity flag to config-logging ConfigMap#9035
Add klog-verbosity flag to config-logging ConfigMap#9035Ankitsinghsisodya wants to merge 8 commits intoknative:mainfrom
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Ankitsinghsisodya The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Hi @Ankitsinghsisodya. Thanks for your PR. I'm waiting for a knative member to verify that this patch is reasonable to test. If it is, they should reply with Tip We noticed you've done this a few times! Consider joining the org to skip this step and gain Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
There was a problem hiding this comment.
Pull request overview
Adds support for configuring Kubernetes klog verbosity via the existing config-logging ConfigMap (and via env var for env-configured adapters), so operators can increase client-go/internal Kubernetes logging without code changes.
Changes:
- Adds
klog-verbosity: "0"to theconfig-loggingConfigMap (core + test manifests). - Implements parsing/apply + live update of klog verbosity in
pkg/utils/logging.go, wires it into adapter v2 configmap-based and env-based logger setup. - Adds unit tests for
SetKlogVerbosityFromConfigMapand introduces a new.github/dependabot.ymlconfig.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
config/core/configmaps/logging.yaml |
Documents and adds the klog-verbosity key to the shipped logging ConfigMap. |
test/config/configmaps/logging.yaml |
Updates test ConfigMap to include klog-verbosity. |
pkg/utils/logging.go |
Adds klog verbosity parsing/apply logic and a ConfigMap watcher callback. |
pkg/utils/logging_test.go |
Unit tests for valid/invalid/absent verbosity values. |
pkg/adapter/v2/configurator_configmap.go |
Applies initial klog verbosity from ConfigMap and watches for live updates. |
pkg/adapter/v2/config.go |
Adds K_KLOG_VERBOSITY env support and applies it when constructing the logger. |
.github/dependabot.yml |
Adds weekly GitHub Actions Dependabot updates configuration. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #9035 +/- ##
==========================================
+ Coverage 51.07% 51.12% +0.05%
==========================================
Files 409 409
Lines 21999 22042 +43
==========================================
+ Hits 11236 11270 +34
- Misses 9903 9905 +2
- Partials 860 867 +7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Exposes klog verbosity level (used by k8s client-go) via the config-logging ConfigMap under the klog-verbosity key. Adapters using ConfigMap-based logging apply the level on startup and update it dynamically via the existing ConfigMap watcher. Env-based adapters read K_KLOG_VERBOSITY injected from the ConfigMap. Fixes: knative#8706
- Use package-level FlagSet (init once, reuse) instead of creating new FlagSet + calling klog.InitFlags on every ConfigMap update - Combine int validation with range check (0-9); drop redundant Atoi - Only emit INFO log when verbosity actually changes to non-zero value - Inject K_KLOG_VERBOSITY env var from ConfigWatcher.ToEnvVars() so env-based adapters receive it via the reconciler (fixes dead-code path) - Add test coverage for UpdateKlogVerbosityFromConfigMap (all log paths) - Add out-of-range and negative test cases for SetKlogVerbosityFromConfigMap
- Remove "0" short-circuit so operators can reset verbosity after raising it - Guard klogFlagSet.Set with sync.Mutex — flag.FlagSet.Set is not goroutine-safe - Call SetKlogVerbosityFromConfigMap in updateFromLoggingConfigMap so the controller/reconciler process applies klog verbosity to its own output
- Add K_KLOG_VERBOSITY to expected env vars in apiserversource and pingsource receive adapter tests (EmptyVarsGenerator now returns 3 vars) - Promote k8s.io/klog/v2 from indirect to direct in go.mod
42fd2ef to
f220df4
Compare
|
@creydr can you please run /retest? I think these these test cases are flaky. |
Return whether verbosity was actually applied so callers don't need to re-implement the no-op check. UpdateKlogVerbosityFromConfigMap now branches on the returned bool instead of duplicating the level != "" && level != "0" guard. Three callers that only need the error use _, err :=.
aa69fb2 to
d2f9943
Compare
creydr
left a comment
There was a problem hiding this comment.
Thanks for working on this.
I left a few comments
Replace SetKlogVerbosityFromConfigMap with a direct SetKlogVerbosity function that simplifies verbosity application. Update related calls in ConfigWatcher and tests to reflect this change, ensuring proper logging behavior for zero values. This refactor enhances clarity and reduces redundancy in the logging configuration process.
Summary
Fixes #8706
klog-verbositykey toconfig-loggingConfigMap (default"0") to control klog verbosity for Kubernetes client-go internalsK_KLOG_VERBOSITYenv var, which can be injected from the ConfigMappkg/utils/logging.gowith unit testsTest plan
go test ./pkg/utils/...passes (7 new unit tests for valid/invalid/absent values)klog-verbosity: "5"inconfig-logging, confirm klog output at verbosity 5 appears in adapter logs