search

cert-manager / trust-manager

trust-manager is an operator for distributing trust bundles across a Kubernetes cluster.
https://cert-manager.io/docs/projects/trust-manager/
Apache License 2.0
229 stars 63 forks source link

Add support for JSON logging format #354

Closed erikgb closed 1 month ago

erikgb commented 1 month ago

In this PR I propose to introduce slog.Handler as a backend for logr (and klog) - as documented here.

I am doing a minimal change in this PR where my primary goal is to add support for a structured log format (ref. https://github.com/cert-manager/trust-manager/issues/282).

NOTE: This might break for some users, depending on if/how they parse trust-manager log output. I cannot imagine the slog text format (the new default) is the same as the deprecated klog format. We always try to avoid breaking changes, but I think a change is required here anyway - since the currently used klog format is deprecated.

Fixes https://github.com/cert-manager/trust-manager/issues/282

hawksight commented 1 month ago

/lgtm

cert-manager-prow[bot] commented 1 month ago

@hawksight: adding LGTM is restricted to approvers and reviewers in OWNERS files.

In response to [this](https://github.com/cert-manager/trust-manager/pull/354#issuecomment-2120216062): >/lgtm > Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.
erikgb commented 1 month ago

Had a bit of a read through of the PR and of the linked documentation. This seems to make sense to me. The notion for using slog is that this is in the Go standard library now? I can't approve and can't comment explicitly on the code, but if it works 👍

Can I run this? just need to branch checkout and build the image?

Thanks @hawksight! You should be able to just checkout the branch and install it. My workstation environment is really borked for working with open source.

cert-manager-prow[bot] commented 1 month ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hawksight, SgtCoDFish

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: - ~~[OWNERS](https://github.com/cert-manager/trust-manager/blob/main/OWNERS)~~ [SgtCoDFish] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment