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

Terraform vs. Rspec logger isolation #46

Open
plukevdh opened this issue Jan 19, 2023 · 2 comments
Open

Terraform vs. Rspec logger isolation #46

plukevdh opened this issue Jan 19, 2023 · 2 comments

Comments

@plukevdh
Copy link

Hi there! Thanks for this project. We found your RubyTerraform project and used it to write an rspec-terraform plugin... until we realized you all had done the same thing (and better).

Some usage feedback: One of the things it'd be nice to do is have the option to silence the terraform plan output. To do so, I can do

RSpec.configure do |config|
    config.terraform_stdout = Logger::LogDevice.new(IO::NULL)
end

However this appears to have the unintended side-effect of also silencing the rspec-terraform plugin's logging output. I assume this is because the logger gets reused between the RubyTerraform and RSpec::Terraform. It seems like it might be worthwhile to isolate the loggers so that we can toggle them/redirect them independently of one another.

Perhaps alternatively (or in addition to), it might be nice to have an rspec option flag called silence_terraform_output that would redirect the terraform output to IO::NULL.

I'm happy to submit a PR implementing either/both of these options if you all think this would be valuable. Thoughts?

@tobyclemson
Copy link
Member

tobyclemson commented Jan 19, 2023

Hi @plukevdh,

We're glad you found it as we haven't even finished documenting / officially released it yet! I hope you are finding it useful and would be very open to further feedback in terms of how to improve. I'm not sure whether you are already aware but all the InfraBlocks terraform modules are already using rspec-terraform for their testing, both at unit and integration level so if you need any inspiration, they are worth a look.

In terms of your specific issue, the logging/output configuration probably needs some more work as it's a bit over complex. However, as it stands, there are 3 different outputs at play:

  • rspec-terraform's log output
  • ruby_terraform's log output
  • The standard output produced by Terraform when it is invoked

ruby_terraform allows its logger to be configured (in order to control the libraries log output) and also allows the stdout and stderr streams that get passed to shell invocations to be configured (in order to control Terraform's output). Then, wrapping that within rspec-terraform, we log additional lifecycle information throughout test execution.

Right now, rspec-terraform exposes a few different configuration options for managing all this output:

  • terraform_log_file_path: if file logging is enabled, this is the path where the file will be stored, required when file logging is enabled.
  • terraform_log_level: the level of logs to output, shared for rspec-terraform logs and ruby-terraform logs, defaulting to INFO.
  • terraform_log_streams: where to log to, zero or more of [:file, :standard] allowing logging to be output in the terminal or to a file, defaulting to :standard.
  • terraform_logger: the IO device to use for logging, used for both rspec-terraform logs and ruby-terraform logs (but not for Terraform output, see terraform_stdout), defaulting to a log device that logs to the provided terraform_stdout.
  • terraform_stdout: the IO device to use as the stdout stream provided to the Terraform commands invoked via a shell
  • terraform_stderr: the IO device to use as the stderr stream provided to the Terraform commands invoked via a shell

As emboldened above, I believe the problem you are facing is that because you are overwriting terraform_stdout to be a null device, the default terraform_logger is also redirecting to that null device. If you were to additionally specify terraform_logger as a logger sending its output to $stdout you should resume capturing both rspec-terraform logging and the (small amount of) ruby_terraform logging, e.g.:

RSpec.configure do |config|
    config.terraform_logger = Logger::LogDevice.new($stdout)
    config.terraform_stdout = Logger::LogDevice.new(IO::NULL)
end

I do like the idea of separating the IO devices used for rspec-terraform and ruby_terraform but I don't believe it will solve your problem and since ruby_terraforms logging is not particularly noisy, it might be fine to leave that outputting to the same place as rspec-terraform and instead just silence Terraform's standard output.

Let me know how you get on and as I say, feedback very welcome,
Thanks,
Toby

@plukevdh
Copy link
Author

plukevdh commented Jan 19, 2023

@tobyclemson Great feedback. I'd not associated the split-ability in the logger functionality with the stdout configuration. I missed that after I saw the default association. Also, super helpful delineation on what the three logging output components/concerns are. I'm actually not so concerned to split ruby_terraform the gem output and rspec-terraform. What I meant by ruby_terraform was actually the stdout output from terraform itself. Your solution handles that really well (though I think you want to assign a Logger.new($stdout) to terraform_logger rather than the LogDevice).

With that said, it might be helpful to delineate which of the output options is associated with what component. stdout to me is just any output to the console, not specifically just the output of the terraform runtime. Maybe rspec_terraform_logger vs. terraform_logger? Not sure.

In the end, I think it would be clearer to make the options configurable based on behavior (do I want terraform plan output to the console, do I want info/debug logging from the underlying Rspec plugin) rather than the underlying system concerns (what are my loggers, stdout, etc.) So instead of terraform_logger or terraform_stdout/stderr, it seems like it could help to provide a descriptive interface with options like silence_terraform_output = true or debug_terraform_runtime = true.

It is still 💯 worth it to have that stuff configurable in the ruby_terraform library, perhaps less so in the rspec-terraform library.

Definitely take this all as constructive feedback from a very thankful developer. This library is solving a definite pain point and it's super validating to see someone else solving the same pains the way we would.

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

No branches or pull requests

2 participants