Mark the span as error when the HTTP transporter fails (#950)

* Mark the span as error when the HTTP transporter fails

Signed-off-by: Xabier Larrakoetxea <me@slok.dev>

* Update changelog with fix and PR

Signed-off-by: Xabier Larrakoetxea <me@slok.dev>

* Add HTTP Transport tests for error status on spans

Signed-off-by: Xabier Larrakoetxea <me@slok.dev>

* Fix linting issues of an unused context set

Signed-off-by: Xabier Larrakoetxea <me@slok.dev>

* Remove unused tracer properties and change Fatalf in favor of Errorf on some test parts

Signed-off-by: Xabier Larrakoetxea <me@slok.dev>

Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>

CHANGELOG.md

@@ -12,6 +12,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 
 - `otelmongodb` span attributes, name and span status now conform to specification. (#769)
 
+### Fixed
+
+- Fix span not marked as error in `otelhttp.Transport` when `RoundTrip` fails with an error. (#950)
+
 ## [0.22.0] - 2021-07-26
 
 ### Added

instrumentation/net/http/otelhttp/transport.go

@@ -19,6 +19,7 @@ import (
 	"io"
 	"net/http"
 
+	"go.opentelemetry.io/otel/codes"
 	"go.opentelemetry.io/otel/propagation"
 	semconv "go.opentelemetry.io/otel/semconv/v1.4.0"
 	"go.opentelemetry.io/otel/trace"
@@ -97,6 +98,7 @@ func (t *Transport) RoundTrip(r *http.Request) (*http.Response, error) {
 	res, err := t.rt.RoundTrip(r)
 	if err != nil {
 		span.RecordError(err)
+		span.SetStatus(codes.Error, err.Error())
 		span.End()
 		return res, err
 	}
@@ -126,6 +128,7 @@ func (wb *wrappedBody) Read(b []byte) (int, error) {
 		wb.span.End()
 	default:
 		wb.span.RecordError(err)
+		wb.span.SetStatus(codes.Error, err.Error())
 	}
 	return n, err
 }

instrumentation/net/http/otelhttp/transport_test.go

@@ -16,12 +16,15 @@ package otelhttp
 
 import (
 	"bytes"
+	"context"
 	"fmt"
 	"io/ioutil"
 	"net/http"
 	"net/http/httptest"
+	"strings"
 	"testing"
 
+	"go.opentelemetry.io/otel/codes"
 	"go.opentelemetry.io/otel/oteltest"
 	"go.opentelemetry.io/otel/propagation"
 	"go.opentelemetry.io/otel/trace"
@@ -243,3 +246,99 @@ func TestTransportUsesFormatter(t *testing.T) {
 	}
 
 }
+
+func TestTransportErrorStatus(t *testing.T) {
+	// Prepare tracing stuff.
+	spanRecorder := new(oteltest.SpanRecorder)
+	provider := oteltest.NewTracerProvider(
+		oteltest.WithSpanRecorder(spanRecorder),
+	)
+
+	// Run a server and stop to make sure nothing is listening and force the error.
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}))
+	server.Close()
+
+	// Create our Transport and make request.
+	tr := NewTransport(
+		http.DefaultTransport,
+		WithTracerProvider(provider),
+	)
+	c := http.Client{Transport: tr}
+	r, err := http.NewRequest(http.MethodGet, server.URL, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	_, err = c.Do(r)
+	if err == nil {
+		t.Fatal("transport should have returned an error, it didn't")
+	}
+
+	// Check span.
+	gotSpans := spanRecorder.Completed()
+	if len(gotSpans) != 1 {
+		t.Fatalf("expected 1 span; got: %d", len(gotSpans))
+	}
+
+	spanEnded := gotSpans[0].Ended()
+	if !spanEnded {
+		t.Errorf("span should be ended; it isn't")
+	}
+
+	spanStatusCode := gotSpans[0].StatusCode()
+	if spanStatusCode != codes.Error {
+		t.Errorf("expected error status code on span; got: %q", spanStatusCode)
+	}
+
+	spanStatusMessage := gotSpans[0].StatusMessage()
+	if !strings.Contains(spanStatusMessage, "connect: connection refused") {
+		t.Errorf("expected error status message on span; got: %q", spanStatusMessage)
+	}
+}
+
+type testErrorReadCloser struct{}
+
+func (testErrorReadCloser) Read(p []byte) (n int, err error) { return 0, fmt.Errorf("something") }
+func (testErrorReadCloser) Close() error                     { return nil }
+
+func TestWrappedBodyReadErrorStatus(t *testing.T) {
+	// Prepare tracing stuff.
+	spanRecorder := new(oteltest.SpanRecorder)
+	provider := oteltest.NewTracerProvider(
+		oteltest.WithSpanRecorder(spanRecorder),
+	)
+	tracer := provider.Tracer("")
+	ctx := context.Background()
+	_, span := tracer.Start(ctx, "test")
+
+	// Create our wrapper.
+	wb := wrappedBody{
+		span: span,
+		body: testErrorReadCloser{},
+	}
+	_, err := wb.Read([]byte{})
+	if err == nil {
+		t.Fatalf("expected error while reading")
+	}
+	wb.Close()
+
+	// Check span.
+	gotSpans := spanRecorder.Completed()
+	if len(gotSpans) != 1 {
+		t.Fatalf("expected 1 span; got: %d", len(gotSpans))
+	}
+
+	spanEnded := gotSpans[0].Ended()
+	if !spanEnded {
+		t.Errorf("span should be ended; it isn't")
+	}
+
+	spanStatusCode := gotSpans[0].StatusCode()
+	if spanStatusCode != codes.Error {
+		t.Errorf("expected error status code on span; got: %q", spanStatusCode)
+	}
+
+	spanStatusMessage := gotSpans[0].StatusMessage()
+	if !strings.Contains(spanStatusMessage, "something") {
+		t.Errorf("expected error status message on span; got: %q", spanStatusMessage)
+	}
+}