Discard log messages when initializing the provider #4650

t0yv0 · 2024-10-16T19:41:31Z

Messages sent early to the "log" package end up being forwarded to stderr, and Pulumi displays stderr by default to users, which is undesirable.

Later in the provider life-cycle, Pulumi Terraform Bridge will reset log.SetOutput again to capture and appropriately track diagnostic logs, which should still work with this change.

Fixes #4645

A test is added to ensure we do not leak diagnostics again on a simple program.

Messages sent early to the "log" package end up being forwarded to stderr, and Pulumi displays stderr by default to users, which is undesirable. Later in the provider life-cycle, Pulumi Terraform Bridge will reset `log.SetOutput` again to capture and appropriately track diagnostic logs, which should still work with this change. Fixes #4645 A test is added to ensure we do not leak diagnostics again on a simple program.

github-actions · 2024-10-16T20:07:45Z

Does the PR have any schema changes?

Looking good! No breaking changes found.
No new resources/functions.

Maintainer note: consult the runbook for dealing with any breaking changes.

flostadler · 2024-10-16T20:13:11Z

provider/resources.go

@@ -5876,3 +5877,7 @@ func setupComputedIDs(prov *tfbridge.ProviderInfo) {
 		return attrWithSeparator(state, ":", "name", "restoreTestingPlanName"), nil
 	}
 }
