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

Upgrade to latest terraform-plugin-go, add logging sink. #805

Merged
merged 1 commit into from
Dec 7, 2021

Conversation

paddycarver
Copy link
Contributor

Upgrade to the latest version of terraform-plugin-go, bringing support
for terraform-plugin-log.

Use the terraform-plugin-log logging sink when setting up the
helper/resource test framework. The idea here is that we'll use the
logging sink to reroute all terraform-plugin-log logs, and leave the
log.Print logs the way they are with the existing override. Over time,
it may be nice to migrate those logs to use terraform-plugin-log, which
we've introduced logging.GetTestLogContext as a helper for. But that
may be a mistake, and maybe we should wait until we need it to introduce
that function.

In theory, this PR should result in no logs being written to the CLI
during testing, which was happening in hashicorp/terraform-plugin-go#93.
It should respect TF_LOG and TF_LOG_PATH, as well as any
terraform-plugin-log style environment variables in use.

I think, by nature of this problem, we're going to need to test it manually for the moment, but I'll leave a comment with information on how to do that.

We should also wait to land this until hashicorp/terraform-plugin-log#15 lands and a release is cut, and hashicorp/terraform-plugin-go#93 lands and a release is cut, which will let us use tagged releases in our go.mod.

@paddycarver paddycarver added the subsystem/tests Issues and feature requests related to the testing framework. label Sep 21, 2021
@paddycarver paddycarver requested a review from a team September 21, 2021 12:18
@bflad bflad self-assigned this Dec 7, 2021
@bflad bflad added this to the v2.10.0 milestone Dec 7, 2021
Copy link
Contributor

@bflad bflad left a comment

Choose a reason for hiding this comment

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

Looks good to me! 🚀 Just two little (optional) things to consider.

helper/logging/logging.go Outdated Show resolved Hide resolved
helper/logging/logging.go Show resolved Hide resolved
Upgrade to the latest version of terraform-plugin-go, bringing support
for terraform-plugin-log.

Use the terraform-plugin-log logging sink when setting up the
helper/resource test framework. The idea here is that we'll use the
logging sink to reroute all terraform-plugin-log logs, and leave the
`log.Print` logs the way they are with the existing override. Over time,
it may be nice to migrate those logs to use terraform-plugin-log,
probably by extending `helper/logging.GetTestLogContext` to include a
root SDK logger.

In theory, this PR should result in no logs being written to the CLI
during testing, which was happening in hashicorp/terraform-plugin-go#93.
It should respect `TF_LOG`, `TF_LOG_PATH`, `TF_ACC_LOG_PATH`, and
`TF_LOG_PATH_MASK`, as well as any terraform-plugin-log style
environment variables in use.

This also fixes `TF_ACC_LOG_PATH` to record the logs from the provider
under test, not just the logs from Terraform and any providers the
provider under test depends on.

Co-authored-by: Brian Flad <bflad417@gmail.com>
@paddycarver paddycarver merged commit 84a8f22 into main Dec 7, 2021
@paddycarver paddycarver deleted the paddy_plugin_log branch December 7, 2021 19:55
@github-actions
Copy link

github-actions bot commented Jan 7, 2022

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 7, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
subsystem/tests Issues and feature requests related to the testing framework.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants