fix(telemetry): a held-open MCP session is not a request

coilyco-ops commented

2026-08-13 16:27:00 +00:00

Member

closes #560 - percentiles group on the span name, so two long sessions set a whole service's HTTP POST p99. Only the MCP transport is renamed.

coilyco-ops added 1 commit

2026-08-13 16:27:00 +00:00

fix(telemetry): a held-open MCP session is not a request

ci / test (pull_request) Successful in 34s

Details

ci / publish-echo-image (pull_request) Has been skipped

Details

ci / publish-observed (pull_request) Has been skipped

Details

ci / image-build (pull_request) Successful in 21s

Details

46ac819f0e

Two 180.000s spans set the service's HTTP POST p99, because otelhttp names
every client span for its method and a connection that exists to stay open is
indistinguishable in aggregate from a request that should return in
milliseconds.

Percentiles group on the span name, which is why the attributes added for
sirens-echo#139 do not fix this. That distinction was called out on 560 before
either landed, and it held: 567 adds a parent span and the HTTP POST children
keep their name.

The MCP transport's spans are now named mcp.session. Only that transport. Every
other HTTP POST span is untouched, so a dashboard grouping on HTTP POST keeps
working and stops including connection lifetime, which is the acceptance.

Renaming rather than reparenting, deliberately. The spans stay roots. The
provider dials on a background context so a connection outlives the turn that
opened it, which is the property connection reuse exists for, and 560's
acceptance says so.

Only buildable after pull request 558, which gave the MCP transport its own
client. Before that this would have renamed every model call too.

One thing to expect rather than read as a regression: 558 removed the
whole-request timeout, so sessions no longer die at 180s and will be longer and
more numerous. That is the fix working, and it is the reason they needed their
own name first.

The doc crossed its cap, so the call and transport half is now its own file.
That is the sixth this session and it is tracked on sirens-echo#527.

closes #560

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Kai Siren <coilysiren@gmail.com>
Co-authored-by: Claude <noreply@anthropic.com>

coilyco-ops requested review from coilysiren

2026-08-13 16:27:00 +00:00

coilyco-ops referenced this pull request

2026-08-13 16:27:20 +00:00

A long-lived MCP session is counted in HTTP POST percentiles, so a connection's lifetime reads as request latency #560

coilyco-ops commented

2026-08-13 16:52:20 +00:00

Author

Member

Closing this as superseded. Angie (ENG), and it was my pull request.

6ff5181 landed on main four minutes before I pushed this, fixing the same defect. This went unmergeable against it, which is how I found out.

Theirs is better, and it is not close

// main, 6ff5181
func mcpSessionSpanName(_ string, request *http.Request) string {
	return "mcp.session " + request.Method
}

// this pull request
otelhttp.WithSpanNameFormatter(func(_ string, _ *http.Request) string {
	return mcpSessionSpan
})

I dropped the verb. They prefixed rather than replaced, so a reader still sees the method and a single mcp.session filter still catches every verb at once. Mine would have made GET and POST on a session indistinguishable in exchange for nothing. Both satisfy the acceptance on #560 and only one of them keeps information.

What was in here that is not on main

The code half is gone. Two doc changes are not, and they are unrelated to which formatter won:

docs/sirens-echo-mcp-call-telemetry.md, a new file
the corresponding trim of docs/sirens-echo-tool-discovery-telemetry.md, which had crossed the 80 line cap

I will re-land that split on its own, after #567 lands, because 567 adds to the same file and sequencing them the other way just makes a second conflict. The prose will describe mcp.session POST rather than mcp.session, since that is what actually shipped.

The avoidable part

Two seats built the same 7 line change inside one hour. I checked #560 before starting and it was open and unclaimed, so the claim protocol did what it was supposed to and still did not prevent this. That is the collision shape already filed on #552 and I am adding this instance there rather than opening a new one.

**Closing this as superseded. Angie (ENG), and it was my pull request.** `6ff5181` landed on `main` four minutes before I pushed this, fixing the same defect. This went unmergeable against it, which is how I found out. ## Theirs is better, and it is not close ```go // main, 6ff5181 func mcpSessionSpanName(_ string, request *http.Request) string { return "mcp.session " + request.Method } // this pull request otelhttp.WithSpanNameFormatter(func(_ string, _ *http.Request) string { return mcpSessionSpan }) ``` I dropped the verb. They prefixed rather than replaced, so a reader still sees the method and a single `mcp.session` filter still catches every verb at once. Mine would have made `GET` and `POST` on a session indistinguishable in exchange for nothing. Both satisfy the acceptance on https://forgejo.coilysiren.me/coilyco-gaming/sirens-echo/issues/560 and only one of them keeps information. ## What was in here that is not on main The code half is gone. Two doc changes are not, and they are unrelated to which formatter won: - `docs/sirens-echo-mcp-call-telemetry.md`, a new file - the corresponding trim of `docs/sirens-echo-tool-discovery-telemetry.md`, which had crossed the 80 line cap I will re-land that split on its own, after https://forgejo.coilysiren.me/coilyco-gaming/sirens-echo/pulls/567 lands, because 567 adds to the same file and sequencing them the other way just makes a second conflict. The prose will describe `mcp.session POST` rather than `mcp.session`, since that is what actually shipped. ## The avoidable part Two seats built the same 7 line change inside one hour. I checked https://forgejo.coilysiren.me/coilyco-gaming/sirens-echo/issues/560 before starting and it was open and unclaimed, so the claim protocol did what it was supposed to and still did not prevent this. That is the collision shape already filed on https://forgejo.coilysiren.me/coilyco-gaming/sirens-echo/issues/552 and I am adding this instance there rather than opening a new one.

coilyco-ops closed this pull request

2026-08-13 16:52:44 +00:00

coilyco-ops referenced this pull request

2026-08-13 17:10:43 +00:00

@mentions in replies should surface the replied-to message as primary subject, not channel head #579

coilyco-ops referenced this pull request

2026-08-13 18:00:42 +00:00

feat(telemetry): name the server and stage a discovery round trip was in #567

coilyco-ops referenced this pull request

2026-08-13 19:03:07 +00:00

Seven duplicate builds from one seat in ninety minutes, and the cheapest guard is one fetch before the first edit #690