feat: implementing klog structured logging #79
Conversation
k8s-ci-robot
added
the
kind/cleanup
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: omerap12 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
cncf-cla: yes
|
cc @aramase |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #79 +/- ##
=======================================
Coverage ? 56.47%
=======================================
Files ? 12
Lines ? 1043
Branches ? 0
=======================================
Hits ? 589
Misses ? 410
Partials ? 44 ☔ View full report in Codecov by Sentry. |
omerap12
force-pushed
the
logger
branch
2 times, most recently
from
2a18c55 to
b894f68
Compare
|
When running these local tests, before installing the SSSC helm chart did you uncomment the |
|
/retitle feat: implementing klog for structured logging |
k8s-ci-robot
changed the title
|
/retitle feat: implementing klog structured logging |
k8s-ci-robot
changed the title
| Development: true, | ||
| } | ||
| opts.BindFlags(flag.CommandLine) | ||
| klog.InitFlags(nil) |
There was a problem hiding this comment.
| flag.Parse() | ||
|
|
||
| ctrl.SetLogger(zap.New(zap.UseFlagOptions(&opts))) |
There was a problem hiding this comment.
all setupLog instances here should be replaced to use klog
internal/controller/conditions.go
Outdated
| if ss.Status.Conditions == nil { | ||
| ss.Status.Conditions = []metav1.Condition{} | ||
| } | ||
|
|
||
| if len(oldConditionType) > 0 { | ||
| logger.V(10).Info("Removing old condition", "oldConditionType", oldConditionType) | ||
| klog.V(10).InfoS("Removing old condition", "oldConditionType", oldConditionType) |
There was a problem hiding this comment.
This is probably too high and will never get caught up in logs. Usually default log level is 2. Let's get this at V(4)
internal/controller/conditions.go
Outdated
| @@ -164,16 +162,16 @@ func (r *SecretSyncReconciler) updateStatusConditions(ctx context.Context, ss *s | |||
| condition.Message = ConditionMessageUnknown | |||
| } | |||
|
|
|||
| logger.V(10).Info("Adding new condition", "newConditionType", newConditionType, "conditionReason", conditionReason) | |||
| klog.V(10).InfoS("Adding new condition", "newConditionType", newConditionType, "conditionReason", conditionReason) | |||
internal/controller/conditions.go
Outdated
| } | ||
|
|
||
| logger.V(10).Info("Updated status", "condition", condition) | ||
| klog.V(10).InfoS("Updated status", "condition", condition) |
| @@ -180,7 +180,7 @@ func (p *PluginClientBuilder) Cleanup() { | |||
|
|
|||
| for k := range p.conns { | |||
| if err := p.conns[k].Close(); err != nil { | |||
| klog.ErrorS(err, "error shutting down provider connection", "provider", k) | |||
| klog.ErrorS(err, "Error shutting down provider connection", "provider", k) | |||
| @@ -208,10 +208,10 @@ func (p *PluginClientBuilder) HealthCheck(ctx context.Context, interval time.Dur | |||
|
|
|||
| runtimeVersion, err := Version(c, client) | |||
| if err != nil { | |||
| klog.V(5).ErrorS(err, "provider healthcheck failed", "provider", provider) | |||
| klog.V(5).ErrorS(err, "Provider healthcheck failed", "provider", provider) | |||
| continue | ||
| } | ||
| klog.V(5).InfoS("provider healthcheck successful", "provider", provider, "runtimeVersion", runtimeVersion) | ||
| klog.V(5).InfoS("Provider healthcheck successful", "provider", provider, "runtimeVersion", runtimeVersion) |
| @@ -248,7 +248,7 @@ func MountContent(ctx context.Context, client v1alpha1.CSIDriverProviderClient, | |||
| } | |||
| return nil, nil, err | |||
| } | |||
| klog.V(5).Info("finished mount request") | |||
| klog.V(5).InfoS("Finished mount request") | |||
| @@ -266,7 +266,7 @@ func MountContent(ctx context.Context, client v1alpha1.CSIDriverProviderClient, | |||
| // Individual k8s secrets are limited to 1MiB in size. | |||
| // Ref: https://kubernetes.io/docs/concepts/configuration/secret/#restrictions | |||
| if size := proto.Size(resp); size > 1048576 { | |||
| klog.InfoS("proto above 1MiB, secret sync may fail", "size", size) | |||
| klog.InfoS("Proto above 1MiB, secret sync may fail", "size", size) | |||
Signed-off-by: Omer Aplatony <omerap12@gmail.com>
k8s-ci-robot
added
size/L
Signed-off-by: Omer Aplatony <omerap12@gmail.com>
|
/retest-required |
|
The Kubernetes project currently lacks enough contributors to adequately respond to all PRs. This bot triages PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle stale |
k8s-ci-robot
added
the
lifecycle/stale
|
@omerap12: The following tests failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. Instructions 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. I understand the commands that are listed here. |
k8s-ci-robot
added
the
needs-rebase
|
PR needs rebase. Instructions 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. |
What type of PR is this?
/kind cleanup
What this PR does / why we need it:
Switches the controller logging to use klog instead of the current controller-runtime logger.
Which issue(s) this PR fixes (optional, using
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when the PR gets merged):Fixes #13
Special notes for your reviewer:
TODOs: