gardener-attic / gardener-resource-manager

Kubernetes resource reconciliation controller capable of performing health checks for managed resources.
https://gardener.cloud
5 stars 19 forks source link

Make log level configurable #98

Closed timebertt closed 3 years ago

timebertt commented 3 years ago

How to categorize this PR?

/area logging usability /kind enhancement /priority normal

What this PR does / why we need it:

This PR adds the following flags:

Both options can also be set via make variables during make start.

Additionally, this PR cleans up logging across the whole codebase in order to make full use of structured logging and the newly configurable log level.

Which issue(s) this PR fixes: Fixes #66

Special notes for your reviewer:

Release note:

The log level of gardener-resource-manager is now configurable via `--zap-log-level`.
The log messages have been improved across the whole codebase to make log output more helpful and readable.
timebertt commented 3 years ago

https://github.com/gardener/gardener-resource-manager/pull/96 is merged, rebased. /ready

gardener-robot commented 3 years ago

@timebertt The pull request was assigned to you under author-action. Please unassign yourself when you are done. Thank you.

timebertt commented 3 years ago

I evaluated our options wrt choosing a logging library together with @timuthy again.

Although, logging implementations in zapr, klog, klogr, controller-runtime and all the plumbing in between are kind of confusing, we reached to the following conclusion: It probably makes sense for our components (grm, extensions and later on also g/g), to switch from zap/zapr and the flags defined by controller-runtime to klog/klogr and the flags defined by k8s.io/component-base/logs. Then, we can use to logr-adapter of klog (klogr) in our controllers and the controller-runtime managers.

This brings several advantages and solves some challenges:

The problem with this currently is, that the new structured logging options in component-base and the other k8s libraries are only available from v1.19 on, which means c-r would also need to be upgraded to v0.7.0, which is not done in g/g yet (ref https://github.com/gardener/gardener/issues/3109). There might still be some undiscovered drawbacks and uncertainties, which I couldn't check in detail because of this (I already expect some pitfalls with different log levels in the c-r manager's logs).

I would propose to close this PR for now and revisit it, once https://github.com/gardener/gardener/issues/3109 is done. Then we will be able to get a clearer picture of how all of this can be implemented in a clean way and decide on a target picture for grm and all our other components.

/close