otelecho: 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 otelecho.

Symptom

When a client disconnects mid-request, the handler observes the cancelled request context and typically returns an error, which surfaces as a 500. otelecho 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.

Middleware reads the status from c.Response().Status after next(c) and never inspects the request context:

opentelemetry-go-contrib/instrumentation/github.com/labstack/echo/otelecho/echo.go

Lines 96 to 131 in 5f30960

	err := next(c)
	if err != nil {
	span.SetAttributes(attribute.String("echo.error", err.Error()))
	cfg.OnError(c, err)
	}
	status := c.Response().Status
	span.SetStatus(semconvSrv.Status(status))
	span.SetAttributes(semconvSrv.ResponseTraceAttrs(semconv.ResponseTelemetry{
	StatusCode: status,
	WriteBytes: c.Response().Size,
	})...)
	// Record the server-side attributes.
	var additionalAttributes []attribute.KeyValue
	if cfg.MetricAttributeFn != nil {
	additionalAttributes = append(additionalAttributes, cfg.MetricAttributeFn(request)...)
	}
	if cfg.EchoMetricAttributeFn != nil {
	additionalAttributes = append(additionalAttributes, cfg.EchoMetricAttributeFn(c)...)
	}
	semconvSrv.RecordMetrics(ctx, semconv.ServerMetricData{
	ServerName: serverName,
	ResponseSize: c.Response().Size,
	MetricAttributes: semconv.MetricAttributes{
	Req: request,
	StatusCode: status,
	Route: c.Path(),
	AdditionalAttributes: additionalAttributes,
	},
	MetricData: semconv.MetricData{
	RequestSize: request.ContentLength,
	RequestDuration: time.Since(requestStartTime),
	},
	})

otelecho has an extra wrinkle worth fixing in the same pass. When the handler returns an error it records the full error string as a non-semconv echo.error attribute:

if err != nil {
    span.SetAttributes(attribute.String("echo.error", err.Error()))
    cfg.OnError(c, err)
}

That is unbounded-cardinality free text on a span attribute, and it is the only place the failure cause is captured — error.type is never set. Idiomatic Echo handlers return the error rather than writing a status themselves, so this is the common path, not an edge case. RecordMetrics carries StatusCode only, with no error.type.

Reproduction

Confirmed on main at 5f30960 with a real HTTP/1.1 disconnect, in both handler shapes. The client issues the request with a cancellable context and cancels it after 100ms.

Handler returns the context error (idiomatic Echo):

e.Use(otelecho.Middleware("srv", otelecho.WithTracerProvider(tp)))
e.GET("/hello", func(c echo.Context) error {
    <-c.Request().Context().Done()
    return c.Request().Context().Err()
})

span name   = "GET /hello"
span status = Error ""
attr http.route                 = /hello
attr echo.error                 = context canceled
attr http.response.body.size    = 36
attr http.response.status_code  = 500

Handler writes the status itself (matching the #9358 repro):

e.GET("/hello", func(c echo.Context) error {
    <-c.Request().Context().Done()
    return c.String(http.StatusInternalServerError, "cancelled")
})

span name   = "GET /hello"
span status = Error ""
attr http.route                 = /hello
attr http.response.body.size    = 9
attr http.response.status_code  = 500

Neither sets error.type. In the first case the cause is present but only as free-text echo.error; in the second it is lost entirely.

Suggested fix

Mirroring the triage in #9358 (comment), adapted to Echo:

1. Prefer an observed failure cause over the bare status code — the error returned by next(c), and the request-context error.
2. Set span status to Error and add a predictable, low-cardinality error.type for that cause, alongside (or instead of) the free-text echo.error attribute.
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 error.type to the metric attributes recorded by RecordMetrics, per the HTTP metric conventions.
5. Add a real disconnect regression test covering both handler shapes above.

Worth deciding separately whether echo.error should stay as-is, be reduced to a bounded value, or be dropped in favour of error.type — happy to split that into its own issue if preferred.

Note on shared code

otelecho/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 otelrestful. 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, HTTP metric conventions, and the dropped-connection example.

[MrAlias]

Confirmed. I independently reproduced this with a real HTTP/1.1 disconnect in both reported handler shapes, repeated 10 times each:

• returning c.Request().Context().Err() records span status Error, http.response.status_code=500, and free-text echo.error="context canceled", but no error.type;
• writing a 500 after cancellation records span status Error and http.response.status_code=500, but neither echo.error nor error.type;
• http.server.request.duration records the 500 in both cases and omits error.type.

I also verified the affected echo.go block at the reported commit and confirmed that the generated server.go copies for otelhttp, otelgin, otelmux, otelecho, and otelrestful are the same blob as internal/shared/semconv/server.go.tmpl at that commit.

Triage: confirmed bug.

The HTTP semantic conventions make error.type conditionally required when a server request ends with an error, including on http.server.request.duration. The current echo.error attribute is not a substitute: it is non-semantic-convention free text and does not cover the second handler shape.

The selected 500 should remain. Echo records the handler-selected response status, and the response writer cannot prove whether the peer received it. The dropped-connection example similarly retains http.response.status_code while adding a more specific error.type.

A focused fix for this issue should:

1. classify the request-context error and the error returned by next(c), preferring a specific observed cause over the status code;
2. set a predictable, low-cardinality error.type on the server span and server metrics, and keep span status Error for the failed operation;
3. preserve the selected status code; when no more specific cause exists, use the error status string (for example, "500") as error.type;
4. update internal/shared/semconv/server.go.tmpl, regenerate consumers, and wire the cause from otelecho.Middleware;
5. add a real-disconnect regression test for both Echo handler shapes.

I recommend leaving removal or semantic changes to the existing echo.error attribute to a separate follow-up. It has an unbounded-cardinality/sensitive-data risk, but changing it also has compatibility implications and is not required to fix the missing standard attribute.