add new function to exclude span kind 4xx for status code (#2339)

* add new function to exclude span kind 4xx for status code

* add changelog

* make precommit

* add unit test

* feat: redo by reusing validateHTTPStatusCode

* feat: make precommit

* feat: move the judgement to SpanStatusFromHTTPStatusCodeAndSpanKind

* feat: increase unittest coverage

* feat: fix logic

* feat: fix unittest

* fix code style

* Update CHANGELOG.md

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

CHANGELOG.md

@@ -23,6 +23,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
   - The following interface types simply moved from `metric` to `metric/sdkapi`: `Descriptor`, `MeterImpl`, `InstrumentImpl`, `SyncImpl`, `BoundSyncImpl`, `AsyncImpl`, `AsyncRunner`, `AsyncSingleRunner`, and `AsyncBatchRunner`
   - The following struct types moved and are replaced with type aliases, since they are exposed to the user: `Observation`, `Measurement`.
   - The No-op implementations of sync and async instruments are no longer exported, new functions `sdkapi.NewNoopAsyncInstrument()` and `sdkapi.NewNoopSyncInstrument()` are provided instead. (#2271)
+- Add `SpanStatusFromHTTPStatusCodeAndSpanKind` to all `semconv` packages to return a span status code similar to `SpanStatusFromHTTPStatusCode`, but exclude `4XX` HTTP errors as span errors if the span is of server kind. (#2296)
 - Update the SDK `BatchSpanProcessor` to export all queued spans when `ForceFlush` is called. (#2080, #2335)
 - Change `resource.Default` to be evaluated the first time it is called, rather than on import. This allows the caller the option to update `OTEL_RESOURCE_ATTRIBUTES` first, such as with `os.Setenv`. (#2371)
 

semconv/v1.4.0/http.go

@@ -23,6 +23,7 @@ import (
 
 	"go.opentelemetry.io/otel/attribute"
 	"go.opentelemetry.io/otel/codes"
+	"go.opentelemetry.io/otel/trace"
 )
 
 // HTTP scheme attributes.
@@ -269,6 +270,21 @@ func SpanStatusFromHTTPStatusCode(code int) (codes.Code, string) {
 	return spanCode, ""
 }
 
+// SpanStatusFromHTTPStatusCodeAndSpanKind generates a status code and a message
+// as specified by the OpenTelemetry specification for a span.
+// Exclude 4xx for SERVER to set the appropriate status.
+func SpanStatusFromHTTPStatusCodeAndSpanKind(code int, spanKind trace.SpanKind) (codes.Code, string) {
+	spanCode, valid := validateHTTPStatusCode(code)
+	if !valid {
+		return spanCode, fmt.Sprintf("Invalid HTTP status code %d", code)
+	}
+	category := code / 100
+	if spanKind == trace.SpanKindServer && category == 4 {
+		return codes.Unset, ""
+	}
+	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.

semconv/v1.4.0/http_test.go

@@ -20,6 +20,8 @@ import (
 	"strings"
 	"testing"
 
+	"go.opentelemetry.io/otel/trace"
+
 	"github.com/google/go-cmp/cmp"
 	"github.com/stretchr/testify/assert"
 
@@ -854,7 +856,7 @@ func TestHTTPAttributesFromHTTPStatusCode(t *testing.T) {
 
 func TestSpanStatusFromHTTPStatusCode(t *testing.T) {
 	for code := 0; code < 1000; code++ {
-		expected := getExpectedCodeForHTTPCode(code)
+		expected := getExpectedCodeForHTTPCode(code, trace.SpanKindClient)
 		got, msg := SpanStatusFromHTTPStatusCode(code)
 		assert.Equalf(t, expected, got, "%s vs %s", expected, got)
 
@@ -867,7 +869,24 @@ func TestSpanStatusFromHTTPStatusCode(t *testing.T) {
 	}
 }
 
-func getExpectedCodeForHTTPCode(code int) codes.Code {
+func TestSpanStatusFromHTTPStatusCodeAndSpanKind(t *testing.T) {
+	for code := 0; code < 1000; code++ {
+		expected := getExpectedCodeForHTTPCode(code, trace.SpanKindClient)
+		got, msg := SpanStatusFromHTTPStatusCodeAndSpanKind(code, trace.SpanKindClient)
+		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")
+		}
+	}
+	code, _ := SpanStatusFromHTTPStatusCodeAndSpanKind(400, trace.SpanKindServer)
+	assert.Equalf(t, codes.Unset, code, "message should be set if error cannot be inferred from code")
+}
+
+func getExpectedCodeForHTTPCode(code int, spanKind trace.SpanKind) codes.Code {
 	if http.StatusText(code) == "" {
 		return codes.Error
 	}
@@ -886,6 +905,9 @@ func getExpectedCodeForHTTPCode(code int) codes.Code {
 	if category > 0 && category < 4 {
 		return codes.Unset
 	}
+	if spanKind == trace.SpanKindServer && category == 4 {
+		return codes.Unset
+	}
 	return codes.Error
 }
 

semconv/v1.5.0/http.go

@@ -23,6 +23,7 @@ import (
 
 	"go.opentelemetry.io/otel/attribute"
 	"go.opentelemetry.io/otel/codes"
+	"go.opentelemetry.io/otel/trace"
 )
 
 // HTTP scheme attributes.
@@ -269,6 +270,21 @@ func SpanStatusFromHTTPStatusCode(code int) (codes.Code, string) {
 	return spanCode, ""
 }
 
+// SpanStatusFromHTTPStatusCodeAndSpanKind generates a status code and a message
+// as specified by the OpenTelemetry specification for a span.
+// Exclude 4xx for SERVER to set the appropriate status.
+func SpanStatusFromHTTPStatusCodeAndSpanKind(code int, spanKind trace.SpanKind) (codes.Code, string) {
+	spanCode, valid := validateHTTPStatusCode(code)
+	if !valid {
+		return spanCode, fmt.Sprintf("Invalid HTTP status code %d", code)
+	}
+	category := code / 100
+	if spanKind == trace.SpanKindServer && category == 4 {
+		return codes.Unset, ""
+	}
+	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.

semconv/v1.5.0/http_test.go

@@ -23,6 +23,8 @@ import (
 	"github.com/google/go-cmp/cmp"
 	"github.com/stretchr/testify/assert"
 
+	"go.opentelemetry.io/otel/trace"
+
 	"go.opentelemetry.io/otel/attribute"
 	"go.opentelemetry.io/otel/codes"
 )
@@ -854,7 +856,7 @@ func TestHTTPAttributesFromHTTPStatusCode(t *testing.T) {
 
 func TestSpanStatusFromHTTPStatusCode(t *testing.T) {
 	for code := 0; code < 1000; code++ {
-		expected := getExpectedCodeForHTTPCode(code)
+		expected := getExpectedCodeForHTTPCode(code, trace.SpanKindClient)
 		got, msg := SpanStatusFromHTTPStatusCode(code)
 		assert.Equalf(t, expected, got, "%s vs %s", expected, got)
 
@@ -867,7 +869,24 @@ func TestSpanStatusFromHTTPStatusCode(t *testing.T) {
 	}
 }
 
-func getExpectedCodeForHTTPCode(code int) codes.Code {
+func TestSpanStatusFromHTTPStatusCodeAndSpanKind(t *testing.T) {
+	for code := 0; code < 1000; code++ {
+		expected := getExpectedCodeForHTTPCode(code, trace.SpanKindClient)
+		got, msg := SpanStatusFromHTTPStatusCodeAndSpanKind(code, trace.SpanKindClient)
+		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")
+		}
+	}
+	code, _ := SpanStatusFromHTTPStatusCodeAndSpanKind(400, trace.SpanKindServer)
+	assert.Equalf(t, codes.Unset, code, "message should be set if error cannot be inferred from code")
+}
+
+func getExpectedCodeForHTTPCode(code int, spanKind trace.SpanKind) codes.Code {
 	if http.StatusText(code) == "" {
 		return codes.Error
 	}
@@ -886,6 +905,9 @@ func getExpectedCodeForHTTPCode(code int) codes.Code {
 	if category > 0 && category < 4 {
 		return codes.Unset
 	}
+	if spanKind == trace.SpanKindServer && category == 4 {
+		return codes.Unset
+	}
 	return codes.Error
 }
 

semconv/v1.6.1/http.go

@@ -23,6 +23,7 @@ import (
 
 	"go.opentelemetry.io/otel/attribute"
 	"go.opentelemetry.io/otel/codes"
+	"go.opentelemetry.io/otel/trace"
 )
 
 // HTTP scheme attributes.
@@ -269,6 +270,21 @@ func SpanStatusFromHTTPStatusCode(code int) (codes.Code, string) {
 	return spanCode, ""
 }
 
+// SpanStatusFromHTTPStatusCodeAndSpanKind generates a status code and a message
+// as specified by the OpenTelemetry specification for a span.
+// Exclude 4xx for SERVER to set the appropriate status.
+func SpanStatusFromHTTPStatusCodeAndSpanKind(code int, spanKind trace.SpanKind) (codes.Code, string) {
+	spanCode, valid := validateHTTPStatusCode(code)
+	if !valid {
+		return spanCode, fmt.Sprintf("Invalid HTTP status code %d", code)
+	}
+	category := code / 100
+	if spanKind == trace.SpanKindServer && category == 4 {
+		return codes.Unset, ""
+	}
+	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.

semconv/v1.6.1/http_test.go

@@ -25,6 +25,7 @@ import (
 
 	"go.opentelemetry.io/otel/attribute"
 	"go.opentelemetry.io/otel/codes"
+	"go.opentelemetry.io/otel/trace"
 )
 
 type tlsOption int
@@ -854,7 +855,7 @@ func TestHTTPAttributesFromHTTPStatusCode(t *testing.T) {
 
 func TestSpanStatusFromHTTPStatusCode(t *testing.T) {
 	for code := 0; code < 1000; code++ {
-		expected := getExpectedCodeForHTTPCode(code)
+		expected := getExpectedCodeForHTTPCode(code, trace.SpanKindClient)
 		got, msg := SpanStatusFromHTTPStatusCode(code)
 		assert.Equalf(t, expected, got, "%s vs %s", expected, got)
 
@@ -867,7 +868,24 @@ func TestSpanStatusFromHTTPStatusCode(t *testing.T) {
 	}
 }
 
-func getExpectedCodeForHTTPCode(code int) codes.Code {
+func TestSpanStatusFromHTTPStatusCodeAndSpanKind(t *testing.T) {
+	for code := 0; code < 1000; code++ {
+		expected := getExpectedCodeForHTTPCode(code, trace.SpanKindClient)
+		got, msg := SpanStatusFromHTTPStatusCodeAndSpanKind(code, trace.SpanKindClient)
+		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")
+		}
+	}
+	code, _ := SpanStatusFromHTTPStatusCodeAndSpanKind(400, trace.SpanKindServer)
+	assert.Equalf(t, codes.Unset, code, "message should be set if error cannot be inferred from code")
+}
+
+func getExpectedCodeForHTTPCode(code int, spanKind trace.SpanKind) codes.Code {
 	if http.StatusText(code) == "" {
 		return codes.Error
 	}
@@ -886,6 +904,9 @@ func getExpectedCodeForHTTPCode(code int) codes.Code {
 	if category > 0 && category < 4 {
 		return codes.Unset
 	}
+	if spanKind == trace.SpanKindServer && category == 4 {
+		return codes.Unset
+	}
 	return codes.Error
 }
 

semconv/v1.7.0/http.go

@@ -21,6 +21,8 @@ import (
 	"strconv"
 	"strings"
 
+	"go.opentelemetry.io/otel/trace"
+
 	"go.opentelemetry.io/otel/attribute"
 	"go.opentelemetry.io/otel/codes"
 )
@@ -269,6 +271,21 @@ func SpanStatusFromHTTPStatusCode(code int) (codes.Code, string) {
 	return spanCode, ""
 }
 
+// SpanStatusFromHTTPStatusCodeAndSpanKind generates a status code and a message
+// as specified by the OpenTelemetry specification for a span.
+// Exclude 4xx for SERVER to set the appropriate status.
+func SpanStatusFromHTTPStatusCodeAndSpanKind(code int, spanKind trace.SpanKind) (codes.Code, string) {
+	spanCode, valid := validateHTTPStatusCode(code)
+	if !valid {
+		return spanCode, fmt.Sprintf("Invalid HTTP status code %d", code)
+	}
+	category := code / 100
+	if spanKind == trace.SpanKindServer && category == 4 {
+		return codes.Unset, ""
+	}
+	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.

semconv/v1.7.0/http_test.go

@@ -20,6 +20,8 @@ import (
 	"strings"
 	"testing"
 
+	"go.opentelemetry.io/otel/trace"
+
 	"github.com/google/go-cmp/cmp"
 	"github.com/stretchr/testify/assert"
 
@@ -854,7 +856,7 @@ func TestHTTPAttributesFromHTTPStatusCode(t *testing.T) {
 
 func TestSpanStatusFromHTTPStatusCode(t *testing.T) {
 	for code := 0; code < 1000; code++ {
-		expected := getExpectedCodeForHTTPCode(code)
+		expected := getExpectedCodeForHTTPCode(code, trace.SpanKindClient)
 		got, msg := SpanStatusFromHTTPStatusCode(code)
 		assert.Equalf(t, expected, got, "%s vs %s", expected, got)
 
@@ -867,7 +869,24 @@ func TestSpanStatusFromHTTPStatusCode(t *testing.T) {
 	}
 }
 
-func getExpectedCodeForHTTPCode(code int) codes.Code {
+func TestSpanStatusFromHTTPStatusCodeAndSpanKind(t *testing.T) {
+	for code := 0; code < 1000; code++ {
+		expected := getExpectedCodeForHTTPCode(code, trace.SpanKindClient)
+		got, msg := SpanStatusFromHTTPStatusCodeAndSpanKind(code, trace.SpanKindClient)
+		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")
+		}
+	}
+	code, _ := SpanStatusFromHTTPStatusCodeAndSpanKind(400, trace.SpanKindServer)
+	assert.Equalf(t, codes.Unset, code, "message should be set if error cannot be inferred from code")
+}
+
+func getExpectedCodeForHTTPCode(code int, spanKind trace.SpanKind) codes.Code {
 	if http.StatusText(code) == "" {
 		return codes.Error
 	}
@@ -886,6 +905,9 @@ func getExpectedCodeForHTTPCode(code int) codes.Code {
 	if category > 0 && category < 4 {
 		return codes.Unset
 	}
+	if spanKind == trace.SpanKindServer && category == 4 {
+		return codes.Unset
+	}
 	return codes.Error
 }