[translator/jaeger] For HTTP status codes in the 4xx range span status MUST be left unset in case of SpanKind.SERVER

.chloggen/let_statuscode_unset_in_case_of_spankind_server_error.yaml

@@ -0,0 +1,11 @@
+# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
+change_type: bug_fix
+
+# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
+component: jaegertranslator
+
+# A brief description of the change.  Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: For HTTP status codes in the 4xx range span status MUST be left unset in case of SpanKind.SERVER and MUST be set to Error in case of SpanKind.CLIENT.
+
+# One or more tracking issues related to the change
+issues: [8273]

pkg/translator/jaeger/jaegerproto_to_traces.go

@@ -222,7 +222,7 @@ func jSpanToInternal(span *model.Span, dest ptrace.Span) {
 	attrs := dest.Attributes()
 	attrs.EnsureCapacity(len(span.Tags))
 	jTagsToInternalAttributes(span.Tags, attrs)
-	setInternalSpanStatus(attrs, dest.Status())
+	setInternalSpanStatus(attrs, dest)
 	if spanKindAttr, ok := attrs.Get(tracetranslator.TagSpanKind); ok {
 		dest.SetKind(jSpanKindToInternal(spanKindAttr.Str()))
 		attrs.Remove(tracetranslator.TagSpanKind)
@@ -258,12 +258,13 @@ func jTagsToInternalAttributes(tags []model.KeyValue, dest pcommon.Map) {
 	}
 }
 
-func setInternalSpanStatus(attrs pcommon.Map, dest ptrace.Status) {
+func setInternalSpanStatus(attrs pcommon.Map, span ptrace.Span) {
+	dest := span.Status()
 	statusCode := ptrace.StatusCodeUnset
 	statusMessage := ""
 	statusExists := false
 
-	if errorVal, ok := attrs.Get(tracetranslator.TagError); ok {
+	if errorVal, ok := attrs.Get(tracetranslator.TagError); ok && errorVal.Type() == pcommon.ValueTypeBool {
 		if errorVal.Bool() {
 			statusCode = ptrace.StatusCodeError
 			attrs.Remove(tracetranslator.TagError)
@@ -302,7 +303,7 @@ func setInternalSpanStatus(attrs pcommon.Map, dest ptrace.Status) {
 		// Fallback to introspecting if this span represents a failed HTTP
 		// request or response, but again, only do so if the `error` tag was
 		// not set to true and no explicit status was sent.
-		if code, err := getStatusCodeFromHTTPStatusAttr(httpCodeAttr); err == nil {
+		if code, err := getStatusCodeFromHTTPStatusAttr(httpCodeAttr, span.Kind()); err == nil {
 			if code != ptrace.StatusCodeUnset {
 				statusExists = true
 				statusCode = code
@@ -353,12 +354,25 @@ func codeFromAttr(attrVal pcommon.Value) (int64, error) {
 	return val, nil
 }
 
-func getStatusCodeFromHTTPStatusAttr(attrVal pcommon.Value) (ptrace.StatusCode, error) {
+func getStatusCodeFromHTTPStatusAttr(attrVal pcommon.Value, kind ptrace.SpanKind) (ptrace.StatusCode, error) {
 	statusCode, err := codeFromAttr(attrVal)
 	if err != nil {
 		return ptrace.StatusCodeUnset, err
 	}
 
+	// For HTTP status codes in the 4xx range span status MUST be left unset
+	// in case of SpanKind.SERVER and MUST be set to Error in case of SpanKind.CLIENT.
+	// For HTTP status codes in the 5xx range, as well as any other code the client
+	// failed to interpret, span status MUST be set to Error.
+	if statusCode >= 400 && statusCode < 500 {
+		switch kind {
+		case ptrace.SpanKindClient:
+			return ptrace.StatusCodeError, nil
+		case ptrace.SpanKindServer:
+			return ptrace.StatusCodeUnset, nil
+		}
+	}
+
 	return tracetranslator.StatusCodeFromHTTP(statusCode), nil
 }
 

pkg/translator/jaeger/jaegerproto_to_traces_test.go

@@ -93,41 +93,52 @@ func TestGetStatusCodeFromHTTPStatusAttr(t *testing.T) {
 	tests := []struct {
 		name string
 		attr pcommon.Value
+		kind ptrace.SpanKind
 		code ptrace.StatusCode
 	}{
 		{
 			name: "string-unknown",
 			attr: pcommon.NewValueStr("10"),
+			kind: ptrace.SpanKindClient,
 			code: ptrace.StatusCodeError,
 		},
 
 		{
 			name: "string-ok",
 			attr: pcommon.NewValueStr("101"),
+			kind: ptrace.SpanKindClient,
 			code: ptrace.StatusCodeUnset,
 		},
 
 		{
 			name: "int-not-found",
 			attr: pcommon.NewValueInt(404),
+			kind: ptrace.SpanKindClient,
 			code: ptrace.StatusCodeError,
 		},
+		{
+			name: "int-not-found-client-span",
+			attr: pcommon.NewValueInt(404),
+			kind: ptrace.SpanKindServer,
+			code: ptrace.StatusCodeUnset,
+		},
 		{
 			name: "int-invalid-arg",
 			attr: pcommon.NewValueInt(408),
+			kind: ptrace.SpanKindClient,
 			code: ptrace.StatusCodeError,
 		},
-
 		{
 			name: "int-internal",
 			attr: pcommon.NewValueInt(500),
+			kind: ptrace.SpanKindClient,
 			code: ptrace.StatusCodeError,
 		},
 	}
 
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			code, err := getStatusCodeFromHTTPStatusAttr(test.attr)
+			code, err := getStatusCodeFromHTTPStatusAttr(test.attr, test.kind)
 			assert.NoError(t, err)
 			assert.Equal(t, test.code, code)
 		})
@@ -346,6 +357,7 @@ func TestSetInternalSpanStatus(t *testing.T) {
 		name             string
 		attrs            map[string]interface{}
 		status           ptrace.Status
+		kind             ptrace.SpanKind
 		attrsModifiedLen int // Length of attributes map after dropping converted fields
 	}{
 		{
@@ -416,14 +428,26 @@ func TestSetInternalSpanStatus(t *testing.T) {
 			status:           errorStatus,
 			attrsModifiedLen: 1,
 		},
+		{
+			name: "the 4xx range span status MUST be left unset in case of SpanKind.SERVER",
+			kind: ptrace.SpanKindServer,
+			attrs: map[string]interface{}{
+				tracetranslator.TagError:            false,
+				conventions.AttributeHTTPStatusCode: 404,
+			},
+			status:           emptyStatus,
+			attrsModifiedLen: 2,
+		},
 	}
 
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			status := ptrace.NewStatus()
+			span := ptrace.NewSpan()
+			span.SetKind(test.kind)
+			status := span.Status()
 			attrs := pcommon.NewMap()
 			attrs.FromRaw(test.attrs)
-			setInternalSpanStatus(attrs, status)
+			setInternalSpanStatus(attrs, span)
 			assert.EqualValues(t, test.status, status)
 			assert.Equal(t, test.attrsModifiedLen, attrs.Len())
 		})

pkg/translator/jaeger/jaegerthrift_to_traces.go

@@ -106,7 +106,7 @@ func jThriftSpanToInternal(span *jaeger.Span, dest ptrace.Span) {
 	attrs := dest.Attributes()
 	attrs.EnsureCapacity(len(span.Tags))
 	jThriftTagsToInternalAttributes(span.Tags, attrs)
-	setInternalSpanStatus(attrs, dest.Status())
+	setInternalSpanStatus(attrs, dest)
 	if spanKindAttr, ok := attrs.Get(tracetranslator.TagSpanKind); ok {
 		dest.SetKind(jSpanKindToInternal(spanKindAttr.Str()))
 		attrs.Remove(tracetranslator.TagSpanKind)