feat: Trace weather agent to MCP gateway tool calls#145
feat: Trace weather agent to MCP gateway tool calls#145evaline-ju wants to merge 3 commits intokagenti:mainfrom
Conversation
Signed-off-by: Evaline Ju <69598118+evaline-ju@users.noreply.github.com>
Signed-off-by: Evaline Ju <69598118+evaline-ju@users.noreply.github.com>
evaline-ju
changed the title
Assisted-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Evaline Ju <69598118+evaline-ju@users.noreply.github.com>
evaline-ju
force-pushed
the
weather-trace-langchain
branch
from
0839ef2 to
0e77b2f
Compare
mrsabath
left a comment
mrsabath
left a comment
There was a problem hiding this comment.
Review Summary
Clean, focused PR that adds end-to-end distributed tracing from the weather agent through to MCP gateway tool calls. Two key changes: (1) adds opentelemetry-instrumentation-httpx for automatic traceparent propagation on outgoing HTTP requests, and (2) switches the tracing middleware from creating root spans to joining incoming W3C trace context. Both changes are correct for the distributed tracing use case.
Areas reviewed: Python, Dependencies, Security
Commits: 3 commits, all signed-off: yes
CI status: DCO pass, build pass
🤖 Reviewed with Claude Code
| logger.info("httpx instrumented for automatic trace context propagation") | ||
| except ImportError: | ||
| logger.warning("opentelemetry-instrumentation-httpx not available - " | ||
| "MCP tool calls will not propagate trace context") |
There was a problem hiding this comment.
praise: Good approach — instrumenting httpx at the SDK level means all outgoing HTTP calls (including MCP tool calls via langchain-mcp-adapters' streamable_http transport) automatically propagate traceparent. No per-call instrumentation needed, and the graceful ImportError fallback is clean.
| # Without this, the span would inherit parent from W3C Trace Context headers | ||
| empty_ctx = context.Context() | ||
| detach_token = context.attach(empty_ctx) | ||
| # Extract incoming W3C Trace Context from request headers to connect |
There was a problem hiding this comment.
suggestion: The switch from context.Context() (empty/root span) to extract(dict(request.headers)) (join caller's trace) is a significant behavioral change. Previously every inbound request created an isolated root span; now it joins the caller's trace when a traceparent header is present. Consider adding a brief comment noting that callers without traceparent still get root spans (extract returns empty context), while callers with traceparent (like MCP gateway via AuthBridge) get connected end-to-end traces.
|
|
||
| def get_mcpclient(): | ||
| """Create an MCP client. | ||
|
|
There was a problem hiding this comment.
nit: The docstring focuses on trace context propagation (which is great documentation), but the opening line "Create an MCP client" could be slightly more specific — e.g., "Create MCP client configured with math and weather tool servers."
3 participants
Draft since the MCP gateway otel changes are not in a release yet (on mainline post 0.5)
Summary
For the weather agent, be able to trace agent calls to tool calls through Envoy+MCP gateway, all linked to the same root span.
Related issue(s)
For tracing from agent->MCP gateway for kagenti/kagenti#619