Add status message parameter (#524)

* Add status message parameter

* Cleanups around use of codes.OK

* Update for status message

api/testharness/harness.go

@@ -323,7 +323,7 @@ func (h *Harness) testSpan(tracerFactory func() trace.Tracer) {
 			span.AddEventWithTimestamp(context.Background(), time.Now(), "test event")
 		},
 		"#SetStatus": func(span trace.Span) {
-			span.SetStatus(codes.Internal)
+			span.SetStatus(codes.Internal, "internal")
 		},
 		"#SetName": func(span trace.Span) {
 			span.SetName("new name")

api/trace/api.go

@@ -62,8 +62,8 @@ func WithEndTime(t time.Time) EndOption {
 
 // ErrorConfig provides options to set properties of an error event at the time it is recorded.
 type ErrorConfig struct {
-	Timestamp time.Time
-	Status    codes.Code
+	Timestamp  time.Time
+	StatusCode codes.Code
 }
 
 // ErrorOption applies changes to ErrorConfig that sets options when an error event is recorded.
@@ -79,7 +79,7 @@ func WithErrorTime(t time.Time) ErrorOption {
 // WithErrorStatus indicates the span status that should be set when recording an error event.
 func WithErrorStatus(s codes.Code) ErrorOption {
 	return func(c *ErrorConfig) {
-		c.Status = s
+		c.StatusCode = s
 	}
 }
 
@@ -107,9 +107,14 @@ type Span interface {
 	// even after the span ends.
 	SpanContext() core.SpanContext
 
-	// SetStatus sets the status of the span. The status of the span can be updated
-	// even after span ends.
-	SetStatus(codes.Code)
+	// SetStatus sets the status of the span in the form of a code
+	// and a message.  SetStatus overrides the value of previous
+	// calls to SetStatus on the Span.
+	//
+	// The default span status is OK, so it is not necessary to
+	// explicitly set an OK status on successful Spans unless it
+	// is to add an OK message or to override a previous status on the Span.
+	SetStatus(codes.Code, string)
 
 	// SetName sets the name of the span.
 	SetName(name string)

api/trace/context_test.go

@@ -68,7 +68,7 @@ func (mockSpan) IsRecording() bool {
 }
 
 // SetStatus does nothing.
-func (mockSpan) SetStatus(status codes.Code) {
+func (mockSpan) SetStatus(status codes.Code, msg string) {
 }
 
 // SetName does nothing.

api/trace/noop_span.go

@@ -39,7 +39,7 @@ func (NoopSpan) IsRecording() bool {
 }
 
 // SetStatus does nothing.
-func (NoopSpan) SetStatus(status codes.Code) {
+func (NoopSpan) SetStatus(status codes.Code, msg string) {
 }
 
 // SetError does nothing.

api/trace/testtrace/span.go

@@ -36,18 +36,19 @@ const (
 var _ trace.Span = (*Span)(nil)
 
 type Span struct {
-	lock         *sync.RWMutex
-	tracer       *Tracer
-	spanContext  core.SpanContext
-	parentSpanID core.SpanID
-	ended        bool
-	name         string
-	startTime    time.Time
-	endTime      time.Time
-	status       codes.Code
-	attributes   map[core.Key]core.Value
-	events       []Event
-	links        map[core.SpanContext][]core.KeyValue
+	lock          *sync.RWMutex
+	tracer        *Tracer
+	spanContext   core.SpanContext
+	parentSpanID  core.SpanID
+	ended         bool
+	name          string
+	startTime     time.Time
+	endTime       time.Time
+	statusCode    codes.Code
+	statusMessage string
+	attributes    map[core.Key]core.Value
+	events        []Event
+	links         map[core.SpanContext][]core.KeyValue
 }
 
 func (s *Span) Tracer() trace.Tracer {
@@ -91,8 +92,8 @@ func (s *Span) RecordError(ctx context.Context, err error, opts ...trace.ErrorOp
 		cfg.Timestamp = time.Now()
 	}
 
-	if cfg.Status != codes.OK {
-		s.SetStatus(cfg.Status)
+	if cfg.StatusCode != codes.OK {
+		s.SetStatus(cfg.StatusCode, "")
 	}
 
 	errType := reflect.TypeOf(err)
@@ -140,15 +141,16 @@ func (s *Span) SpanContext() core.SpanContext {
 	return s.spanContext
 }
 
-func (s *Span) SetStatus(status codes.Code) {
+func (s *Span) SetStatus(code codes.Code, msg string) {
 	s.lock.Lock()
 	defer s.lock.Unlock()
 
 	if s.ended {
 		return
 	}
 
-	s.status = status
+	s.statusCode = code
+	s.statusMessage = msg
 }
 
 func (s *Span) SetName(name string) {
@@ -245,6 +247,12 @@ func (s *Span) Ended() bool {
 // Status returns the status most recently set on the Span,
 // or codes.OK if no status has been explicitly set.
 // It cannot be changed after End has been called on the Span.
-func (s *Span) Status() codes.Code {
-	return s.status
+func (s *Span) StatusCode() codes.Code {
+	return s.statusCode
+}
+
+// StatusMessage returns the status message most recently set on the
+// Span or the empty string if no status mesaage was set.
+func (s *Span) StatusMessage() string {
+	return s.statusMessage
 }

api/trace/testtrace/span_test.go

@@ -17,6 +17,7 @@ package testtrace_test
 import (
 	"context"
 	"errors"
+	"fmt"
 	"sync"
 	"testing"
 	"time"
@@ -164,7 +165,8 @@ func TestSpan(t *testing.T) {
 					},
 				}}
 				e.Expect(subject.Events()).ToEqual(expectedEvents)
-				e.Expect(subject.Status()).ToEqual(codes.OK)
+				e.Expect(subject.StatusCode()).ToEqual(codes.OK)
+				e.Expect(subject.StatusMessage()).ToEqual("")
 			}
 		})
 
@@ -182,8 +184,8 @@ func TestSpan(t *testing.T) {
 			errMsg := "test error message"
 			testErr := ottest.NewTestError(errMsg)
 			testTime := time.Now()
-			expStatus := codes.Unknown
-			subject.RecordError(ctx, testErr, trace.WithErrorTime(testTime), trace.WithErrorStatus(expStatus))
+			expStatusCode := codes.Unknown
+			subject.RecordError(ctx, testErr, trace.WithErrorTime(testTime), trace.WithErrorStatus(expStatusCode))
 
 			expectedEvents := []testtrace.Event{{
 				Timestamp: testTime,
@@ -194,7 +196,7 @@ func TestSpan(t *testing.T) {
 				},
 			}}
 			e.Expect(subject.Events()).ToEqual(expectedEvents)
-			e.Expect(subject.Status()).ToEqual(expStatus)
+			e.Expect(subject.StatusCode()).ToEqual(expStatusCode)
 		})
 
 		t.Run("cannot be set after the span has ended", func(t *testing.T) {
@@ -527,11 +529,11 @@ func TestSpan(t *testing.T) {
 			subject, ok := span.(*testtrace.Span)
 			e.Expect(ok).ToBeTrue()
 
-			e.Expect(subject.Status()).ToEqual(codes.OK)
+			e.Expect(subject.StatusCode()).ToEqual(codes.OK)
 
 			subject.End()
 
-			e.Expect(subject.Status()).ToEqual(codes.OK)
+			e.Expect(subject.StatusCode()).ToEqual(codes.OK)
 		})
 
 		statuses := []codes.Code{
@@ -566,10 +568,11 @@ func TestSpan(t *testing.T) {
 				subject, ok := span.(*testtrace.Span)
 				e.Expect(ok).ToBeTrue()
 
-				subject.SetStatus(codes.OK)
-				subject.SetStatus(status)
+				subject.SetStatus(codes.OK, "OK")
+				subject.SetStatus(status, "Yo!")
 
-				e.Expect(subject.Status()).ToEqual(status)
+				e.Expect(subject.StatusCode()).ToEqual(status)
+				e.Expect(subject.StatusMessage()).ToEqual("Yo!")
 			})
 
 			t.Run("cannot be changed after the span has been ended", func(t *testing.T) {
@@ -585,11 +588,12 @@ func TestSpan(t *testing.T) {
 
 				originalStatus := codes.OK
 
-				subject.SetStatus(originalStatus)
+				subject.SetStatus(originalStatus, "OK")
 				subject.End()
-				subject.SetStatus(status)
+				subject.SetStatus(status, fmt.Sprint(status))
 
-				e.Expect(subject.Status()).ToEqual(originalStatus)
+				e.Expect(subject.StatusCode()).ToEqual(originalStatus)
+				e.Expect(subject.StatusMessage()).ToEqual("OK")
 			})
 		}
 	})

bridge/opentracing/bridge.go

@@ -124,12 +124,8 @@ func (s *bridgeSpan) SetTag(key string, value interface{}) ot.Span {
 	case string(otext.SpanKind):
 		// TODO: Should we ignore it?
 	case string(otext.Error):
-		if b, ok := value.(bool); ok {
-			status := codes.OK
-			if b {
-				status = codes.Unknown
-			}
-			s.otelSpan.SetStatus(status)
+		if b, ok := value.(bool); ok && b {
+			s.otelSpan.SetStatus(codes.Unknown, "")
 		}
 	default:
 		s.otelSpan.SetAttributes(otTagToOtelCoreKeyValue(key, value))
@@ -330,7 +326,7 @@ func (t *BridgeTracer) StartSpan(operationName string, opts ...ot.StartSpanOptio
 		})
 	}
 	if hadTrueErrorTag {
-		otelSpan.SetStatus(codes.Unknown)
+		otelSpan.SetStatus(codes.Unknown, "")
 	}
 	// One does not simply pass a concrete pointer to function
 	// that takes some interface. In case of passing nil concrete

bridge/opentracing/internal/mock.go

@@ -33,11 +33,12 @@ import (
 )
 
 var (
-	ComponentKey = otelkey.New("component")
-	ServiceKey   = otelkey.New("service")
-	StatusKey    = otelkey.New("status")
-	ErrorKey     = otelkey.New("error")
-	NameKey      = otelkey.New("name")
+	ComponentKey     = otelkey.New("component")
+	ServiceKey       = otelkey.New("service")
+	StatusCodeKey    = otelkey.New("status.code")
+	StatusMessageKey = otelkey.New("status.message")
+	ErrorKey         = otelkey.New("error")
+	NameKey          = otelkey.New("name")
 )
 
 type MockContextKeyValue struct {
@@ -220,8 +221,8 @@ func (s *MockSpan) IsRecording() bool {
 	return s.recording
 }
 
-func (s *MockSpan) SetStatus(status codes.Code) {
-	s.SetAttributes(NameKey.Uint32(uint32(status)))
+func (s *MockSpan) SetStatus(code codes.Code, msg string) {
+	s.SetAttributes(StatusCodeKey.Uint32(uint32(code)), StatusMessageKey.String(msg))
 }
 
 func (s *MockSpan) SetName(name string) {
@@ -279,8 +280,8 @@ func (s *MockSpan) RecordError(ctx context.Context, err error, opts ...oteltrace
 		cfg.Timestamp = time.Now()
 	}
 
-	if cfg.Status != codes.OK {
-		s.SetStatus(cfg.Status)
+	if cfg.StatusCode != codes.OK {
+		s.SetStatus(cfg.StatusCode, "")
 	}
 
 	s.AddEventWithTimestamp(ctx, cfg.Timestamp, "error",

example/grpc/middleware/tracing/tracing.go

@@ -22,7 +22,6 @@ import (
 	"go.opentelemetry.io/otel/plugin/grpctrace"
 
 	"google.golang.org/grpc"
-	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/metadata"
 	"google.golang.org/grpc/status"
 
@@ -81,8 +80,6 @@ func UnaryClientInterceptor(ctx context.Context, method string, req, reply inter
 func setTraceStatus(ctx context.Context, err error) {
 	if err != nil {
 		s, _ := status.FromError(err)
-		trace.SpanFromContext(ctx).SetStatus(s.Code())
-	} else {
-		trace.SpanFromContext(ctx).SetStatus(codes.OK)
+		trace.SpanFromContext(ctx).SetStatus(s.Code(), s.Message())
 	}
 }

example/http/client/client.go

@@ -23,12 +23,9 @@ import (
 	"net/http"
 	"time"
 
-	"google.golang.org/grpc/codes"
-
 	"go.opentelemetry.io/otel/api/correlation"
 	"go.opentelemetry.io/otel/api/global"
 	"go.opentelemetry.io/otel/api/key"
-	"go.opentelemetry.io/otel/api/trace"
 	"go.opentelemetry.io/otel/exporters/trace/stdout"
 	"go.opentelemetry.io/otel/plugin/httptrace"
 	sdktrace "go.opentelemetry.io/otel/sdk/trace"
@@ -77,7 +74,6 @@ func main() {
 			}
 			body, err = ioutil.ReadAll(res.Body)
 			_ = res.Body.Close()
-			trace.SpanFromContext(ctx).SetStatus(codes.OK)
 
 			return err
 		})

example/http/go.mod

@@ -4,7 +4,4 @@ go 1.13
 
 replace go.opentelemetry.io/otel => ../..
 
-require (
-	go.opentelemetry.io/otel v0.2.3
-	google.golang.org/grpc v1.27.1
-)
+require go.opentelemetry.io/otel v0.2.3

exporters/otlp/transform_spans.go

@@ -37,7 +37,7 @@ func otSpanToProtoSpan(sd *export.SpanData) *tracepb.Span {
 		TraceId:           sd.SpanContext.TraceID[:],
 		SpanId:            sd.SpanContext.SpanID[:],
 		ParentSpanId:      sd.ParentSpanID[:],
-		Status:            otStatusToProtoStatus(sd.Status),
+		Status:            otStatusToProtoStatus(sd.StatusCode, sd.StatusMessage),
 		StartTimeUnixnano: uint64(sd.StartTime.Nanosecond()),
 		EndTimeUnixnano:   uint64(sd.EndTime.Nanosecond()),
 		Links:             otLinksToProtoLinks(sd.Links),
@@ -52,10 +52,10 @@ func otSpanToProtoSpan(sd *export.SpanData) *tracepb.Span {
 	}
 }
 
-func otStatusToProtoStatus(status codes.Code) *tracepb.Status {
+func otStatusToProtoStatus(status codes.Code, message string) *tracepb.Status {
 	return &tracepb.Status{
-		Code: tracepb.Status_StatusCode(status),
-		// TODO (rghetia) : Add Status Message: when supported.
+		Code:    tracepb.Status_StatusCode(status),
+		Message: message,
 	}
 }
 

exporters/otlp/transform_spans_test.go

@@ -91,7 +91,8 @@ func TestOtSpanToOtlpSpan_Basic(t *testing.T) {
 							},
 						},
 					},
-					Status:          codes.Internal,
+					StatusCode:      codes.Internal,
+					StatusMessage:   "utterly unrecognized",
 					HasRemoteParent: true,
 					Attributes: []core.KeyValue{
 						core.Key("timeout_ns").Int64(12e9),
@@ -109,7 +110,8 @@ func TestOtSpanToOtlpSpan_Basic(t *testing.T) {
 					StartTimeUnixnano: uint64(startTime.Nanosecond()),
 					EndTimeUnixnano:   uint64(endTime.Nanosecond()),
 					Status: &tracepb.Status{
-						Code: 13,
+						Code:    13,
+						Message: "utterly unrecognized",
 					},
 					Events: []*tracepb.Span_Event{
 						{

exporters/trace/jaeger/jaeger.go

@@ -157,14 +157,15 @@ func spanDataToThrift(data *export.SpanData) *gen.Span {
 		}
 	}
 
-	tags = append(tags, getInt64Tag("status.code", int64(data.Status)),
-		getStringTag("status.message", data.Status.String()),
+	tags = append(tags,
+		getInt64Tag("status.code", int64(data.StatusCode)),
+		getStringTag("status.message", data.StatusMessage),
 		getStringTag("span.kind", data.SpanKind.String()),
 	)
 
 	// Ensure that if Status.Code is not OK, that we set the "error" tag on the Jaeger span.
 	// See Issue https://github.com/census-instrumentation/opencensus-go/issues/1041
-	if data.Status != codes.OK {
+	if data.StatusCode != codes.OK {
 		tags = append(tags, getBoolTag("error", true))
 	}