feat(provisioner): propagate trace info #17166
Conversation
|
Thanks for the PR! Can you say a little more about the use case for this change? It appears that |
Correct, terraform itself won't use it, but it will maintain it in the environment for the provider binaries it calls. We (Netflix) have a custom provider to implement our deployment strategies and would like the ability to continue traces that were started in Coder or further upstream. For example, our provider Create and Update methods will just call: Then any new spans created will use the inherited span context. The primary use case is to enhance our integration testing. In our tests, we start a trace and can see the spans between our test into Coder where it invokes terraform. Currently, in our terraform provider we have to start a new trace, where we collect spans through to workspace startup. With this patch, we will be able to connect our traces from the test entirely through to workspace startup, which should help with visibility/debugging. |
|
That makes sense! I have to say, from a supply-chain security standpoint, we are very reluctant to add a dependency on The other thing I'd ask for is a unit test of the propagation. |
Thanks, works for me, I will update. You should be able to swap out the implementation with an official OTEL support once they get that sorted. |
1b211bd to
4459db5
Compare
|
@spikecurtis copied the required code to inject the trace env, and made all the functions unexported. Also added a test to ensure the TRACEPARENT env var is set/updated. |
|
updated for lint rules... |
If tracing is enabled, propagate the trace information to the terraform provisioner via environment variables. This sets the `TRACEPARENT` environment variable using the default W3C trace propagators. Users can choose to continue the trace by adding new spans in the provisioner by reading from the environment like: ctx := env.ContextWithRemoteSpanContext(context.Background(), os.Environ())
provisioner/terraform/otelenv.go
Outdated
| // don't directly update the slice so we don't modify the slice | ||
| // passed in | ||
| newEnv := slices.Clone(c.Env) | ||
| newEnv = append(newEnv[:i], append([]string{fmt.Sprintf("%s=%s", key, value)}, newEnv[i+1:]...)...) |
There was a problem hiding this comment.
Once you've cloned the initial slice, seems like it would be simpler to just modify in place, rather than re-slice and append.
There was a problem hiding this comment.
hah, I think that code went through too many refactors and came out nonsense. I have simplified it.
| ) | ||
|
|
||
| // TODO: replace this with the upstream OTEL env propagation when it is | ||
| // released. |
| @@ -0,0 +1,63 @@ | |||
| package terraform // nolint:testpackage | |||
There was a problem hiding this comment.
file should be named otelenv_internal_test.go then you don't need this nolint
Co-authored-by: Spike Curtis <spike@spikecurtis.com>
Co-authored-by: Spike Curtis <spike@spikecurtis.com>
Simplified the slice copy/update logic. Removed the panics for non required interface functions, the impl was trivial, added simple tests to ensure they work as expected.
If tracing is enabled, propagate the trace information to the terraform provisioner via environment variables. This sets the
TRACEPARENTenvironment variable using the default W3C trace propagators. Users can choose to continue the trace by adding new spans in the provisioner by reading from the environment like: