search

riandyrn / otelchi

OpenTelemetry instrumentation for go-chi/chi
Apache License 2.0
116 stars 38 forks source link

panic doesn't mark span as error #12

Closed aep closed 1 year ago

aep commented 2 years ago

the middleware doesn't call recover anywhere, pressumably because otel already does that according to the trace

go.opentelemetry.io/otel/sdk/trace.(*recordingSpan).End.func1()
    /home/aep/.go/pkg/mod/go.opentelemetry.io/otel/sdk@v1.11.0/trace/span.go:396 +0x2a
go.opentelemetry.io/otel/sdk/trace.(*recordingSpan).End(0xc00033e480, {0x0, 0x0, 0xc0000b4780?})
    /home/aep/.go/pkg/mod/go.opentelemetry.io/otel/sdk@v1.11.0/trace/span.go:435 +0x8ee

this records an event, but does not set the span as error.

confusingly, a 404 is set as error, but i don't understand where that's happening either. because otelchi calls span.SetStatus . should it also do that for panic, or is that a problem in otel?

aep commented 2 years ago

https://github.com/open-telemetry/opentelemetry-go/issues/3283

aep commented 2 years ago

it works when using middleware.Recoverer after otelchi, so that otelchi sees a 500. possibly otel does it correctly but since otelchi always calls span.SetStatus it is overriden?

riandyrn commented 1 year ago

Closed based on https://github.com/riandyrn/otelchi/pull/13#issuecomment-1374393645.