diff --git a/CHANGELOG.md b/CHANGELOG.md index 40953d25784..69c08212742 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,11 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - The metric portion of the OpenCensus bridge (`go.opentelemetry.io/otel/bridge/opencensus`) has been reintroduced. (#3192) +### Changed + +- `span.SetStatus` has been updated to comply with the OpenTelemetry specification. + Calls that lower the status are now noops. (#3214) + ## [0.32.0] Revised Metric SDK (Alpha) - 2022-09-18 ### Changed diff --git a/sdk/trace/span.go b/sdk/trace/span.go index 449cf6c2552..9760923f702 100644 --- a/sdk/trace/span.go +++ b/sdk/trace/span.go @@ -189,15 +189,18 @@ func (s *recordingSpan) SetStatus(code codes.Code, description string) { if !s.IsRecording() { return } + s.mu.Lock() + defer s.mu.Unlock() + if s.status.Code > code { + return + } status := Status{Code: code} if code == codes.Error { status.Description = description } - s.mu.Lock() s.status = status - s.mu.Unlock() } // SetAttributes sets attributes of this span. diff --git a/sdk/trace/span_test.go b/sdk/trace/span_test.go index 2441defae71..20148a78702 100644 --- a/sdk/trace/span_test.go +++ b/sdk/trace/span_test.go @@ -22,8 +22,84 @@ import ( "github.com/stretchr/testify/assert" "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/codes" ) +func TestSetStatus(t *testing.T) { + tests := []struct { + name string + span recordingSpan + code codes.Code + description string + expected Status + }{ + { + "Error and description should overwrite Unset", + recordingSpan{}, + codes.Error, + "description", + Status{Code: codes.Error, Description: "description"}, + }, + { + "Ok should overwrite Unset and ignore description", + recordingSpan{}, + codes.Ok, + "description", + Status{Code: codes.Ok}, + }, + { + "Error and description should return error and overwrite description", + recordingSpan{status: Status{Code: codes.Error, Description: "d1"}}, + codes.Error, + "d2", + Status{Code: codes.Error, Description: "d2"}, + }, + { + "Ok should overwrite error and remove description", + recordingSpan{status: Status{Code: codes.Error, Description: "d1"}}, + codes.Ok, + "d2", + Status{Code: codes.Ok}, + }, + { + "Error and description should be ignored when already Ok", + recordingSpan{status: Status{Code: codes.Ok}}, + codes.Error, + "d2", + Status{Code: codes.Ok}, + }, + { + "Ok should be noop when already Ok", + recordingSpan{status: Status{Code: codes.Ok}}, + codes.Ok, + "d2", + Status{Code: codes.Ok}, + }, + { + "Unset should be noop when already Ok", + recordingSpan{status: Status{Code: codes.Ok}}, + codes.Unset, + "d2", + Status{Code: codes.Ok}, + }, + { + "Unset should be noop when already Error", + recordingSpan{status: Status{Code: codes.Error, Description: "d1"}}, + codes.Unset, + "d2", + Status{Code: codes.Error, Description: "d1"}, + }, + } + + for i := range tests { + tc := &tests[i] + t.Run(tc.name, func(t *testing.T) { + tc.span.SetStatus(tc.code, tc.description) + assert.Equal(t, tc.expected, tc.span.status) + }) + } +} + func TestTruncateAttr(t *testing.T) { const key = "key" diff --git a/trace/trace.go b/trace/trace.go index 97f3d83855b..4ff03e86d52 100644 --- a/trace/trace.go +++ b/trace/trace.go @@ -364,8 +364,8 @@ type Span interface { SpanContext() SpanContext // SetStatus sets the status of the Span in the form of a code and a - // description, overriding previous values set. The description is only - // included in a status when the code is for an error. + // description. The description is only included in a status when the code + // is for an error. SetStatus(code codes.Code, description string) // SetName sets the Span name.