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

auth: streamline logging #25965

Merged
merged 4 commits into from Jun 8, 2023

Conversation

mhofstetter
Copy link
Member

@mhofstetter mhofstetter commented Jun 7, 2023

This PR streamlines logging within the auth package.

  • replacing package global logger with injected one (by hive)
  • use structured logging with fields
  • remove prefix auth: from the message itself (which is a duplicate of the field subsys)
  • Start log messages in uppercase

For more detail - have a look at the individual commits.

@mhofstetter mhofstetter added kind/cleanup This includes no functional changes. release-note/misc This PR makes changes that have no direct user impact. area/servicemesh GH issues or PRs regarding servicemesh labels Jun 7, 2023
@mhofstetter mhofstetter requested a review from a team as a code owner June 7, 2023 06:55
@mhofstetter mhofstetter requested a review from meyskens June 7, 2023 06:55
@mhofstetter mhofstetter changed the title auth: refactor logging auth: streamline logging Jun 7, 2023
@mhofstetter
Copy link
Member Author

Runtime test (privileged) consistently fails due to #25968

@mhofstetter
Copy link
Member Author

rebased to main to fetch fix for #25968

Currently package auth is using a package global defined logger.

Hive provides the possibility to have a logger injected, which provides
the context of the hive module.

This way refactors all occurrences to use the injected logger by
configuring all components accordingly.

Signed-off-by: Marco Hofstetter <marco.hofstetter@isovalent.com>
This commit renames the auth module' id from `auth-manager` to `auth`.

Signed-off-by: Marco Hofstetter <marco.hofstetter@isovalent.com>
This commit adjusts the log messages by using fields for providing
context and by removing the prefix `auth: ` which is redundant to the
logger' subsystem.

Signed-off-by: Marco Hofstetter <marco.hofstetter@isovalent.com>
This commit adjusts the log messages by using fields for providing
context.

Signed-off-by: Marco Hofstetter <marco.hofstetter@isovalent.com>
@mhofstetter
Copy link
Member Author

rebased to main without any changes to get and check against the latest auth changes

Copy link
Contributor

@youngnick youngnick left a comment

Choose a reason for hiding this comment

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

LGTM, nice work.

@youngnick youngnick removed the request for review from meyskens June 8, 2023 10:09
@mhofstetter
Copy link
Member Author

/test

@mhofstetter
Copy link
Member Author

ci-ginkgo testrun failed with #25964 -> won't retry because it's not required

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jun 8, 2023
@dylandreimerink dylandreimerink merged commit d515de3 into cilium:main Jun 8, 2023
63 of 64 checks passed
@mhofstetter mhofstetter deleted the pr/mhofstetter/auth-logger branch June 8, 2023 14:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/servicemesh GH issues or PRs regarding servicemesh kind/cleanup This includes no functional changes. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants