Merge branch 'main' into main

CHANGELOG.md

@@ -17,6 +17,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

Makefile

@@ -24,7 +24,7 @@ endif
 GO = go
 GOTEST_MIN = go test -v -timeout 30s
 GOTEST = $(GOTEST_MIN) -race
-GOTEST_WITH_COVERAGE = $(GOTEST) -coverprofile=coverage.out -covermode=atomic -coverpkg=./...
+GOTEST_WITH_COVERAGE = $(GOTEST) -coverprofile=coverage.out -covermode=atomic -coverpkg=go.opentelemetry.io/contrib/...
 
 .DEFAULT_GOAL := precommit
 
@@ -40,24 +40,27 @@ $(TOOLS_DIR)/misspell: $(TOOLS_MOD_DIR)/go.mod $(TOOLS_MOD_DIR)/go.sum $(TOOLS_M
 	cd $(TOOLS_MOD_DIR) && \
 	go build -o $(TOOLS_DIR)/misspell github.com/client9/misspell/cmd/misspell
 
+$(TOOLS_DIR)/gocovmerge: $(TOOLS_MOD_DIR)/go.mod $(TOOLS_MOD_DIR)/go.sum $(TOOLS_MOD_DIR)/tools.go
+	cd $(TOOLS_MOD_DIR) && \
+	go build -o $(TOOLS_DIR)/gocovmerge github.com/wadey/gocovmerge
+
 $(TOOLS_DIR)/stringer: $(TOOLS_MOD_DIR)/go.mod $(TOOLS_MOD_DIR)/go.sum $(TOOLS_MOD_DIR)/tools.go
 	cd $(TOOLS_MOD_DIR) && \
 	go build -o $(TOOLS_DIR)/stringer golang.org/x/tools/cmd/stringer
 
 precommit: dependabot-check license-check generate lint build test
 
 .PHONY: test-with-coverage
-test-with-coverage:
+test-with-coverage: $(TOOLS_DIR)/gocovmerge
 	set -e; \
 	printf "" > coverage.txt; \
 	for dir in $(ALL_COVERAGE_MOD_DIRS); do \
 	  echo "go test ./... + coverage in $${dir}"; \
 	  (cd "$${dir}" && \
 	    $(GOTEST_WITH_COVERAGE) ./... && \
 	    go tool cover -html=coverage.out -o coverage.html); \
-	  [ -f "$${dir}/coverage.out" ] && cat "$${dir}/coverage.out" >> coverage.txt; \
 	done; \
-	sed -i.bak -e '2,$$ { /^mode: /d; }' coverage.txt && rm coverage.txt.bak
+	$(TOOLS_DIR)/gocovmerge $$(find . -name coverage.out) > coverage.txt
 
 .PHONY: ci
 ci: precommit check-clean-work-tree test-with-coverage test-386

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)
+	}
+}

tools/go.mod

@@ -5,5 +5,6 @@ go 1.13
 require (
 	github.com/client9/misspell v0.3.4
 	github.com/golangci/golangci-lint v1.41.1
+	github.com/wadey/gocovmerge v0.0.0-20160331181800-b5bfa59ec0ad
 	golang.org/x/tools v0.1.5
 )

tools/go.sum

@@ -638,6 +638,8 @@ github.com/valyala/fasthttp v1.16.0/go.mod h1:YOKImeEosDdBPnxc0gy7INqi3m1zK6A+xl
 github.com/valyala/quicktemplate v1.6.3/go.mod h1:fwPzK2fHuYEODzJ9pkw0ipCPNHZ2tD5KW4lOuSdPKzY=
 github.com/valyala/tcplisten v0.0.0-20161114210144-ceec8f93295a/go.mod h1:v3UYOV9WzVtRmSR+PDvWpU/qWl4Wa5LApYYX4ZtKbio=
 github.com/viki-org/dnscache v0.0.0-20130720023526-c70c1f23c5d8/go.mod h1:dniwbG03GafCjFohMDmz6Zc6oCuiqgH6tGNyXTkHzXE=
+github.com/wadey/gocovmerge v0.0.0-20160331181800-b5bfa59ec0ad h1:W0LEBv82YCGEtcmPA3uNZBI33/qF//HAAs3MawDjRa0=
+github.com/wadey/gocovmerge v0.0.0-20160331181800-b5bfa59ec0ad/go.mod h1:Hy8o65+MXnS6EwGElrSRjUzQDLXreJlzYLlWiHtt8hM=
 github.com/xiang90/probing v0.0.0-20190116061207-43a291ad63a2/go.mod h1:UETIi67q53MR2AWcXfiuqkDkRtnGDLqkBTpCHuJHxtU=
 github.com/xo/terminfo v0.0.0-20210125001918-ca9a967f8778/go.mod h1:2MuV+tbUrU1zIOPMxZ5EncGwgmMJsa+9ucAQZXxsObs=
 github.com/xordataexchange/crypt v0.0.3-0.20170626215501-b2862e3d0a77/go.mod h1:aYKd//L2LvnjZzWKhF00oedf4jCCReLcmhLdhm1A27Q=

tools/tools.go

@@ -19,5 +19,6 @@ package tools
 import (
 	_ "github.com/client9/misspell/cmd/misspell"
 	_ "github.com/golangci/golangci-lint/cmd/golangci-lint"
+	_ "github.com/wadey/gocovmerge"
 	_ "golang.org/x/tools/cmd/stringer"
 )