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

Update log calls to be more consistent #92

Merged

Conversation

JustinKuli
Copy link
Member

I was looking at this for ACM-7778, but I don't think that the logs here are particularly noisy - since the controller is event driven, knowing when it starts reconciling something (for example) can be quite useful. Instead, this PR just ensures that the controller's log messages are consistently formatted.

Signed-off-by: Justin Kulikauskas <jkulikau@redhat.com>
Copy link
Member

@dhaiducek dhaiducek left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for the updates!

I found the DisableStackTrace, which might go a long way toward making logs smaller all around since I find the error logs to be particularly noisy (and somewhat frequent): kubernetes-sigs/controller-runtime#1348

@openshift-ci
Copy link

openshift-ci bot commented Oct 13, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dhaiducek, JustinKuli

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 [JustinKuli,dhaiducek]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot merged commit f0e061c into open-cluster-management-io:main Oct 13, 2023
4 checks passed
@JustinKuli
Copy link
Member Author

That is neat! I hadn't thought about it much. The stack traces are kind of annoying when they're attached to common update failures because the resource has already been updated or something... but I feel like they could be useful for other errors (especially when we see an error that doesn't happen very often, any extra info on how it got there could be helpful).

What's more annoying to me is that the stack traces aren't in the same line as the log, so when I grep for specific things, they become unattached.

@dhaiducek
Copy link
Member

What's more annoying to me is that the stack traces aren't in the same line as the log, so when I grep for specific things, they become unattached.

...though that may be more because we're using console logging rather than json logging? Just a guess, though...

@JustinKuli JustinKuli deleted the log-alignment branch October 16, 2023 18:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants