Skip to content

Fix tool counter per-request creation, omitempty config, and streamable-http version#3761

Merged
ChrisJBurns merged 4 commits intomainfrom
otel/code-quality-b3-b6-b10-t1
Feb 11, 2026
Merged

Fix tool counter per-request creation, omitempty config, and streamable-http version#3761
ChrisJBurns merged 4 commits intomainfrom
otel/code-quality-b3-b6-b10-t1

Conversation

@ChrisJBurns
Copy link
Copy Markdown
Collaborator

@ChrisJBurns ChrisJBurns commented Feb 10, 2026

Summary

  • 1: Remove omitempty from TracingEnabled/MetricsEnabled struct tags so YAML false values are not silently dropped during serialization (same class of bug as fixed in Fix SSE gauge leak, config bool handling, and metric error logging #3756)
  • 2: Move toolhive_mcp_tool_calls counter from per-request creation to struct initialization in NewHTTPMiddleware, consistent with all other counters
  • 3: Return empty string for streamable-http protocol version in mapTransport since the actual backend HTTP version is unknown (was hardcoded to "2")
  • 4: Add TestRecordSSEConnection_DualEmission test verifying recordSSEConnection emits both new OTEL semconv and legacy span attributes based on UseLegacyAttributes flag

Parent issue: #3399

Test plan

  • task test — all telemetry and vMCP tests pass
  • task lint — 0 issues
  • New T1 test covers both legacy-enabled and legacy-disabled SSE paths

🤖 Generated with Claude Code

ChrisJBurns and others added 3 commits February 10, 2026 22:53
- Fix SSE active connections gauge that was incremented but never
  decremented. Move increment into Handler with a defer decrement so
  the gauge correctly tracks SSE connection lifecycle.

- Fix config file `use-legacy-attributes: false` being silently ignored.
  Change UseLegacyAttributes from bool to *bool in app config so nil
  (not set) is distinguishable from explicit false. Add two test cases
  covering both scenarios.

- Log metric creation errors at debug level instead of silently
  discarding them. The OTEL SDK returns no-op instruments on error so
  there is no runtime risk, but logging aids troubleshooting.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ng, and client span kind

- Add mcp.protocol.version span attribute by reading the
  MCP-Protocol-Version header from streamable HTTP requests.

- Add mcp.client.operation.duration histogram metric to vMCP backend
  client using the OTEL MCP semconv bucket boundaries, alongside the
  existing toolhive_vmcp_backend_requests_duration custom metric.

- Include target name in span names per OTEL MCP semconv. Span names
  now follow the "{method} {target}" format, e.g. "tools/call
  github_search" instead of just "tools/call".

- Set SpanKindClient on vMCP backend call spans for proper distributed
  trace linking.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…le-http version

- B3: Remove omitempty from TracingEnabled/MetricsEnabled struct tags
  so YAML `false` values are not silently dropped during serialization
- B6: Move tool call counter from per-request creation to struct
  initialization in NewHTTPMiddleware for consistency with other counters
- B10: Return empty string for streamable-http protocol version in
  mapTransport since the actual backend version is unknown
- T1: Add SSE dual emission test verifying recordSSEConnection emits
  both new and legacy span attributes based on UseLegacyAttributes flag

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@github-actions github-actions bot added the size/S Small PR: 100-299 lines changed label Feb 10, 2026
@ChrisJBurns ChrisJBurns changed the base branch from otel/spec-compliance-s2-s4-s5-s6 to main February 10, 2026 23:37
@ChrisJBurns ChrisJBurns changed the base branch from main to otel/spec-compliance-s2-s4-s5-s6 February 10, 2026 23:38
rdimitrov
rdimitrov previously approved these changes Feb 10, 2026
@codecov
Copy link
Copy Markdown

codecov bot commented Feb 10, 2026

Codecov Report

❌ Patch coverage is 77.77778% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 66.49%. Comparing base (5e2fa08) to head (a33f78e).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
pkg/telemetry/middleware.go 77.77% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3761      +/-   ##
==========================================
+ Coverage   66.47%   66.49%   +0.02%     
==========================================
  Files         428      428              
  Lines       42084    42087       +3     
==========================================
+ Hits        27974    27987      +13     
+ Misses      11979    11963      -16     
- Partials     2131     2137       +6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions github-actions bot added size/S Small PR: 100-299 lines changed and removed size/S Small PR: 100-299 lines changed labels Feb 10, 2026
@ChrisJBurns ChrisJBurns force-pushed the otel/code-quality-b3-b6-b10-t1 branch from 25145e3 to 250103c Compare February 10, 2026 23:50
@github-actions github-actions bot added size/S Small PR: 100-299 lines changed and removed size/S Small PR: 100-299 lines changed labels Feb 10, 2026
Base automatically changed from otel/spec-compliance-s2-s4-s5-s6 to main February 10, 2026 23:54
@ChrisJBurns ChrisJBurns dismissed rdimitrov’s stale review February 10, 2026 23:54

The base branch was changed.

@github-actions github-actions bot added size/S Small PR: 100-299 lines changed and removed size/S Small PR: 100-299 lines changed labels Feb 10, 2026
@ChrisJBurns ChrisJBurns merged commit 0696351 into main Feb 11, 2026
41 of 46 checks passed
@ChrisJBurns ChrisJBurns deleted the otel/code-quality-b3-b6-b10-t1 branch February 11, 2026 00:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/S Small PR: 100-299 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants