SpanStatus description set only when status code is set to Error (#1662

) * Fix #1658 SpanStatus description set only when status code is set to error * Update CHANGELOG.md Co-authored-by: Tyler Yahn <[email protected]> Co-authored-by: Tyler Yahn <[email protected]>
open-telemetry · Mar 8, 2021 · a7f7aba · a7f7aba
1 parent 05252f4
commit a7f7aba
Show file tree

Hide file tree

Showing 3 changed files with 39 additions and 3 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -11,7 +11,11 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 ## Added
 
 - Added `Marshler` config option to `otlphttp` to enable otlp over json or protobufs. (#1586)
+
+### Changed
+
 - Added non-empty string check for trace `Attribute` keys. (#1659)
+- Add `description` to SpanStatus only when `StatusCode` is set to error. (#1662)
 
 ### Removed
 

diff --git a/sdk/trace/span.go b/sdk/trace/span.go
@@ -170,14 +170,17 @@ func (s *span) IsRecording() bool {
 
 // SetStatus sets the status of this span in the form of a code and a
 // message. This overrides the existing value of this span's status if one
-// exists. If this span is not being recorded than this method does nothing.
+// exists. Message will be set only if status is error. If this span is not being
+// recorded than this method does nothing.
 func (s *span) SetStatus(code codes.Code, msg string) {
 	if !s.IsRecording() {
 		return
 	}
 	s.mu.Lock()
 	s.statusCode = code
-	s.statusMessage = msg
+	if code == codes.Error {
+		s.statusMessage = msg
+	}
 	s.mu.Unlock()
 }
 

diff --git a/sdk/trace/trace_test.go b/sdk/trace/trace_test.go
@@ -789,6 +789,35 @@ func TestSetSpanStatus(t *testing.T) {
 	}
 }
 
+func TestSetSpanStatusWithoutMessageWhenStatusIsNotError(t *testing.T) {
+	te := NewTestExporter()
+	tp := NewTracerProvider(WithSyncer(te), WithResource(resource.Empty()))
+
+	span := startSpan(tp, "SpanStatus")
+	span.SetStatus(codes.Ok, "This message will be ignored")
+	got, err := endSpan(te, span)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	want := &export.SpanSnapshot{
+		SpanContext: trace.SpanContext{
+			TraceID:    tid,
+			TraceFlags: 0x1,
+		},
+		ParentSpanID:           sid,
+		Name:                   "span0",
+		SpanKind:               trace.SpanKindInternal,
+		StatusCode:             codes.Ok,
+		StatusMessage:          "",
+		HasRemoteParent:        true,
+		InstrumentationLibrary: instrumentation.Library{Name: "SpanStatus"},
+	}
+	if diff := cmpDiff(got, want); diff != "" {
+		t.Errorf("SetSpanStatus: -got +want %s", diff)
+	}
+}
+
 func cmpDiff(x, y interface{}) string {
 	return cmp.Diff(x, y,
 		cmp.AllowUnexported(attribute.Value{}),
@@ -1396,7 +1425,7 @@ func TestReadOnlySpan(t *testing.T) {
 	assert.Equal(t, kv.Key, ro.Events()[0].Attributes[0].Key)
 	assert.Equal(t, kv.Value, ro.Events()[0].Attributes[0].Value)
 	assert.Equal(t, codes.Ok, ro.StatusCode())
-	assert.Equal(t, "foo", ro.StatusMessage())
+	assert.Equal(t, "", ro.StatusMessage())
 	assert.Equal(t, "ReadOnlySpan", ro.InstrumentationLibrary().Name)
 	assert.Equal(t, "3", ro.InstrumentationLibrary().Version)
 	assert.Equal(t, kv.Key, ro.Resource().Attributes()[0].Key)