Skip to content

fix(otel): restore distributed tracing for the controller and agent runtimes - #2663

Merged
EItanya merged 6 commits into
kagent-dev:mainfrom
codyhartsook:address-tracing-regression-oss
Sep 2, 2026
Merged

EItanya merged 6 commits into
kagent-dev:mainfrom
codyhartsook:address-tracing-regression-oss

Conversation

@codyhartsook

@codyhartsook codyhartsook commented Sep 1, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Restores distributed tracing for the v2 controller and the established kagent runtime.

  • Initialize the controller tracer provider before constructing the instrumented gRPC server.
  • Forward controller OTEL_* variables into kagent runtime revisions; tracing configuration therefore participates in the revision digest.
  • Add focused coverage for tracer registration and OTEL environment forwarding.
jaeger-trace

Verification

  • go test ./core/...
  • make -C go lint
  • Focused tests: go test ./core/internal/telemetry ./core/v2/translator ./core/v2/translator/kagent ./core/v2/translator/claude
  • Kind smoke test: a kagent-harness invocation produces one connected Jaeger trace from kagent-controller into 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.

@github-actions github-actions Bot added bug Something isn't working and removed bug Something isn't working labels Sep 1, 2026
@codyhartsook
codyhartsook marked this pull request as ready for review September 1, 2026 23:22
@codyhartsook
codyhartsook requested a review from a team as a code owner September 1, 2026 23:22

@krisztianfekete krisztianfekete left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is looking very good, added some comments on potentially improving what we had before.

Comment on lines +82 to +83
// otelgrpc snapshots the global TracerProvider and propagator when its handler
// is constructed, so tracing has to be registered before any server is built.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Great thing to call out, thanks!

Comment on lines +14 to +30
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
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

good idea, added the allow list.

Comment on lines +46 to +47
// "none" selects a noop exporter, so the test dials no collector.
t.Setenv("OTEL_TRACES_EXPORTER", "none")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice!

@codyhartsook
codyhartsook force-pushed the address-tracing-regression-oss branch from 8a538ce to 5aad890 Compare September 2, 2026 15:01
@EItanya
EItanya enabled auto-merge (squash) September 2, 2026 16:17
@EItanya
EItanya merged commit 19eafc0 into kagent-dev:main Sep 2, 2026
25 checks passed
pull Bot pushed a commit to TheTechOddBug/kagent that referenced this pull request Sep 16, 2026
…v#2805)

Adds correlated OpenTelemetry tracing and logging across the Kagent,
Codex, and Claude harnesses. Refactors shared tracing configuration and
setup code. Builds on top of previous V2 observability works like kagent-dev#2663
and kagent-dev#2786. Also closes kagent-dev#2760 and partially addresses kagent-dev#2762.

### Key Changes

- Resolve and validate OTLP trace and log settings consistently across
all harness compilers.
- Propagate telemetry configuration, service identity, and collector
egress into runtime revisions.
- Configure Codex and Claude native trace and log exporters with content
capture enabled.
- Propagate W3C trace context through Codex JSON-RPC requests and Claude
process environment.
- Initialize shared Go tracing in the native harness wrappers and move
common tracing code out of the ADK package.
- Flush request spans before terminal and input-required events cause an
Actor to pause.

### Notes

- Traces and logs are independently enabled through
`OTEL_TRACING_ENABLED` and `OTEL_LOGGING_ENABLED`
- Invalid or missing enabled-signal endpoints and unsupported protocols
now fail harness compilation.
- Span attributes for Claude Code is [documented
here](https://code.claude.com/docs/en/monitoring-usage#span-attributes)
and available events / logs is [documented
here](https://code.claude.com/docs/en/monitoring-usage#events)
- Events / logs emitted by Codex is [documented
here](https://learn.chatgpt.com/docs/config-file/config-advanced#what-gets-emitted).
There is no official docs for Codex's span hierarchy and attributes
- Codex and Claude Code force flushes traces when their process exists
before the gateway Suspends the instance. The adapter process will also
flush its own traces before it responds with a terminal event as per the
changes in `go/adk/pkg/a2a/server/server.go`. However, during a pending
HITL, since we do not quit the Codex or Claude process, it is not
guaranteed that all traces are delivered immediately.
- For both Codex and Claude, native spans after HITL completes remain in
the original request trace because the in-flight process carries the
same trace context (through env var or JSON-RPC). The resume A2A trace
may contain only controller and Go wrapper spans.

---------

Signed-off-by: Jet Chiang <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants