From 7218278c01eee06ef13eb1d2b5b89fe5892f4a73 Mon Sep 17 00:00:00 2001 From: toim Date: Thu, 5 Mar 2026 22:48:16 +0200 Subject: [PATCH 1/2] Improve span status code handling and docs --- extrator.go | 17 ++++++++++- extrator_test.go | 78 ++++++++++++++++++++++++++++++++++++++++++++++++ otel.go | 10 ++----- 3 files changed, 97 insertions(+), 8 deletions(-) diff --git a/extrator.go b/extrator.go index 6dcef4e..166d5d6 100644 --- a/extrator.go +++ b/extrator.go @@ -560,7 +560,22 @@ func SpanNameFormatter(v Values) string { return method } -func spanStatus(code int) (codes.Code, string) { +// SpanStatus returns the span status code and error description based on the HTTP status code and error. +// +// Spec: +// +// Span Status MUST be left unset if HTTP status code was in the 1xx, 2xx or 3xx ranges, unless there was another error +// (e.g., network error receiving the response body; or 3xx codes with max redirects exceeded), in which case status +// MUST be set to Error. +// +// Reference: +// - [Span.Status](https://opentelemetry.io/docs/specs/semconv/http/http-spans/#status) +// - [Recording errors on spans](https://opentelemetry.io/docs/specs/semconv/general/recording-errors/) +func SpanStatus(code int, err error) (codes.Code, string) { + if err != nil { // When the operation ends with an error, instrumentation: SHOULD set the span status code to Error + return codes.Error, err.Error() + } + if code < 100 || code >= 600 { return codes.Error, fmt.Sprintf("Invalid HTTP status code %d", code) } diff --git a/extrator_test.go b/extrator_test.go index cb3d84c..7b531be 100644 --- a/extrator_test.go +++ b/extrator_test.go @@ -2,11 +2,13 @@ package echootel import ( "context" + "errors" "net/http" "testing" "github.com/stretchr/testify/assert" "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/codes" ) func TestValues_ExtractRequest(t *testing.T) { @@ -683,3 +685,79 @@ func TestSplitProto(t *testing.T) { }) } } + +func TestSpanStatus(t *testing.T) { + var testCases = []struct { + name string + whenStatus int + whenError error + expectCode codes.Code + expectDesc string + }{ + { + name: "error overrides status code", + whenStatus: 200, + whenError: errors.New("network error"), + expectCode: codes.Error, + expectDesc: "network error", + }, + { + name: "invalid status code below range", + whenStatus: 99, + whenError: nil, + expectCode: codes.Error, + expectDesc: "Invalid HTTP status code 99", + }, + { + name: "invalid status code above range", + whenStatus: 600, + whenError: nil, + expectCode: codes.Error, + expectDesc: "Invalid HTTP status code 600", + }, + { + name: "server error 500", + whenStatus: 500, + whenError: nil, + expectCode: codes.Error, + expectDesc: "", + }, + { + name: "server error 503", + whenStatus: 503, + whenError: nil, + expectCode: codes.Error, + expectDesc: "", + }, + { + name: "informational status 100", + whenStatus: 100, + whenError: nil, + expectCode: codes.Unset, + expectDesc: "", + }, + { + name: "success status 200", + whenStatus: 200, + whenError: nil, + expectCode: codes.Unset, + expectDesc: "", + }, + { + name: "redirect status 302", + whenStatus: 302, + whenError: nil, + expectCode: codes.Unset, + expectDesc: "", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + code, desc := SpanStatus(tc.whenStatus, tc.whenError) + + assert.Equal(t, tc.expectCode, code) + assert.Equal(t, tc.expectDesc, desc) + }) + } +} diff --git a/otel.go b/otel.go index aa23842..d858150 100644 --- a/otel.go +++ b/otel.go @@ -11,7 +11,6 @@ import ( "github.com/labstack/echo/v5/middleware" "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" - "go.opentelemetry.io/otel/codes" "go.opentelemetry.io/otel/metric" "go.opentelemetry.io/otel/propagation" semconv "go.opentelemetry.io/otel/semconv/v1.39.0" @@ -213,30 +212,27 @@ func (config Config) ToMiddleware() (echo.MiddlewareFunc, error) { } }() - // serve the request to the next middleware err := next(c) + + resp, status := echo.ResolveResponseStatus(c.Response(), err) + span.SetStatus(SpanStatus(status, err)) if err != nil { span.SetAttributes(semconv.ErrorType(err)) - span.SetStatus(codes.Error, err.Error()) if config.OnNextError != nil { config.OnNextError(c, err) } } - resp, status := echo.ResolveResponseStatus(c.Response(), err) ev.HTTPResponseStatusCode = status if resp != nil { ev.HTTPResponseBodySize = resp.Size } - span.SetStatus(spanStatus(status)) - endAttributes := ev.SpanEndAttributes() if config.SpanEndAttributes != nil { endAttributes = config.SpanEndAttributes(c, &ev, endAttributes) } span.SetAttributes(endAttributes...) - // Record the server-side attributes. iv := RecordValues{ RequestDuration: time.Since(requestStartTime), ExtractedValues: ev, From 5d4c46837f89352e245a89f6e76768d9511a81cb Mon Sep 17 00:00:00 2001 From: toim Date: Thu, 5 Mar 2026 22:51:59 +0200 Subject: [PATCH 2/2] relocate span status handling --- otel.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/otel.go b/otel.go index d858150..bd90e01 100644 --- a/otel.go +++ b/otel.go @@ -214,14 +214,14 @@ func (config Config) ToMiddleware() (echo.MiddlewareFunc, error) { err := next(c) - resp, status := echo.ResolveResponseStatus(c.Response(), err) - span.SetStatus(SpanStatus(status, err)) if err != nil { span.SetAttributes(semconv.ErrorType(err)) if config.OnNextError != nil { config.OnNextError(c, err) } } + resp, status := echo.ResolveResponseStatus(c.Response(), err) + span.SetStatus(SpanStatus(status, err)) ev.HTTPResponseStatusCode = status if resp != nil {