From ae32ac76d2ebd3628c8ec5694f4f9aec5cd14c37 Mon Sep 17 00:00:00 2001 From: Veera Pirla <17177346+veera83372@users.noreply.github.com> Date: Sat, 6 Mar 2021 03:10:45 +0530 Subject: [PATCH 1/2] Fix #1658 SpanStatus description set only when status code is set to error --- CHANGELOG.md | 1 + sdk/trace/span.go | 7 +++++-- sdk/trace/trace_test.go | 31 ++++++++++++++++++++++++++++++- 3 files changed, 36 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7eb3c8d2bcf..7720a2259ad 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ 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) +- Add `description` to SpanStatus only when `StatusCode` is set to error. (#1662) ### Removed - Removed the exported `SimpleSpanProcessor` and `BatchSpanProcessor` structs. diff --git a/sdk/trace/span.go b/sdk/trace/span.go index 0bb4155b83f..ef976d472a0 100644 --- 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 index 93dde22147c..be8c2bf2a9f 100644 --- a/sdk/trace/trace_test.go +++ b/sdk/trace/trace_test.go @@ -744,6 +744,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{}), @@ -1351,7 +1380,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) From 05b915040aec5c841f3dd713d4ac1ed2c9125f38 Mon Sep 17 00:00:00 2001 From: Veera Pirla <17177346+veera83372@users.noreply.github.com> Date: Sat, 6 Mar 2021 03:18:07 +0530 Subject: [PATCH 2/2] Update CHANGELOG.md Co-authored-by: Tyler Yahn --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7720a2259ad..9b97e094fea 100644 --- 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 + - Add `description` to SpanStatus only when `StatusCode` is set to error. (#1662) + ### Removed - Removed the exported `SimpleSpanProcessor` and `BatchSpanProcessor` structs.