Skip to content

Commit

Permalink
Fix #1658 SpanStatus description set only when status code is set to …
Browse files Browse the repository at this point in the history
…error
  • Loading branch information
mrveera committed Mar 5, 2021
1 parent 7153ef2 commit ae32ac7
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 3 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
7 changes: 5 additions & 2 deletions sdk/trace/span.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}

Expand Down
31 changes: 30 additions & 1 deletion sdk/trace/trace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}),
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit ae32ac7

Please sign in to comment.