1
0
mirror of https://github.com/open-telemetry/opentelemetry-go.git synced 2025-07-15 01:04:25 +02:00

Avoid setting span status to Unknown when no HTTP status is provided; stdlib assumes it to be 200 OK (#908)

Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
This commit is contained in:
Anthony Mirabella
2020-07-07 12:00:21 -04:00
committed by GitHub
parent 9342eb24f1
commit eaa94e92b9
5 changed files with 51 additions and 6 deletions

View File

@ -20,6 +20,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
### Fixed ### Fixed
- The B3 Single Header name is now correctly `b3` instead of the previous `X-B3`. (#881) - The B3 Single Header name is now correctly `b3` instead of the previous `X-B3`. (#881)
- Ensure span status is not set to `Unknown` when no HTTP status code is provided as it is assumed to be `200 OK`. (#908)
## [0.7.0] - 2020-06-26 ## [0.7.0] - 2020-06-26

View File

@ -155,7 +155,6 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
h.handler.ServeHTTP(rww, r.WithContext(ctx)) h.handler.ServeHTTP(rww, r.WithContext(ctx))
setAfterServeAttributes(span, bw.read, rww.written, rww.statusCode, bw.err, rww.err) setAfterServeAttributes(span, bw.read, rww.written, rww.statusCode, bw.err, rww.err)
span.SetStatus(standard.SpanStatusFromHTTPStatusCode(rww.statusCode))
// Add request metrics // Add request metrics
@ -185,6 +184,7 @@ func setAfterServeAttributes(span trace.Span, read, wrote int64, statusCode int,
} }
if statusCode > 0 { if statusCode > 0 {
kv = append(kv, standard.HTTPAttributesFromHTTPStatusCode(statusCode)...) kv = append(kv, standard.HTTPAttributesFromHTTPStatusCode(statusCode)...)
span.SetStatus(standard.SpanStatusFromHTTPStatusCode(statusCode))
} }
if werr != nil && werr != io.EOF { if werr != nil && werr != io.EOF {
kv = append(kv, WriteErrorKey.String(werr.Error())) kv = append(kv, WriteErrorKey.String(werr.Error()))

View File

@ -22,9 +22,11 @@ import (
"testing" "testing"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
"google.golang.org/grpc/codes"
"go.opentelemetry.io/otel/api/kv" "go.opentelemetry.io/otel/api/kv"
"go.opentelemetry.io/otel/api/standard" "go.opentelemetry.io/otel/api/standard"
"go.opentelemetry.io/otel/api/trace"
mockmeter "go.opentelemetry.io/otel/internal/metric" mockmeter "go.opentelemetry.io/otel/internal/metric"
mocktrace "go.opentelemetry.io/otel/internal/trace" mocktrace "go.opentelemetry.io/otel/internal/trace"
) )
@ -71,7 +73,6 @@ func TestHandlerBasics(t *testing.T) {
standard.HTTPFlavorKey.String(fmt.Sprintf("1.%d", r.ProtoMinor)), standard.HTTPFlavorKey.String(fmt.Sprintf("1.%d", r.ProtoMinor)),
} }
standard.HTTPServerMetricAttributesFromHTTPRequest(operation, r)
assertMetricLabels(t, labelsToVerify, meterimpl.MeasurementBatches) assertMetricLabels(t, labelsToVerify, meterimpl.MeasurementBatches)
if got, expected := rr.Result().StatusCode, http.StatusOK; got != expected { if got, expected := rr.Result().StatusCode, http.StatusOK; got != expected {
@ -91,3 +92,43 @@ func TestHandlerBasics(t *testing.T) {
t.Fatalf("got %q, expected %q", got, expected) t.Fatalf("got %q, expected %q", got, expected)
} }
} }
func TestHandlerNoWrite(t *testing.T) {
rr := httptest.NewRecorder()
var id uint64
tracer := mocktrace.MockTracer{StartSpanID: &id}
operation := "test_handler"
var span trace.Span
h := NewHandler(
http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
span = trace.SpanFromContext(r.Context())
}), operation,
WithTracer(&tracer),
)
r, err := http.NewRequest(http.MethodGet, "http://localhost/", nil)
if err != nil {
t.Fatal(err)
}
h.ServeHTTP(rr, r)
if got, expected := rr.Result().StatusCode, http.StatusOK; got != expected {
t.Fatalf("got %d, expected %d", got, expected)
}
if got := rr.Header().Get("Traceparent"); got != "" {
t.Fatal("expected empty trace header")
}
if got, expected := id, uint64(1); got != expected {
t.Fatalf("got %d, expected %d", got, expected)
}
if mockSpan, ok := span.(*mocktrace.MockSpan); ok {
if got, expected := mockSpan.Status, codes.OK; got != expected {
t.Fatalf("got %q, expected %q", got, expected)
}
} else {
t.Fatalf("Expected *moctrace.MockSpan, got %T", span)
}
}

View File

@ -76,7 +76,6 @@ func (w *respWriterWrapper) Header() http.Header {
func (w *respWriterWrapper) Write(p []byte) (int, error) { func (w *respWriterWrapper) Write(p []byte) (int, error) {
if !w.wroteHeader { if !w.wroteHeader {
w.WriteHeader(http.StatusOK) w.WriteHeader(http.StatusOK)
w.wroteHeader = true
} }
n, err := w.ResponseWriter.Write(p) n, err := w.ResponseWriter.Write(p)
n1 := int64(n) n1 := int64(n)

View File

@ -29,6 +29,8 @@ type MockSpan struct {
sc apitrace.SpanContext sc apitrace.SpanContext
tracer apitrace.Tracer tracer apitrace.Tracer
Name string Name string
Status codes.Code
StatusMsg string
} }
var _ apitrace.Span = (*MockSpan)(nil) var _ apitrace.Span = (*MockSpan)(nil)
@ -49,6 +51,8 @@ func (ms *MockSpan) IsRecording() bool {
// SetStatus does nothing. // SetStatus does nothing.
func (ms *MockSpan) SetStatus(status codes.Code, msg string) { func (ms *MockSpan) SetStatus(status codes.Code, msg string) {
ms.Status = status
ms.StatusMsg = msg
} }
// SetError does nothing. // SetError does nothing.