Change span names in HTTP instrumentation package to conform with gui…

…delines (#757)

* Change span name formatter and add tests

* Fix test

* Remove comment

* Use string concatenation

* Add changelog entry

* Change function name

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

CHANGELOG.md

@@ -26,6 +26,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 ### Changed
 
 - Upgrade to v0.19.0 of `go.opentelemetry.io/otel`.
+- Fix Span names created in HTTP Instrumentation package to conform with guidelines. (#757)
 
 ## [0.18.0] - 2021-03-04
 

instrumentation/net/http/otelhttp/client_test.go

@@ -78,8 +78,8 @@ func TestConvenienceWrappers(t *testing.T) {
 
 	spans := sr.Completed()
 	require.Equal(t, 4, len(spans))
-	assert.Equal(t, "GET", spans[0].Name())
-	assert.Equal(t, "HEAD", spans[1].Name())
-	assert.Equal(t, "POST", spans[2].Name())
-	assert.Equal(t, "POST", spans[3].Name())
+	assert.Equal(t, "HTTP GET", spans[0].Name())
+	assert.Equal(t, "HTTP HEAD", spans[1].Name())
+	assert.Equal(t, "HTTP POST", spans[2].Name())
+	assert.Equal(t, "HTTP POST", spans[3].Name())
 }

instrumentation/net/http/otelhttp/config_test.go

@@ -85,7 +85,7 @@ func TestSpanNameFormatter(t *testing.T) {
 		{
 			name:      "default transport formatter",
 			formatter: defaultTransportFormatter,
-			expected:  http.MethodGet,
+			expected:  "HTTP GET",
 		},
 		{
 			name: "custom formatter",

instrumentation/net/http/otelhttp/transport.go

@@ -72,7 +72,7 @@ func (t *Transport) applyConfig(c *config) {
 }
 
 func defaultTransportFormatter(_ string, r *http.Request) string {
-	return r.Method
+	return "HTTP " + r.Method
 }
 
 // RoundTrip creates a Span and propagates its context via the provided request's headers

instrumentation/net/http/otelhttp/transport_test.go

@@ -122,3 +122,124 @@ func TestNilTransport(t *testing.T) {
 		t.Fatalf("unexpected content: got %s, expected %s", body, content)
 	}
 }
+
+func TestTransportFormatter(t *testing.T) {
+
+	var httpMethods = []struct {
+		name     string
+		method   string
+		expected string
+	}{
+		{
+			"GET method",
+			http.MethodGet,
+			"HTTP GET",
+		},
+		{
+			"HEAD method",
+			http.MethodHead,
+			"HTTP HEAD",
+		},
+		{
+			"POST method",
+			http.MethodPost,
+			"HTTP POST",
+		},
+		{
+			"PUT method",
+			http.MethodPut,
+			"HTTP PUT",
+		},
+		{
+			"PATCH method",
+			http.MethodPatch,
+			"HTTP PATCH",
+		},
+		{
+			"DELETE method",
+			http.MethodDelete,
+			"HTTP DELETE",
+		},
+		{
+			"CONNECT method",
+			http.MethodConnect,
+			"HTTP CONNECT",
+		},
+		{
+			"OPTIONS method",
+			http.MethodOptions,
+			"HTTP OPTIONS",
+		},
+		{
+			"TRACE method",
+			http.MethodTrace,
+			"HTTP TRACE",
+		},
+	}
+
+	for _, tc := range httpMethods {
+		t.Run(tc.name, func(t *testing.T) {
+			r, err := http.NewRequest(tc.method, "http://localhost/", nil)
+			if err != nil {
+				t.Fatal(err)
+			}
+			formattedName := defaultTransportFormatter("", r)
+
+			if formattedName != tc.expected {
+				t.Fatalf("unexpected name: got %s, expected %s", formattedName, tc.expected)
+			}
+		})
+	}
+
+}
+
+func TestTransportUsesFormatter(t *testing.T) {
+	prop := propagation.TraceContext{}
+	spanRecorder := new(oteltest.SpanRecorder)
+	provider := oteltest.NewTracerProvider(
+		oteltest.WithSpanRecorder(spanRecorder),
+	)
+	content := []byte("Hello, world!")
+
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		ctx := prop.Extract(r.Context(), propagation.HeaderCarrier(r.Header))
+		span := trace.SpanContextFromContext(ctx)
+		tgtID, err := trace.SpanIDFromHex(fmt.Sprintf("%016x", uint(2)))
+		if err != nil {
+			t.Fatalf("Error converting id to SpanID: %s", err.Error())
+		}
+		if span.SpanID() != tgtID {
+			t.Fatalf("testing remote SpanID: got %s, expected %s", span.SpanID(), tgtID)
+		}
+		if _, err := w.Write(content); err != nil {
+			t.Fatal(err)
+		}
+	}))
+	defer ts.Close()
+
+	r, err := http.NewRequest(http.MethodGet, ts.URL, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	tr := NewTransport(
+		http.DefaultTransport,
+		WithTracerProvider(provider),
+		WithPropagators(prop),
+	)
+
+	c := http.Client{Transport: tr}
+	res, err := c.Do(r)
+	if err != nil {
+		t.Fatal(err)
+	}
+	res.Body.Close()
+
+	spans := spanRecorder.Completed()
+	spanName := spans[0].Name()
+	expectedName := "HTTP GET"
+	if spanName != expectedName {
+		t.Fatalf("unexpected name: got %s, expected %s", spanName, expectedName)
+	}
+
+}