diff options
| -rw-r--r-- | contributors/devel/sig-instrumentation/migration-to-structured-logging.md | 104 |
1 files changed, 79 insertions, 25 deletions
diff --git a/contributors/devel/sig-instrumentation/migration-to-structured-logging.md b/contributors/devel/sig-instrumentation/migration-to-structured-logging.md index cd213297..c29814b5 100644 --- a/contributors/devel/sig-instrumentation/migration-to-structured-logging.md +++ b/contributors/devel/sig-instrumentation/migration-to-structured-logging.md @@ -201,6 +201,12 @@ has different meaning for variadic arguments. Instead of just passing arguments, argument name and argument value. This means when migrating a log call we need to add an additional string before each argument, that will be used as it's name. +How variable arguments should be used: + +```go +klog.InfoS("message", "key1", value1, "key2", "value2") +``` + For example ```go func LogHTTP(r *http.Request) { @@ -214,37 +220,85 @@ func LogHTTP(r *http.Request) { } ``` -Names of arguments should use [lowerCamelCase] and be alphanumeric. Arguments names in one log call should be unique. -Names should be picked based on semantic meaning of value itself, not the context in which is used (log message should -imply the context). For example names like `status` should be used over (`desiredStatus`, `oldStatus`, `badStatus`) thus -allowing to query and join different log lines of the `status` field. - -Kubernetes objects should be referenced using only their kind, no matter their api group or version. Example argument -names: `deployment`, `pod`, `node`, `replicaSet`. For objects of unknown type, is ok to log them under `object` key with -addition of `apiVersion` and `kind` fields describing the k8s object type. - -In situations when we want to the log value of the same meaning twice (e.g. transition between state) it is ok to name -an additional argument based on context, but leaving one most current/correct value with canonical name. - -Examples of keys (strongly suggested, will be extended when pattern emerge, no standard schema yet): -* `err` - error when using `klog.InfoS`. Used for expected errors that are not `klog.ErrorS`. -* `object` - reference to k8s objects of unknown type. Should be used with `kind` and `apiVersion`. -* `kind` - kind of k8s object of unknown type. -* `apiVersion` - API version of k8s object of unknown type. - -Example: +When deciding on names of arguments you should: +* Always use [lowerCamelCase], for example use `containerName` and not `container name` or `container_name`. +* Use [alphanumeric] characters: no special characters like `%$*`, non-latin, or unicode characters. +* Use object kind when referencing Kubernetes objects, for example `deployment`, `pod` and `node`. +* Describe the type of value stored under the key and use normalized labels: + * Don't include implementation-specific details in the labels. Don't use `directory`, do use `path`. + * Do not provide additional context for how value is used. Don't use `podIP`, do use `IP`. + * With the exception of acronyms like "IP" and the standard "err", don't shorten names. Don't use `addr`, do use `address`. + * When names are very ambiguous, try to include context in the label. For example, instead of + `key` use `cacheKey` or instead of `version` use `dockerVersion`. +* Be consistent, for example when logging file path we should always use `path` and not switch between + `hostPath`, `path`, `file`. + +Here are a few exceptions to the rules above---some cases are temporary workarounds that may change if we settle on better solution: +* Do use `err` rather than `error` to match the key used by `klog.ErrorS` +* Context in name is acceptable to distinguish between values that normally go under same key. For example using both + `status` and `oldStatus` in log that needs to show the change between statuses. +* When Kubernetes object kind is unknown without runtime checking we should use `object` key. To provide information + about kind we should add separate `apiVersion` and `kind` fields. +* If we cannot use `klog.KObj` nor `klog.KRef` for Kubernetes object, like in cases when we only have access to name or UID, + 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`. + +Examples of **good keys** (strongly suggested, will be extended when pattern emerge, no standard schema yet): +* `cacheKey` +* `cacheValue` +* `CIDR` +* `containerID` +* `containerName` +* `controller` +* `cronJob` +* `deployment` +* `dockerVersion` +* `duration` +* `err` +* `job` +* `object` +* `pod` +* `podName` +* `podUID` +* `PVC` +* `PV` +* `volumeName` +* `replicaSet` + +Examples of **bad** keys: +* `addr` - replace with `address` +* `container` - replace with `containerName` or `containerID` depending on value +* `currentNode` - replace with `node` +* `directory` - replace with `path` +* `elapsed` - replace with `duration` +* `externalIP` - replace with `IP` +* `file` - replace with `path` +* `hostPath` - replace with `path` +* `ip` - replace with `IP` +* `key` - replace with key describing what kind of key it is, for example `cacheKey` +* `loadBalancerIP` - replace with `IP` +* `podFullName` - try to rewrite code so that pod name or pod object can be used with `pod` or `podName` keys +* `podIP` - replace with `IP` +* `podList` - replace with `pods` +* `version` - replace with key describing what it belongs to so that it can be compared, for example `dockerVersion` +* `servicePortName` - replace with `portName` +* `svc` - replace with `service` + +Example of using context in to distinguish between two same keys: ```go -func ChangeStatus(newStatus, currentStatus string) { - err := changeStatus(newStatus) - if err != nil { - klog.ErrorS(err, "Failed changing status", "desiredStatus", newStatus, "status", currentStatus) - } - klog.InfoS("Changed status", "previousStatus", currentStatus, "status", newStatus) +func ChangePodStatus(newStatus, currentStatus string) { + klog.InfoS("PodStatusController found pod with status", "status", currentStatus) + ... + // Logic that changes status + ... + klog.InfoS("PodStatusController changed pod status", "oldStatus", currentStatus, "status", newStatus) } ``` [lowerCamelCase]: https://en.wiktionary.org/wiki/lowerCamelCase +[alphanumeric]: https://en.wikipedia.org/wiki/Alphanumeric ### Use `klog.KObj` and `klog.KRef` for Kubernetes objects |