Build in log sink functionality. #9
Conversation
paddycarver
force-pushed
the
paddy_sink
branch
from
3f47346
to
d69cf77
Compare
|
In case these notes are helpful, here's how I'm thinking about the flow of logs between processes and components, and what environment variables get applied where.
|
paddycarver
force-pushed
the
paddy_sink
branch
from
7d5df3f
to
a35183b
Compare
|
Rebased on #10. |
|
Will review this when #10 is merged and this is rebased 😄 |
paddycarver
force-pushed
the
paddy_sink
branch
3 times, most recently
from
71dca88
to
76e1d83
Compare
Update breaking changes and refactoring so we're ready for logging sinks.
Starting to address #6. This is just the start of the work, but it gives us something to build on. Basically, what we're trying to do is build log sink functionality into terraform-plugin-log. Normally, Terraform acts as a log sink, gathering up logs from core and from all the plugins, filtering them appropriately, and combining them into a single log output stream, and deciding where to send that stream. However, when running acceptance tests, the plugin is actually the longest-running process, and so this model breaks down. Instead, we need to have the plugin act as the sink, gathering the logs from core and the plugin and other plugins, combining them into a single stream, filtering the stream, and deciding where to output the stream. Rather than implement this functionality multiple times in different SDKs, it makes more sense to add it to the tfsdklog package and just call it from the test frameworks. There are a few parts to this. First, we need a new sink logger that all other loggers are derived from. It should read environment variables to determine what level of output is desired, what format of output is desired, and where to send that output. Second, we should make all our loggers be sub-loggers of that sink when the sink is set. Third, we should make sure our sub-loggers can only log at levels equal to or higher than the level of this new sink logger when it's set. And finally, we should provide functionality to process hclog JSON output (like from terraform when TF_LOG=json) and route it through this sink logger, so we can combine the log output of the CLI and other providers with the log output native to the process. This commit falls short in a few places: * sub-loggers aren't limited to equal-or-higher levels from the sink logger yet. * none of this has been tested, even manually. It builds? * we need to check that all the environment variables we need to be honoring are getting honored for the provider under test, the CLI, and the providers the CLI launches. This depends on #10.
paddycarver
force-pushed
the
paddy_sink
branch
from
76e1d83
to
d7f5936
Compare
There was a problem hiding this comment.
Overall this is shaping up well, very exciting. Great work. Just some code-level questions/comments. Since we need to manually test this anyways, I wonder if it is worth dropping any sort of documentation/checklist (README.md or whatever) about that process until #11 can be a focus.
| if err != nil && tc.expectedErr != nil && err.Error() != tc.expectedErr.Error() { | ||
| t.Fatalf("expected error to be %v, got %v", tc.expectedErr, err) | ||
| } else if err == nil && tc.expectedErr != nil { | ||
| t.Fatalf("expected error to be %v, didn't get an error", tc.expectedErr) | ||
| } else if err != nil && tc.expectedErr == nil { | ||
| t.Fatalf("unexpected error %v", err) | ||
| } |
There was a problem hiding this comment.
Aside: I think using go-cmp's Diff works well for this scenario as well.
There was a problem hiding this comment.
It doesn't let me cheat by using errors.New or fmt.Errorf, I'd need to use the right types of errors, too.
Update breaking changes and refactoring so we're ready for logging sinks.
|
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. |
Starting to address #6. This is just the start of the work, but it gives
us something to build on.
Basically, what we're trying to do is build log sink functionality into
terraform-plugin-log. Normally, Terraform acts as a log sink, gathering
up logs from core and from all the plugins, filtering them
appropriately, and combining them into a single log output stream, and
deciding where to send that stream. However, when running acceptance
tests, the plugin is actually the longest-running process, and so this
model breaks down. Instead, we need to have the plugin act as the sink,
gathering the logs from core and the plugin and other plugins, combining
them into a single stream, filtering the stream, and deciding where to
output the stream.
Rather than implement this functionality multiple times in different
SDKs, it makes more sense to add it to the tfsdklog package and just
call it from the test frameworks.
There are a few parts to this. First, we need a new sink logger that all
other loggers are derived from. It should read environment variables to
determine what level of output is desired, what format of output is
desired, and where to send that output. Second, we should make all our
loggers be sub-loggers of that sink when the sink is set. Third, we
should make sure our sub-loggers can only log at levels equal to or
higher than the level of this new sink logger when it's set. And
finally, we should provide functionality to process hclog JSON output
(like from terraform when TF_LOG=json) and route it through this sink
logger, so we can combine the log output of the CLI and other providers
with the log output native to the process.
This commit falls short in a few places:
logger yet.
package. This likely involves moving the sink functionality into an
internal package.
honoring are getting honored for the provider under test, the CLI, and
the providers the CLI launches.