update migration-to-structured-logging.md #6719
Conversation
k8s-ci-robot
added
cncf-cla: yes
| @@ -407,6 +416,7 @@ Here are a few exceptions to the rules above---some cases are temporary workarou | |||
| then we should fallback to using object kind with suffix based on value type. For example `podName`, `podUID`. | |||
| * When providing multiple indistinguishable values (for example list of evicted pods), then we can use plural version of | |||
| argument name. For example we should use `pods` and not `podList`. | |||
| * we can use `Klog.KObjSlice` for Kubernetes objects. | |||
There was a problem hiding this comment.
IMHO, this is required here. This section talks about Here are a few exceptions to the rules above.
| @@ -467,16 +477,17 @@ func ChangePodStatus(newStatus, currentStatus string) { | |||
| ## Good practice for passing values in structured logging | |||
|
|
|||
| When passing a value for a key-value pair, please use following rules: | |||
| * Prefer using Kubernetes objects (for example `*v1.Pod`) and log them using `klog.KObj` | |||
| * Prefer using Kubernetes objects (for example `*v1.Pod`) and log them using `klog.KObj` or `klog.KObjSlice` | |||
There was a problem hiding this comment.
KObjSlice is better off with resources having more than one entries. Using that for *v1.Pod for example is probably not a good idea right ?
| * When the original object is not available, use `klog.KRef` instead | ||
| * When object sets, we use `klog.KObjSlice` |
There was a problem hiding this comment.
I think this line needs a bit more clarification. Did you mean When the object is slice type ?
yangjunmyfm192085
force-pushed
the
updatestructlog
branch
from
5da8214
to
d7ab088
Compare
There was a problem hiding this comment.
@harshanarayana , Thanks for the very detailed review, I have applied all the suggestions, Could you please take a look again?
k8s-ci-robot
added
the
lgtm
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: pohly, serathius, yangjunmyfm192085 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
Which issue(s) this PR fixes:
Fixes part of #110737