Span status from HTTP code: Do not set status message if it can be inferred (#1681)

* Do not set status message if reason can be inferred

* Update CHANGELOG

* Add comment on validation func

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

CHANGELOG.md

@@ -40,6 +40,9 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 - Jaeger Exporter: Ensure mapping between OTEL and Jaeger span data complies with the specification. (#1626)
 - The `otel-collector` example now correctly flushes metric events prior to shutting down the exporter. (#1678)
 
+### Fixed
+- Do not set span status message in `SpanStatusFromHTTPStatusCode` if it can be inferred from `http.status_code`. (#1681)
+
 ## [0.18.0] - 2020-03-03
 
 ### Added

semconv/http.go

@@ -264,29 +264,34 @@ var validRangesPerCategory = map[int][]codeRange{
 // SpanStatusFromHTTPStatusCode generates a status code and a message
 // as specified by the OpenTelemetry specification for a span.
 func SpanStatusFromHTTPStatusCode(code int) (codes.Code, string) {
-	spanCode, valid := func() (codes.Code, bool) {
-		category := code / 100
-		ranges, ok := validRangesPerCategory[category]
-		if !ok {
-			return codes.Error, false
-		}
-		ok = false
-		for _, crange := range ranges {
-			ok = crange.contains(code)
-			if ok {
-				break
-			}
-		}
-		if !ok {
-			return codes.Error, false
-		}
-		if category > 0 && category < 4 {
-			return codes.Unset, true
-		}
-		return codes.Error, true
-	}()
+	spanCode, valid := validateHTTPStatusCode(code)
 	if !valid {
 		return spanCode, fmt.Sprintf("Invalid HTTP status code %d", code)
 	}
-	return spanCode, fmt.Sprintf("HTTP status code: %d", code)
+	return spanCode, ""
+}
+
+// Validates the HTTP status code and returns corresponding span status code.
+// If the `code` is not a valid HTTP status code, returns span status Error
+// and false.
+func validateHTTPStatusCode(code int) (codes.Code, bool) {
+	category := code / 100
+	ranges, ok := validRangesPerCategory[category]
+	if !ok {
+		return codes.Error, false
+	}
+	ok = false
+	for _, crange := range ranges {
+		ok = crange.contains(code)
+		if ok {
+			break
+		}
+	}
+	if !ok {
+		return codes.Error, false
+	}
+	if category > 0 && category < 4 {
+		return codes.Unset, true
+	}
+	return codes.Error, true
 }

semconv/http_test.go

@@ -696,8 +696,15 @@ func TestHTTPAttributesFromHTTPStatusCode(t *testing.T) {
 func TestSpanStatusFromHTTPStatusCode(t *testing.T) {
 	for code := 0; code < 1000; code++ {
 		expected := getExpectedCodeForHTTPCode(code)
-		got, _ := SpanStatusFromHTTPStatusCode(code)
+		got, msg := SpanStatusFromHTTPStatusCode(code)
 		assert.Equalf(t, expected, got, "%s vs %s", expected, got)
+
+		_, valid := validateHTTPStatusCode(code)
+		if !valid {
+			assert.NotEmpty(t, msg, "message should be set if error cannot be inferred from code")
+		} else {
+			assert.Empty(t, msg, "message should not be set if error can be inferred from code")
+		}
 	}
 }