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

@@ -63,7 +63,7 @@ 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
+	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
+	// SetStatus sets the status of the span in the form of a code
-	// even after span ends.
+	// and a message.  SetStatus overrides the value of previous
-	SetStatus(codes.Code)
+	// 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

@@ -44,7 +44,8 @@ type Span struct {
 	name          string
 	startTime     time.Time
 	endTime       time.Time
-	status       codes.Code
+	statusCode    codes.Code
+	statusMessage string
 	attributes    map[core.Key]core.Value
 	events        []Event
 	links         map[core.SpanContext][]core.KeyValue
@@ -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 {
+	if cfg.StatusCode != codes.OK {
-		s.SetStatus(cfg.Status)
+		s.SetStatus(cfg.StatusCode, "")
 	}
 
 	errType := reflect.TypeOf(err)
@@ -140,7 +141,7 @@ 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()
 
@@ -148,7 +149,8 @@ func (s *Span) SetStatus(status codes.Code) {
 		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 {
+func (s *Span) StatusCode() codes.Code {
-	return s.status
+	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
+			expStatusCode := codes.Unknown
-			subject.RecordError(ctx, testErr, trace.WithErrorTime(testTime), trace.WithErrorStatus(expStatus))
+			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(codes.OK, "OK")
-				subject.SetStatus(status)
+				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 {
+		if b, ok := value.(bool); ok && b {
-			status := codes.OK
+			s.otelSpan.SetStatus(codes.Unknown, "")
-			if b {
-				status = codes.Unknown
-			}
-			s.otelSpan.SetStatus(status)
 		}
 	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

@@ -35,7 +35,8 @@ import (
 var (
 	ComponentKey     = otelkey.New("component")
 	ServiceKey       = otelkey.New("service")
-	StatusKey    = otelkey.New("status")
+	StatusCodeKey    = otelkey.New("status.code")
+	StatusMessageKey = otelkey.New("status.message")
 	ErrorKey         = otelkey.New("error")
 	NameKey          = otelkey.New("name")
 )
@@ -220,8 +221,8 @@ func (s *MockSpan) IsRecording() bool {
 	return s.recording
 }
 
-func (s *MockSpan) SetStatus(status codes.Code) {
+func (s *MockSpan) SetStatus(code codes.Code, msg string) {
-	s.SetAttributes(NameKey.Uint32(uint32(status)))
+	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 {
+	if cfg.StatusCode != codes.OK {
-		s.SetStatus(cfg.Status)
+		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())
+		trace.SpanFromContext(ctx).SetStatus(s.Code(), s.Message())
-	} else {
-		trace.SpanFromContext(ctx).SetStatus(codes.OK)
 	}
 }

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 (
+require go.opentelemetry.io/otel v0.2.3
-	go.opentelemetry.io/otel v0.2.3
-	google.golang.org/grpc v1.27.1
-)

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.
+		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),
@@ -110,6 +111,7 @@ func TestOtSpanToOtlpSpan_Basic(t *testing.T) {
 					EndTimeUnixnano:   uint64(endTime.Nanosecond()),
 					Status: &tracepb.Status{
 						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)),
+	tags = append(tags,
-		getStringTag("status.message", data.Status.String()),
+		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))
 	}
 

exporters/trace/jaeger/jaeger_test.go

@@ -157,7 +157,7 @@ func Test_spanDataToThrift(t *testing.T) {
 	statusCodeValue := int64(2)
 	doubleValue := 123.456
 	boolTrue := true
-	statusMessage := "Unknown"
+	statusMessage := "this is a problem"
 	spanKind := "client"
 
 	tests := []struct {
@@ -192,7 +192,8 @@ func Test_spanDataToThrift(t *testing.T) {
 				MessageEvents: []export.Event{
 					{Name: eventNameValue, Attributes: []core.KeyValue{key.String("k1", keyValue)}, Time: now},
 				},
-				Status:   codes.Unknown,
+				StatusCode:    codes.Unknown,
+				StatusMessage: statusMessage,
 				SpanKind:      apitrace.SpanKindClient,
 			},
 			want: &gen.Span{

exporters/trace/stdout/stdout_test.go

@@ -61,7 +61,8 @@ func TestExporter_ExportSpan(t *testing.T) {
 			{Name: "bar", Attributes: []core.KeyValue{key.Float64("double", doubleValue)}, Time: now},
 		},
 		SpanKind:      trace.SpanKindInternal,
-		Status:   codes.Unknown,
+		StatusCode:    codes.Unknown,
+		StatusMessage: "interesting",
 	}
 	ex.ExportSpan(context.Background(), testSpan)
 
@@ -109,7 +110,8 @@ func TestExporter_ExportSpan(t *testing.T) {
 		`}` +
 		`],` +
 		`"Links":null,` +
-		`"Status":2,` +
+		`"StatusCode":2,` +
+		`"StatusMessage":"interesting",` +
 		`"HasRemoteParent":false,` +
 		`"DroppedAttributeCount":0,` +
 		`"DroppedMessageEventCount":0,` +

internal/trace/mock_span.go

@@ -47,7 +47,7 @@ func (ms *MockSpan) IsRecording() bool {
 }
 
 // SetStatus does nothing.
-func (ms *MockSpan) SetStatus(status codes.Code) {
+func (ms *MockSpan) SetStatus(status codes.Code, msg string) {
 }
 
 // SetError does nothing.

plugin/httptrace/clienttrace.go

@@ -35,7 +35,6 @@ var (
 	HTTPHeaderMIME = key.New("http.mime")
 	HTTPRemoteAddr = key.New("http.remote")
 	HTTPLocalAddr  = key.New("http.local")
-	MessageKey     = key.New("message")
 )
 
 type clientTracer struct {
@@ -97,8 +96,7 @@ func (ct *clientTracer) end(hook string, err error, attrs ...core.KeyValue) {
 	defer ct.mtx.Unlock()
 	if span, ok := ct.activeHooks[hook]; ok {
 		if err != nil {
-			span.SetStatus(codes.Unknown)
+			span.SetStatus(codes.Unknown, err.Error())
-			span.SetAttributes(MessageKey.String(err.Error()))
 		}
 		span.SetAttributes(attrs...)
 		span.End()
@@ -172,8 +170,7 @@ func (ct *clientTracer) wroteHeaders() {
 
 func (ct *clientTracer) wroteRequest(info httptrace.WroteRequestInfo) {
 	if info.Err != nil {
-		ct.root.SetAttributes(MessageKey.String(info.Err.Error()))
+		ct.root.SetStatus(codes.Unknown, info.Err.Error())
-		ct.root.SetStatus(codes.Unknown)
 	}
 	ct.end("http.send", info.Err)
 }

sdk/export/trace/trace.go

@@ -58,7 +58,8 @@ type SpanData struct {
 	Attributes               []core.KeyValue
 	MessageEvents            []Event
 	Links                    []apitrace.Link
-	Status                   codes.Code
+	StatusCode               codes.Code
+	StatusMessage            string
 	HasRemoteParent          bool
 	DroppedAttributeCount    int
 	DroppedMessageEventCount int

sdk/trace/span.go

@@ -80,7 +80,7 @@ func (s *span) IsRecording() bool {
 	return s.data != nil
 }
 
-func (s *span) SetStatus(status codes.Code) {
+func (s *span) SetStatus(code codes.Code, msg string) {
 	if s == nil {
 		return
 	}
@@ -88,7 +88,8 @@ func (s *span) SetStatus(status codes.Code) {
 		return
 	}
 	s.mu.Lock()
-	s.data.Status = status
+	s.data.StatusCode = code
+	s.data.StatusMessage = msg
 	s.mu.Unlock()
 }
 
@@ -150,8 +151,8 @@ func (s *span) RecordError(ctx context.Context, err error, opts ...apitrace.Erro
 		cfg.Timestamp = time.Now()
 	}
 
-	if cfg.Status != codes.OK {
+	if cfg.StatusCode != codes.OK {
-		s.SetStatus(cfg.Status)
+		s.SetStatus(cfg.StatusCode, "")
 	}
 
 	errType := reflect.TypeOf(err)

sdk/trace/trace_test.go

@@ -559,7 +559,7 @@ func TestSetSpanStatus(t *testing.T) {
 	tp, _ := NewProvider(WithSyncer(te))
 
 	span := startSpan(tp, "SpanStatus")
-	span.SetStatus(codes.Canceled)
+	span.SetStatus(codes.Canceled, "canceled")
 	got, err := endSpan(te, span)
 	if err != nil {
 		t.Fatal(err)
@@ -573,7 +573,8 @@ func TestSetSpanStatus(t *testing.T) {
 		ParentSpanID:    sid,
 		Name:            "span0",
 		SpanKind:        apitrace.SpanKindInternal,
-		Status:          codes.Canceled,
+		StatusCode:      codes.Canceled,
+		StatusMessage:   "canceled",
 		HasRemoteParent: true,
 	}
 	if diff := cmpDiff(got, want); diff != "" {
@@ -957,7 +958,8 @@ func TestRecordErrorWithStatus(t *testing.T) {
 		ParentSpanID:    sid,
 		Name:            "span0",
 		SpanKind:        apitrace.SpanKindInternal,
-		Status:          codes.Unknown,
+		StatusCode:      codes.Unknown,
+		StatusMessage:   "",
 		HasRemoteParent: true,
 		MessageEvents: []export.Event{
 			{
@@ -996,7 +998,8 @@ func TestRecordErrorNil(t *testing.T) {
 		Name:            "span0",
 		SpanKind:        apitrace.SpanKindInternal,
 		HasRemoteParent: true,
-		Status:          codes.OK,
+		StatusCode:      codes.OK,
+		StatusMessage:   "",
 	}
 	if diff := cmpDiff(got, want); diff != "" {
 		t.Errorf("SpanErrorOptions: -got +want %s", diff)