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

Adopt tflog #246

Closed
wants to merge 2 commits into from
Closed

Adopt tflog #246

wants to merge 2 commits into from

Conversation

bendbennett
Copy link
Contributor

Closes #245

@bendbennett
Copy link
Contributor Author

bendbennett commented Apr 27, 2022

@bflad given hashicorp/terraform-plugin-log#52, I'm assuming the changes in this PR will not yield the desired results as context.TODO() is being used. Can you suggest how logging should be handled as the SDK documentation indicates that MigrateState should remain in place for compatibility with existing state?

@detro
Copy link
Contributor

detro commented Apr 28, 2022

I might be wrong, but I think you essentially don't have to setup context.Context ever: as long as you use the interfaces and types that provide you with a context (and yes, will need to move from MigrateState to StateUpgraders), you can then pass that context to tflog.* calls.

Additionally, I'm planning to make use of the "subsystem facility" like here https://pkg.go.dev/github.com/hashicorp/terraform-plugin-log/tflog#SubsystemDebug - for example, each resource could be considered a subsystem.

@bendbennett
Copy link
Contributor Author

Thanks for the link to subsystem logging.

In terms of switching to StateUpgraders, the issue is that the docs state that "Existing MigrateState implementations should remain for compatibility with existing state."

@detro
Copy link
Contributor

detro commented Apr 28, 2022

Ah crap :(

Then let's see what @bflad thinks about the issue you highlighted above.

@bendbennett
Copy link
Contributor Author

Closing as:

  • The state migration will be going away when the random provider is migrated from sdk/v2 to the framework.
  • The old MigrateState functions do not have the appropriate logging context available from the sdk.
  • tflog will drop logs that don’t have an appropriate logging context until Fallback to Generic Logger Output is done.

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.

Adopt tflog
2 participants