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

Set log.Default's output to io.Discard #482

Merged

Conversation

ulucinar
Copy link
Collaborator

@ulucinar ulucinar commented Mar 14, 2024

Description of your changes

Fixes #471

This PR sets a default io.Discard logger for the controller-runtime if debug logging is not enabled. If debug logging is enabled, then the controller-runtime uses a debug mode zap logger as usual.

It also sets the log.Default's output to io.Discard. According to our experiments with a Topic.pubsub & ServiceAccount.cloudplatform, this prevents the noisy logs from the underlying Terraform provider. We still need to check for direct log messages via the fmt variants, such as fmt.Println.

In a further iteration, we will also consider making the underlying provider's logs available in a structured format.

Because the various self-hosted runners available for the upbound Github organization are not available for the crossplane-contrib organization, this PR also switches to the standard-runners branch of upbound/uptest for the reusable workflows to make the CI jobs run on standard hosted runners.

The PR also bumps the build submodule version to commit 75a9fe3ae6b6de82c5f7ddc6a267617940f16b83 and pins the UXP version used in uptest runs to 1.14.6-up.1. Please see this comment.

I have:

  • Run make reviewable test to ensure this PR is ready for review.

How has this code been tested

Tested manually with the Topic.pubsub & ServiceAccount.cloudplatform example manifests.

if debug logging is not enabled.

- Set log.Default's output to io.Discard.

Signed-off-by: Alper Rifat Ulucinar <[email protected]>
Signed-off-by: Alper Rifat Ulucinar <[email protected]>
(cherry picked from commit 4a1ffb9)
@ulucinar
Copy link
Collaborator Author

/test-examples="examples/pubsub/topic.yaml"

Copy link
Collaborator

@sergenyalcin sergenyalcin left a comment

Choose a reason for hiding this comment

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

Thanks @ulucinar LGTM!

@ulucinar
Copy link
Collaborator Author

/test-examples="examples/pubsub/topic.yaml"

@sergenyalcin
Copy link
Collaborator

sergenyalcin commented Mar 14, 2024

The Uptest Job will fail because of this issue.

@ulucinar could you please pin the UXP_VERSION in Makefile to 1.14.6-up.1 and also update the build submodule at least 75a9fe3?

@ulucinar
Copy link
Collaborator Author

/test-examples="examples/pubsub/topic.yaml"

@ulucinar ulucinar merged commit 06973d5 into crossplane-contrib:release-0.41 Mar 14, 2024
10 checks passed
@ulucinar ulucinar deleted the release-0.41-fix-logs branch March 14, 2024 13:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants