Navigation Menu

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

Notify users when webhooks are disabled #1570

Merged
merged 2 commits into from Mar 10, 2023

Conversation

pavolloffay
Copy link
Member

Resolves #1569

Signed-off-by: Pavol Loffay <p.loffay@gmail.com>
@pavolloffay pavolloffay requested a review from a team as a code owner March 10, 2023 08:39
@pavolloffay pavolloffay added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Mar 10, 2023
main.go Outdated
@@ -241,6 +241,8 @@ func main() {
instrumentation.NewMutator(logger, mgr.GetClient()),
}),
})
} else {
ctrl.Log.Info("Webhooks are disabled, operator is running in not-supported mode", "ENABLE_WEBHOOKS", "false")
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be be warning rather than info? And one small suggestion:

Suggested change
ctrl.Log.Info("Webhooks are disabled, operator is running in not-supported mode", "ENABLE_WEBHOOKS", "false")
ctrl.Log.Info("Webhooks are disabled, operator is running in an unsupported mode", "ENABLE_WEBHOOKS", "false")

Copy link
Member

Choose a reason for hiding this comment

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

A warning sounds reasonable.

Copy link
Member Author

Choose a reason for hiding this comment

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

changed the message.

I am not sure how to produce a message on warn level. There is this just an API to get a logger at specific verbosity level. I didn't manage to get warn logs with it.

logger.V(N).Info("test")

Signed-off-by: Pavol Loffay <p.loffay@gmail.com>
@jaronoff97 jaronoff97 merged commit f23dd8a into open-telemetry:main Mar 10, 2023
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Skip Changelog PRs that do not require a CHANGELOG.md entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

No errors logged when failing to reconcile with webhooks disabled
4 participants