otelrestful: client-initiated request termination is recorded as a 500 with span status Error and no error.type

[danielloader]

Split out from #9358 as requested in #9358 (comment). Same defect class as the otelhttp report, confirmed independently against otelrestful. This package was not named in the original thread, but it is affected for the same reason, so raising it here to keep the set complete.

Symptom

When a client disconnects mid-request, the handler observes the cancelled request context and typically returns an error, which surfaces as a 500. otelrestful then records http.response.status_code=500 and sets the span status to Error, and sets no error.type at all, so a client disconnect is indistinguishable from a genuine server fault.

OTelFilter reads resp.StatusCode() after chain.ProcessFilter and never inspects req.Request.Context().Err():

opentelemetry-go-contrib/instrumentation/github.com/emicklei/go-restful/otelrestful/restful.go

Lines 69 to 75 in 5f30960

	chain.ProcessFilter(req, resp)
	status := resp.StatusCode()
	span.SetStatus(semconvServer.Status(status))
	span.SetAttributes(semconvServer.ResponseTraceAttrs(semconv.ResponseTelemetry{
	StatusCode: status,
	})...)

otelrestful is the thinnest of the server instrumentations: it wraps neither the request body nor the response writer, so there is no observed read or write error to fall back on, and it does not record metrics at all (semconv.NewHTTPServer(nil)), so the metric side of the parent issue does not apply here. The request-context error is the only signal available, and it is not consulted.

Reproduction

Confirmed on main at 5f30960 with a real HTTP/1.1 disconnect. The handler waits on req.Request.Context().Done() and then writes 500:

container.Filter(otelrestful.OTelFilter("srv", otelrestful.WithTracerProvider(tp)))
ws.Route(ws.GET("/hello").To(func(req *restful.Request, resp *restful.Response) {
    <-req.Request.Context().Done()
    resp.WriteHeader(http.StatusInternalServerError)
    _, _ = resp.Write([]byte("cancelled"))
}))

The client issues the request with a cancellable context and cancels it after 100ms. Recorded span:

span name   = "/hello"
span status = Error ""
attr http.route                 = /hello
attr http.response.status_code  = 500

No error.type. Identical outcome to the otelhttp reproduction in #9358.

Suggested fix

Mirroring the triage in #9358 (comment), reduced to what applies here:

1. Prefer the request-context error as the failure cause over the bare status code.
2. Set span status to Error and add a predictable, low-cardinality error.type for that cause.
3. Preserve the observed/selected HTTP status code; for an error status with no more specific cause, use the status-code string as error.type.
4. Add a real disconnect regression test so cancellation is detected before the filter returns.

Whether otelrestful should also start wrapping the body and response writer to observe read/write errors — and whether it should record metrics at all — feels like separate scope; happy to raise that separately if it is wanted.

Note on shared code

otelrestful/internal/semconv/server.go is generated from internal/shared/semconv/server.go.tmpl and is byte-identical to the copy vendored into otelhttp (verified by diff). The same is true for otelgin, otelmux and otelecho. So whatever error.type classification lands in the shared template is picked up by all of them, but each middleware still needs its own wiring to detect and pass the failure cause, plus its own regression test.

Relevant requirements: HTTP span conventions and the dropped-connection example.

[github-actions]

Pinging code owners for instrumentation: otelrestful: @dashpole.

[MrAlias]

Confirmed bug. I independently reproduced this with a synchronized real HTTP/1.1 disconnect: after the client cancelled, the handler observed req.Request.Context().Done(), selected 500, and the ended server span contained http.response.status_code=500 with status Error but no error.type.

The current implementation explains the result: OTelFilter only passes StatusCode to ResponseTraceAttrs; it does not inspect the request-context error, and the shared server semconv helper currently emits no error.type for either that error or an ordinary 5xx. The HTTP span conventions make error.type conditionally required when the request ends with an error, and their dropped-connection example preserves the selected response status while recording the disconnect class.

The proposed scope is right:

1. Preserve the selected 500.
2. Prefer the request-context failure over status-only classification and emit a predictable, low-cardinality error.type for it.
3. Emit the status-code string as error.type for an error status with no more specific cause.
4. Add a real-disconnect regression test.

Wrapping the request body/response writer and adding metrics are separate concerns and are not required to resolve this issue.