+
+func init() {
+	log.SetOutput(io.Discard)


Can we test in any way that the bridge actually resets this again in the end? I'm a bit worried that the bridge behavior will be eventually changed and then this here just send everything into the void for aws

Can possibly try testing something like this statement in ACM certificate read:

log.Printf("[WARN] ACM Certificate %s not found, removing from state", arn)

I couldn't find an existing test for this so constructed one using aws.s3.Bucket and object.

This is very similar to pulumi/pulumi-databricks#474 (comment). If this works, then I don't think it needs to be in init (I think init occurs after all dependencies have been loaded, ordering of Go's life before main is largely UB). If you put this at the top of the Provider function, does it not work? That would give you guaranteed scoping with a defer log.SetOutput(os.Stdout).

t0yv0 · 2024-10-17T13:11:13Z

provider/upstream_provider.go

@@ -30,6 +33,9 @@ import (
 // runtime startup is somewhat performance sensitive. Therefore any modifications undertaken here should complete
 // quickly.
 func newUpstreamProvider(ctx context.Context) awsShim.UpstreamProvider {
+	log.SetOutput(io.Discard)      // ignore logging from upstream constructors


I have moved here @iwahbe @flostadler that still works.

This is not entirely the right thing to do. We need to revisit bridge Main APIs and clean them up. The bridge should be initializing the provider and setting up logging against the Pulumi CLI host before calling into the provider-specific constructors. Then instead of discarding these logs would get the regular log sink.

We can only get so far with that strategy, but we should do it just to clean up the proliferation of Main APIs.

pulumi-databricks hit log statements during variable setup (before main).

flostadler

LGTM

t0yv0 · 2024-10-17T13:24:31Z

/release patch

pulumi-bot · 2024-10-17T18:57:35Z

This PR has been shipped in release v6.56.1.

This commit changes the log level from plugin stdout&stderr to debug, from info. This change is significant because info is shown by default to users, but debug is not. While this is a change of plugin aesthetics, it does not effect any runtime behavior. All plugins, both component providers and custom resource providers, are effected. > [!IMPORTANT] > This is a breaking change, in that information which used to be displayed to users will > now be hidden by default. > > We will need to share this change with our users before rolling it out. The providers team has met and believe that this is the best approach for our ecosystem. For a full discussion on why this change is necessary, see [this doc](https://docs.google.com/document/d/1yYrwTwsNoayaIzKyG1l5cl0MjJxiHLsH4NqAvZkQN-I/edit?tab=t.0#heading=h.34v61lg1x4kl). Fixes pulumi/pulumi-terraform-bridge#2489 Fixes pulumi/pulumi-cloudngfwaws#23 Fixes pulumi/pulumi-ise#9 Taking this change will allow us to revert: - pulumi/pulumi-databricks#609 - pulumi/pulumi-aws#4650

This commit changes the log level from plugin stdout&stderr to debug, from info. This change is significant because info is shown by default to users, but debug is not. While this is a change of plugin aesthetics, it does not effect any runtime behavior. All plugins, both component providers and custom resource providers, are effected. > [!IMPORTANT] > This is a breaking change, in that information which used to be displayed to users will > now be hidden by default. > > We will need to share this change with our users before rolling it out. The providers team has met and believe that this is the best approach for our ecosystem. For a full discussion on why this change is necessary, see [this doc](https://docs.google.com/document/d/1yYrwTwsNoayaIzKyG1l5cl0MjJxiHLsH4NqAvZkQN-I/edit?tab=t.0#heading=h.34v61lg1x4kl). Fixes pulumi/pulumi-terraform-bridge#2489 Fixes pulumi/pulumi-cloudngfwaws#23 Fixes pulumi/pulumi-ise#9 Taking this change will allow us to revert: - pulumi/pulumi-databricks#609 - pulumi/pulumi-aws#4650 --- When a provider exists ungracefully, we dump *all* unstructured logs to the user. This allows providers to output stack traces to users (so they can report them) when a provider fails. We do not search for `panic` messages to isolate the stack trace, since we cannot do that effectively in a cross-language way.

t0yv0 requested a review from a team October 16, 2024 19:41

flostadler reviewed Oct 16, 2024

View reviewed changes

Add a test for log.Printf propagation

412e85a

t0yv0 requested a review from flostadler October 16, 2024 21:28

PR feedback

d2e81cf

t0yv0 commented Oct 17, 2024

View reviewed changes

t0yv0 requested a review from iwahbe October 17, 2024 13:11

flostadler approved these changes Oct 17, 2024

View reviewed changes

t0yv0 enabled auto-merge (squash) October 17, 2024 13:24

github-actions bot added the needs-release/patch When a PR with this label merges, it initiates a release of vX.Y.Z+1 label Oct 17, 2024

t0yv0 merged commit 855caa5 into master Oct 17, 2024
33 checks passed

t0yv0 deleted the t0yv0/fix-4645 branch October 17, 2024 15:02

pulumi-bot mentioned this pull request Oct 17, 2024

Initializing Terraform Provider diagnostic printed for every program #4645

Closed

github-actions bot removed the needs-release/patch When a PR with this label merges, it initiates a release of vX.Y.Z+1 label Oct 17, 2024

mjeffryes assigned t0yv0 Oct 25, 2024

iwahbe mentioned this pull request Dec 6, 2024

Log Plugin stdout & stderr as debug (instead of info) pulumi/pulumi#17943

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Discard log messages when initializing the provider #4650

Discard log messages when initializing the provider #4650

t0yv0 commented Oct 16, 2024

github-actions bot commented Oct 16, 2024

flostadler Oct 16, 2024

t0yv0 Oct 16, 2024

t0yv0 Oct 16, 2024

iwahbe Oct 17, 2024

t0yv0 Oct 17, 2024

iwahbe Oct 17, 2024

flostadler left a comment

t0yv0 commented Oct 17, 2024

pulumi-bot commented Oct 17, 2024

Discard log messages when initializing the provider #4650

Discard log messages when initializing the provider #4650

Conversation

t0yv0 commented Oct 16, 2024

github-actions bot commented Oct 16, 2024

Does the PR have any schema changes?

flostadler Oct 16, 2024

Choose a reason for hiding this comment

t0yv0 Oct 16, 2024

Choose a reason for hiding this comment

t0yv0 Oct 16, 2024

Choose a reason for hiding this comment

iwahbe Oct 17, 2024

Choose a reason for hiding this comment

t0yv0 Oct 17, 2024

Choose a reason for hiding this comment

iwahbe Oct 17, 2024

Choose a reason for hiding this comment

flostadler left a comment

Choose a reason for hiding this comment

t0yv0 commented Oct 17, 2024

pulumi-bot commented Oct 17, 2024