Skip to content

📖 Update logging guidelines #13011

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Nov 21, 2025
Merged

📖 Update logging guidelines #13011

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b0409d6
Update logging guidelines
fabriziopandini Nov 19, 2025
ebb1d91
Address comments
fabriziopandini Nov 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion docs/book/src/developer/core/logging.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ In Cluster API we strive to follow three principles while implementing logging:
Kubernetes defines a set of [logging conventions](https://git.k8s.io/community/contributors/devel/sig-instrumentation/logging.md),
as well as tools and libraries for logging.

Cluster API should align to those guidelines and use those tools as much as possible.

## Continuous improvement

The foundational items of Cluster API logging are:
Expand Down Expand Up @@ -100,6 +102,11 @@ key value pairs (in order of importance):
creates a MachineSet.
- Other Key value pairs.

Notably, over time in CAPI we are also standardizing usage of other key value pairs to improve consistency when reading
logs, e.g.
- key `reason` MUST be used when adding details about WHY a change happened.
- key `diff` MUST be used when documenting the diff in an object that either lead to a chance, or that is resulting from a change.
Copy link
Contributor

@AndiDog AndiDog Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would be very helpful. Can we specify a suggested format? For example, subfields old/new, or string format (less helpful)?

Copy link
Member

@sbueringer sbueringer Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's extremely hard to standardize as we have a lot of different cases where we have different data that we can use

(check the usages of the "diff" key today in CAPI)


## Log Messages

- A Message MUST always start with a capital letter.
Expand All @@ -108,7 +115,13 @@ key value pairs (in order of importance):
the action log and the corresponding error log; While logging before the action, log verbs should use the -ing form.
- Ideally log messages should surface a different level of detail according to the target log level (see [log levels](#log-levels)
for more details).
- If Kubernetes resource name is used in log messages, it should be used as is, For example `Reconciling DockerMachineTemplate`
- If Kubernetes resource name is used in log messages, it MUST be used as is, For example `Reconciling DockerMachineTemplate`
- If an API field name is used in log messages, the entire path MUST be used and field names MUST capitalized like in the
API (not as in the golang type). For example `Waiting for spec.providerID to be set`
- If a log messages about a controlled or a referenced object, e.g. Machine controller performing an action on MachineSet,
the message MUST contain the Kind of the controlled/referenced object and its name, for example `Created MachineSet foo-bar`
- If the controlled/referenced object is in another namespace, use namespace/name instead of name
Comment on lines +122 to +123
Copy link
Contributor

@AndiDog AndiDog Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure to repeat the object name, which should already be a structured field? I'm not against this repetition to make it more readable for humans – just asking if this was considered.

The namespace should be in the structured field automatically (via klog.KRef). If we want the human log message to only contain the namespace if it's outside the "default", then I guess this means a utility function which does that?

Copy link
Member

@sbueringer sbueringer Nov 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure to repeat the object name, which should already be a structured field? I'm not against this repetition to make it more readable for humans – just asking if this was considered.

Yes, I think we should repeat it. My experience is that omitting it makes it unnecessarily hard to figure out what's going on, e.g.

Created Machine
k,v MachineSet namespace/name Machine namespace/name + a ton of other k/v pairs

Created Machine machine-1

With tools like Grafana/Kibana etc. it makes sense in my opinion to only show specific parts of the log, E.g. to only show controller-name + msg. If all the k/v pairs have to be shown for the log to make sense the view becomes extremely verbose

If we want the human log message to only contain the namespace if it's outside the "default", then I guess this means a utility function which does that?

I think we don't need a util for that. I'm not aware of a single case of this in core Cluster API. We just use %s and e.g. machine.Name. I think folks can just decide to either do that or use klog.KObj / klog.KRef as appropriate

- The controlled/referenced MUST also be added as a key value pair (see guidelines above)

## Log Levels

Expand Down