jaeger_mcp: trace tool invocations via MCP middleware (otelhttp for transport)#8160
jaeger_mcp: trace tool invocations via MCP middleware (otelhttp for transport)#8160SoumyaRaikwar wants to merge 15 commits intojaegertracing:mainfrom
Conversation
Signed-off-by: SoumyaRaikwar <somuraik@gmail.com>
|
@yurishkuro please take a look whenever you have chance. |
CI Summary ReportMetrics Comparison❌ 36 metric change(s) detected View changed metricsmetrics_snapshot_cassandras_4.x_v004_e2e_auto metrics_snapshot_cassandras_4.x_v004_e2e_manual metrics_snapshot_cassandras_5.x_v004_e2e_auto metrics_snapshot_cassandras_5.x_v004_e2e_manual metrics_snapshot_elasticsearch_9.x_e2e
Code Coverage✅ Coverage 96.8% (baseline 96.8%) ➡️ View CI run | View publish logs |
Signed-off-by: SoumyaRaikwar <somuraik@gmail.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8160 +/- ##
==========================================
+ Coverage 95.63% 95.66% +0.03%
==========================================
Files 319 319
Lines 16793 16892 +99
==========================================
+ Hits 16060 16160 +100
+ Misses 579 578 -1
Partials 154 154
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
jkowall
added
the
changelog:experimental
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
All of this can be achieved with standard OTEL instrumentation for HTTP, why do we need any of this custom code? |
github-actions
bot
added
the
waiting-for-author
@yurishkuro that is correct and i have updated:
I also added low-cardinality
|
github-actions
bot
removed
the
waiting-for-author
…l semantics Signed-off-by: SoumyaRaikwar <somuraik@gmail.com>
SoumyaRaikwar
changed the title
|
I also updated the pr title and description to reflect these changes |
Signed-off-by: SoumyaRaikwar <somuraik@gmail.com>
|
I prefer not to collect metrics, but collect traces that capture tool usage. If someone wants metrics they can then transform traces to metrics using OTEL processors. If you refactor to collect traces please include screenshots of how they would look in Jaeger. |
github-actions
bot
added
the
waiting-for-author
SoumyaRaikwar
changed the title
…aegermcp-observability # Conflicts: # cmd/jaeger/internal/extension/jaegermcp/server.go
Signed-off-by: SoumyaRaikwar <somuraik@gmail.com>
Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com> Signed-off-by: Soumya Raikwar <164396577+SoumyaRaikwar@users.noreply.github.com>
Signed-off-by: SoumyaRaikwar <somuraik@gmail.com>
| } | ||
| } | ||
|
|
||
| func instrumentTool[In, Out any]( |
There was a problem hiding this comment.
why do we need to use decorator when the MCP SDK already supports mcp.Middleware, which we already use for logging?
There was a problem hiding this comment.
I will switch this from per-tool decorator wrapping to MCP middleware so we use one consistent mechanism with the existing logging middleware. The middleware will create spans for tools/call, extract params.name as mcp.tool.name, and set normalized mcp.status. I will remove the instrumentTool decorator path from registration, does this sound good?
There was a problem hiding this comment.
yes and move tracing decorator and logging decorator into the same new middleware.go file (I already posted this earlier #8160 (comment))
There was a problem hiding this comment.
Switched from per-tool decorators to MCP middleware and merged logging + tracing into a single middleware.go, as requested. Tool spans are now created only in the middleware on tools/call, with mcp.tool.name and mcp.status attributes, and failures set span status/error. No custom transport metrics. I have verified live traces locally: you can see mcp.tool.* spans under the jaeger_mcp HTTP span. Screenshot attached.
Signed-off-by: SoumyaRaikwar <somuraik@gmail.com>
Signed-off-by: SoumyaRaikwar <somuraik@gmail.com>
SoumyaRaikwar
changed the title
Signed-off-by: SoumyaRaikwar <somuraik@gmail.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: SoumyaRaikwar <somuraik@gmail.com>
Which problem is this PR solving?
jaeger_mcpobservability with maintainer guidance: standardotelhttphandles transport-level observability on/mcp, while MCP-specific semantics remain in a single MCP middleware.Description of the changes
/mcptransport observability on standardotelhttp.NewHandler(...)inserver.go.middleware.go(no per-tool decorators).tools/call:mcp.tool.<toolName>mcp.tool.name,mcp.statusokinvalid_argumentnot_founderrorHow was this change tested?
make fmtmake lintmake testManual runtime verification with real MCP calls:
initializetools/call→get_servicestools/call→get_span_namestools/call→search_tracestools/call→get_trace_topologywith invalid trace idVerified in Jaeger UI that traces include:
jaeger_mcpmcp.tool.get_services,mcp.tool.get_span_names,mcp.tool.search_traces,mcp.tool.get_trace_topologyChecklist
make lint testAI Usage in this PR (choose one)