Change SpanKind type an integer type (#288)

* Change SpanKind to an integer

* Take suggestion

* Use the helper from the bridge

api/trace/api.go

@@ -142,16 +142,57 @@ type Link struct {
 
 // SpanKind represents the role of a Span inside a Trace. Often, this defines how a Span
 // will be processed and visualized by various backends.
-type SpanKind string
+type SpanKind int
 
 const (
-	SpanKindInternal SpanKind = "internal"
-	SpanKindServer   SpanKind = "server"
-	SpanKindClient   SpanKind = "client"
-	SpanKindProducer SpanKind = "producer"
-	SpanKindConsumer SpanKind = "consumer"
+	// As a convenience, these match the proto definition, see
+	// opentelemetry/proto/trace/v1/trace.proto
+	//
+	// The unspecified value is not a valid `SpanKind`.  Use
+	// `ValidateSpanKind()` to coerce a span kind to a valid
+	// value.
+	SpanKindUnspecified SpanKind = 0
+	SpanKindInternal    SpanKind = 1
+	SpanKindServer      SpanKind = 2
+	SpanKindClient      SpanKind = 3
+	SpanKindProducer    SpanKind = 4
+	SpanKindConsumer    SpanKind = 5
 )
 
+// ValidateSpanKind returns a valid span kind value.  This will coerce
+// invalid values into the default value, SpanKindInternal.
+func ValidateSpanKind(spanKind SpanKind) SpanKind {
+	switch spanKind {
+	case SpanKindInternal,
+		SpanKindServer,
+		SpanKindClient,
+		SpanKindProducer,
+		SpanKindConsumer:
+		// valid
+		return spanKind
+	default:
+		return SpanKindInternal
+	}
+}
+
+// String returns the specified name of the SpanKind in lower-case.
+func (sk SpanKind) String() string {
+	switch sk {
+	case SpanKindInternal:
+		return "internal"
+	case SpanKindServer:
+		return "server"
+	case SpanKindClient:
+		return "client"
+	case SpanKindProducer:
+		return "producer"
+	case SpanKindConsumer:
+		return "consumer"
+	default:
+		return "unspecified"
+	}
+}
+
 // WithStartTime sets the start time of the span to provided time t, when it is started.
 // In absence of this option, wall clock time is used as start time.
 // This option is typically used when starting of the span is delayed.

bridge/opentracing/bridge.go

@@ -385,8 +385,17 @@ func otTagsToOtelAttributesKindAndError(tags map[string]interface{}) ([]otelcore
 	for k, v := range tags {
 		switch k {
 		case string(otext.SpanKind):
-			if sk, ok := v.(string); ok {
-				kind = oteltrace.SpanKind(sk)
+			if s, ok := v.(string); ok {
+				switch strings.ToLower(s) {
+				case "client":
+					kind = oteltrace.SpanKindClient
+				case "server":
+					kind = oteltrace.SpanKindServer
+				case "producer":
+					kind = oteltrace.SpanKindProducer
+				case "consumer":
+					kind = oteltrace.SpanKindConsumer
+				}
 			}
 		case string(otext.Error):
 			if b, ok := v.(bool); ok && b {

bridge/opentracing/internal/mock.go

@@ -84,10 +84,6 @@ func (t *MockTracer) Start(ctx context.Context, name string, opts ...oteltrace.S
 	if startTime.IsZero() {
 		startTime = time.Now()
 	}
-	spanKind := spanOpts.SpanKind
-	if spanKind == "" {
-		spanKind = oteltrace.SpanKindInternal
-	}
 	spanContext := otelcore.SpanContext{
 		TraceID:    t.getTraceID(ctx, &spanOpts),
 		SpanID:     t.getSpanID(),
@@ -105,7 +101,7 @@ func (t *MockTracer) Start(ctx context.Context, name string, opts ...oteltrace.S
 		EndTime:      time.Time{},
 		ParentSpanID: t.getParentSpanID(ctx, &spanOpts),
 		Events:       nil,
-		SpanKind:     spanKind,
+		SpanKind:     oteltrace.ValidateSpanKind(spanOpts.SpanKind),
 	}
 	if !migration.SkipContextSetup(ctx) {
 		ctx = oteltrace.SetCurrentSpan(ctx, span)

bridge/opentracing/mix_test.go

@@ -20,8 +20,6 @@ import (
 	"testing"
 
 	ot "github.com/opentracing/opentracing-go"
-	//otext "github.com/opentracing/opentracing-go/ext"
-	//otlog "github.com/opentracing/opentracing-go/log"
 
 	otelcore "go.opentelemetry.io/otel/api/core"
 	oteltrace "go.opentelemetry.io/otel/api/trace"

exporter/trace/stdout/stdout_test.go

@@ -74,7 +74,7 @@ func TestExporter_ExportSpan(t *testing.T) {
 		`"TraceID":"0102030405060708090a0b0c0d0e0f10",` +
 		`"SpanID":"0102030405060708","TraceFlags":0},` +
 		`"ParentSpanID":"0000000000000000",` +
-		`"SpanKind":"internal",` +
+		`"SpanKind":1,` +
 		`"Name":"/foo",` +
 		`"StartTime":` + string(expectedSerializedNow) + "," +
 		`"EndTime":` + string(expectedSerializedNow) + "," +

sdk/trace/span.go

@@ -321,14 +321,10 @@ func startSpanInternal(tr *tracer, name string, parent core.SpanContext, remoteP
 	if startTime.IsZero() {
 		startTime = time.Now()
 	}
-	sk := o.SpanKind
-	if sk == "" {
-		sk = apitrace.SpanKindInternal
-	}
 	span.data = &export.SpanData{
 		SpanContext:     span.spanContext,
 		StartTime:       startTime,
-		SpanKind:        sk,
+		SpanKind:        apitrace.ValidateSpanKind(o.SpanKind),
 		Name:            name,
 		HasRemoteParent: remoteParent,
 	}

sdk/trace/trace_test.go

@@ -285,8 +285,6 @@ func TestStartSpanWithFollowsFrom(t *testing.T) {
 	}
 }
 
-// TODO: [rghetia] Equivalent of SpanKind Test.
-
 func TestSetSpanAttributesOnStart(t *testing.T) {
 	te := &testExporter{}
 	tp, _ := NewProvider(WithSyncer(te))
@@ -306,7 +304,7 @@ func TestSetSpanAttributesOnStart(t *testing.T) {
 		Attributes: []core.KeyValue{
 			key.String("key1", "value1"),
 		},
-		SpanKind:        "internal",
+		SpanKind:        apitrace.SpanKindInternal,
 		HasRemoteParent: true,
 	}
 	if diff := cmpDiff(got, want); diff != "" {
@@ -334,7 +332,7 @@ func TestSetSpanAttributes(t *testing.T) {
 		Attributes: []core.KeyValue{
 			key.String("key1", "value1"),
 		},
-		SpanKind:        "internal",
+		SpanKind:        apitrace.SpanKindInternal,
 		HasRemoteParent: true,
 	}
 	if diff := cmpDiff(got, want); diff != "" {
@@ -368,7 +366,7 @@ func TestSetSpanAttributesOverLimit(t *testing.T) {
 			key.Bool("key1", false),
 			key.Int64("key4", 4),
 		},
-		SpanKind:              "internal",
+		SpanKind:              apitrace.SpanKindInternal,
 		HasRemoteParent:       true,
 		DroppedAttributeCount: 1,
 	}
@@ -414,7 +412,7 @@ func TestEvents(t *testing.T) {
 			{Message: "foo", Attributes: []core.KeyValue{k1v1}},
 			{Message: "bar", Attributes: []core.KeyValue{k2v2, k3v3}},
 		},
-		SpanKind: "internal",
+		SpanKind: apitrace.SpanKindInternal,
 	}
 	if diff := cmpDiff(got, want); diff != "" {
 		t.Errorf("Message Events: -got +want %s", diff)
@@ -465,7 +463,7 @@ func TestEventsOverLimit(t *testing.T) {
 		},
 		DroppedMessageEventCount: 2,
 		HasRemoteParent:          true,
-		SpanKind:                 "internal",
+		SpanKind:                 apitrace.SpanKindInternal,
 	}
 	if diff := cmpDiff(got, want); diff != "" {
 		t.Errorf("Message Event over limit: -got +want %s", diff)
@@ -505,7 +503,7 @@ func TestAddLinks(t *testing.T) {
 			{SpanContext: sc1, Attributes: []core.KeyValue{k1v1}},
 			{SpanContext: sc2, Attributes: []core.KeyValue{k2v2}},
 		},
-		SpanKind: "internal",
+		SpanKind: apitrace.SpanKindInternal,
 	}
 	if diff := cmpDiff(got, want); diff != "" {
 		t.Errorf("AddLink: -got +want %s", diff)
@@ -546,7 +544,7 @@ func TestLinks(t *testing.T) {
 			{SpanContext: sc1, Attributes: []core.KeyValue{k1v1}},
 			{SpanContext: sc2, Attributes: []core.KeyValue{k2v2, k3v3}},
 		},
-		SpanKind: "internal",
+		SpanKind: apitrace.SpanKindInternal,
 	}
 	if diff := cmpDiff(got, want); diff != "" {
 		t.Errorf("Link: -got +want %s", diff)
@@ -589,7 +587,7 @@ func TestLinksOverLimit(t *testing.T) {
 		},
 		DroppedLinkCount: 1,
 		HasRemoteParent:  true,
-		SpanKind:         "internal",
+		SpanKind:         apitrace.SpanKindInternal,
 	}
 	if diff := cmpDiff(got, want); diff != "" {
 		t.Errorf("Link over limit: -got +want %s", diff)
@@ -636,7 +634,7 @@ func TestSetSpanStatus(t *testing.T) {
 		},
 		ParentSpanID:    sid,
 		Name:            "SpanStatus/span0",
-		SpanKind:        "internal",
+		SpanKind:        apitrace.SpanKindInternal,
 		Status:          codes.Canceled,
 		HasRemoteParent: true,
 	}