Consider adding UID to klog.ObjectRef

[tallclair]

Is there a reason UID isn't included in klog.ObjectRef?

In the Kubelet, we're seeing a mix of using klog.KObj(pod) and format.Pod(pod). Currently, the latter is preferred as it includes UID, which is often necessary for debugging Kubelet operations, but it would be nice if we could deprecate it in favor of a more standardized approach (KObj), which is contingent on UID being included.

[dgrisonnet]

/triage accepted

cc @pohly

[pohly]

I wasn't involved when ObjectRef was initially introduced (#128, https://github.com/kubernetes/enhancements/blob/master/keps/sig-instrumentation/20191115-structured-logging.md#helper-functions-for-log-format).

@serathius: do you remember whether the inclusion of UID was considered and why it was excluded?

Adding the UID to ObjectRef with omitempty would certainly be doable now. The bigger question is whether klog.KObj and friends should populate that field. I agree that it can be necessary to be precise, but it could also be seen as additional log noise by others.

[serathius]

For the klog.KObj function there were multiple additional fields that were proposed (UID, apiGroup, etc) , in the end we went with simplest and most common reference namespace/name assuming we can expand in the future.

If UID is important for Kubelet codebase, maybe we can come with option like klog.KObj(pod, klog.WithUID()) and make it a required for its subdirectories.

[tallclair]

I'm fine with making UID optional, but have a slight preference for a separate method rather than an option (which seems verbose for something used so frequently). Although, I suppose we could always just wrap it in a helper.

[pohly]

How should the UID be printed as string? <namespace>/<name>/<uid>?

I also prefer separate methods. forbidigo then can be used to restrict usage of the less informative klog.KObj in kubelet code.

klog.KObjUID and klog.KObjUIDSlice?

[tallclair]

How should the UID be printed as string? <namespace>/<name>/<uid>?

I vote <namespace>/<name>(<UID>). It's more consistent with the utility we currently use in kubelet (https://github.com/kubernetes/kubernetes/blob/9da4b7ecfb60ea484f8e6c89c42e63506571bc68/pkg/kubelet/util/format/pod.go#L40)