fix(otel): restore distributed tracing for the controller and agent runtimes - #2663
Conversation
krisztianfekete
left a comment
There was a problem hiding this comment.
This is looking very good, added some comments on potentially improving what we had before.
| // otelgrpc snapshots the global TracerProvider and propagator when its handler | ||
| // is constructed, so tracing has to be registered before any server is built. |
There was a problem hiding this comment.
Great thing to call out, thanks!
| func OtelEnvFromProcess() []corev1.EnvVar { | ||
| var envVars []corev1.EnvVar | ||
| for _, envVar := range os.Environ() { | ||
| if !strings.HasPrefix(envVar, "OTEL_") { | ||
| continue | ||
| } | ||
| name, value, found := strings.Cut(envVar, "=") | ||
| if !found { | ||
| continue | ||
| } | ||
| envVars = append(envVars, corev1.EnvVar{Name: name, Value: value}) | ||
| } | ||
| slices.SortFunc(envVars, func(a, b corev1.EnvVar) int { | ||
| return strings.Compare(a.Name, b.Name) | ||
| }) | ||
| return envVars | ||
| } |
There was a problem hiding this comment.
I know that we had a similar logic before it got removed, but wondering if we should go with an explicit allow-list for OTel env vars instead.
What I mainly want to avoid is silently adding potentially unwanted data as plaintext e.g. when someone decides to propagate OTEL_EXPORTER_OTLP_HEADERS.
There might also be a problem if someone is setting a e.g. service.name via an env var as well, as that would falsely label agents after the var is propagated.
There was a problem hiding this comment.
good idea, added the allow list.
| // "none" selects a noop exporter, so the test dials no collector. | ||
| t.Setenv("OTEL_TRACES_EXPORTER", "none") |
Signed-off-by: Cody Hartsook <cody.hartsook@solo.io>
Signed-off-by: Cody Hartsook <cody.hartsook@solo.io>
Signed-off-by: Cody Hartsook <cody.hartsook@solo.io>
Signed-off-by: Cody Hartsook <cody.hartsook@solo.io>
8a538ce to
5aad890
Compare
Signed-off-by: Cody Hartsook <cody.hartsook@solo.io>
Summary
Restores distributed tracing for the v2 controller and the established kagent runtime.
OTEL_*variables into kagent runtime revisions; tracing configuration therefore participates in the revision digest.Verification
go test ./core/...make -C go lintgo test ./core/internal/telemetry ./core/v2/translator ./core/v2/translator/kagent ./core/v2/translator/claudekagent-controllerinto the actor.Deferred
Claude harness tracing needs runtime telemetry initialization and its mock E2E fixture/image from #2602. That work will be handled with the new harness rather than broaden this controller regression fix.