Skip to content
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.

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

Add logcheck tool #5442

Merged
merged 14 commits into from
Feb 16, 2022
Merged

Add logcheck tool #5442

merged 14 commits into from
Feb 16, 2022

Conversation

timebertt
Copy link
Member

@timebertt timebertt commented Feb 14, 2022

How to categorize this PR?

/area logging dev-productivity
/kind enhancement

What this PR does / why we need it:

Adds the logcheck tool, which enforces some parts of the logging guideline and also detects programmer-level errors (e.g. odd number of args).
After adding the tool, all reported errors on existing code are resolved.

Which issue(s) this PR fixes:
Part of #4251

Special notes for your reviewer:

Use the dedicated commits! The first 5 commits are about the tooling itself, the others resolve reported errors on existing code.
/squash

I might need to fix some stuff later on to make the tool reusable in extensions, but it's too hard to test and fix with replaces and some other go mod quirks.

Release note:

A new `logcheck` tool has been added: it aims at making logs across Gardener components more consistent and help detect programmer-level errors early on. Read more about it in the [tool's documentation](https://github.com/gardener/gardener/blob/master/hack/tools/logcheck).
The `controllercmd.LogErrAndExit` and `controller.*EventLogger` helper functions have been dropped in favor of proper error handling and structured logging, as their usage was not aligned with our logging guideline.

@rfranzke
Copy link
Member

/assign

Copy link
Member

@rfranzke rfranzke left a comment

Choose a reason for hiding this comment

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

Kudos, very nice PR! Well done!

Makefile Show resolved Hide resolved
hack/tools/logcheck/pkg/logcheck/logcheck.go Outdated Show resolved Hide resolved
@gardener-robot gardener-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Feb 16, 2022
Copy link
Member

@rfranzke rfranzke left a comment

Choose a reason for hiding this comment

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

/lgtm

@timebertt timebertt merged commit 46abeba into gardener:master Feb 16, 2022
@timebertt timebertt deleted the logcheck branch February 16, 2022 12:44
acumino added a commit to acumino/gardener-extension-provider-aws that referenced this pull request Mar 15, 2022
acumino added a commit to acumino/gardener-extension-provider-aws that referenced this pull request Mar 17, 2022
krgostev pushed a commit to krgostev/gardener that referenced this pull request Apr 21, 2022
* Document logcheck tool

* Implement logcheck tool

* Add tests for logcheck tool

* Install and execute logcheck golangci-lint plugin

* Drop obsolete go vet execution

golangci-lint already executes go vet, why should we run it twice?

* Remove trailing punction mark from messages

* Make keys lowerCamelCase

* Make messages captitalized

* Remove spaces from keys

* Fix odd number of args

* Make messages constant string expressions

* Drop LogErrAndExit in favor of proper error handling

* Drop controller.*EventLogger in favor of proper structured logging

These helper functions were not aligned with our logging guideline.
When logging objects, callers should simply pass a runtime.Object
or client.ObjectKey. Adding object.something is not so nice and
inconsistent with the codebase.

* Increase readability
krgostev pushed a commit to krgostev/gardener that referenced this pull request Jul 5, 2022
* Document logcheck tool

* Implement logcheck tool

* Add tests for logcheck tool

* Install and execute logcheck golangci-lint plugin

* Drop obsolete go vet execution

golangci-lint already executes go vet, why should we run it twice?

* Remove trailing punction mark from messages

* Make keys lowerCamelCase

* Make messages captitalized

* Remove spaces from keys

* Fix odd number of args

* Make messages constant string expressions

* Drop LogErrAndExit in favor of proper error handling

* Drop controller.*EventLogger in favor of proper structured logging

These helper functions were not aligned with our logging guideline.
When logging objects, callers should simply pass a runtime.Object
or client.ObjectKey. Adding object.something is not so nice and
inconsistent with the codebase.

* Increase readability
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/dev-productivity Developer productivity related (how to improve development) area/logging Logging related kind/enhancement Enhancement, improvement, extension size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants