add WithSpanKind option to span creation (#234)

* add WithSpanKind option to span creation

* change SpanKind to string alias and add support for SpanKind on ot bridge

* fix tests

* fix import order

* fix nits

api/trace/api.go

@@ -110,6 +110,7 @@ type SpanOptions struct {
 	StartTime  time.Time
 	Relation   Relation
 	Record     bool
+	SpanKind   SpanKind
 }
 
 // Relation is used to establish relationship between newly created span and the
@@ -143,8 +144,20 @@ type Link struct {
 	Attributes []core.KeyValue
 }
 
+// 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
+
+const (
+	SpanKindInternal SpanKind = "internal"
+	SpanKindServer   SpanKind = "server"
+	SpanKindClient   SpanKind = "client"
+	SpanKindProducer SpanKind = "producer"
+	SpanKindConsumer SpanKind = "consumer"
+)
+
 // WithStartTime sets the start time of the span to provided time t, when it is started.
-// In absensce of this option, wall clock time is used as start time.
+// 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.
 func WithStartTime(t time.Time) SpanOption {
 	return func(o *SpanOptions) {
@@ -188,3 +201,10 @@ func FollowsFrom(sc core.SpanContext) SpanOption {
 		}
 	}
 }
+
+// WithSpanKind specifies the role a Span on a Trace.
+func WithSpanKind(sk SpanKind) SpanOption {
+	return func(o *SpanOptions) {
+		o.SpanKind = sk
+	}
+}

experimental/bridge/opentracing/bridge.go

@@ -312,14 +312,14 @@ func (t *BridgeTracer) StartSpan(operationName string, opts ...ot.StartSpanOptio
 	}
 	// TODO: handle links, needs SpanData to be in the API first?
 	bRelation, _ := otSpanReferencesToBridgeRelationAndLinks(sso.References)
-	// TODO: handle span kind, needs SpanData to be in the API first?
-	attributes, _, hadTrueErrorTag := otTagsToOtelAttributesKindAndError(sso.Tags)
+	attributes, kind, hadTrueErrorTag := otTagsToOtelAttributesKindAndError(sso.Tags)
 	checkCtx := migration.WithDeferredSetup(context.Background())
 	checkCtx2, otelSpan := t.setTracer.tracer().Start(checkCtx, operationName, func(opts *oteltrace.SpanOptions) {
 		opts.Attributes = attributes
 		opts.StartTime = sso.StartTime
 		opts.Relation = bRelation.ToOtelRelation()
 		opts.Record = true
+		opts.SpanKind = kind
 	})
 	if checkCtx != checkCtx2 {
 		t.warnOnce.Do(func() {
@@ -379,17 +379,16 @@ func (t *BridgeTracer) ContextWithSpanHook(ctx context.Context, span ot.Span) co
 	return ctx
 }
 
-type spanKindTODO struct{}
-
-func otTagsToOtelAttributesKindAndError(tags map[string]interface{}) ([]otelcore.KeyValue, spanKindTODO, bool) {
-	kind := spanKindTODO{}
+func otTagsToOtelAttributesKindAndError(tags map[string]interface{}) ([]otelcore.KeyValue, oteltrace.SpanKind, bool) {
+	kind := oteltrace.SpanKindInternal
 	error := false
 	var pairs []otelcore.KeyValue
 	for k, v := range tags {
 		switch k {
 		case string(otext.SpanKind):
-			// TODO: java has some notion of span kind, it
-			// probably is related to some proto stuff
+			if sk, ok := v.(string); ok {
+				kind = oteltrace.SpanKind(sk)
+			}
 		case string(otext.Error):
 			if b, ok := v.(bool); ok && b {
 				error = true

experimental/bridge/opentracing/internal/mock.go

@@ -27,7 +27,7 @@ import (
 	otelkey "go.opentelemetry.io/api/key"
 	oteltrace "go.opentelemetry.io/api/trace"
 
-	migration "go.opentelemetry.io/experimental/bridge/opentracing/migration"
+	"go.opentelemetry.io/experimental/bridge/opentracing/migration"
 )
 
 var (
@@ -97,6 +97,10 @@ 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(),
@@ -112,6 +116,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,
 	}
 	if !migration.SkipContextSetup(ctx) {
 		ctx = oteltrace.SetCurrentSpan(ctx, span)
@@ -209,6 +214,7 @@ type MockSpan struct {
 	mockTracer     *MockTracer
 	officialTracer oteltrace.Tracer
 	spanContext    otelcore.SpanContext
+	SpanKind       oteltrace.SpanKind
 	recording      bool
 
 	Attributes   oteldctx.Map

experimental/bridge/opentracing/mix_test.go

@@ -478,6 +478,12 @@ func checkTraceAndSpans(t *testing.T, tracer *internal.MockTracer, expectedTrace
 	// the last finished span has no parent
 	parentSpanIDs := append(spanIDs[1:], 0)
 
+	sks := map[uint64]oteltrace.SpanKind{
+		3456: oteltrace.SpanKindProducer,
+		2345: oteltrace.SpanKindInternal,
+		1234: oteltrace.SpanKindClient,
+	}
+
 	if len(tracer.FinishedSpans) != expectedSpanCount {
 		t.Errorf("Expected %d finished spans, got %d", expectedSpanCount, len(tracer.FinishedSpans))
 	}
@@ -494,6 +500,9 @@ func checkTraceAndSpans(t *testing.T, tracer *internal.MockTracer, expectedTrace
 		if span.ParentSpanID != expectedParentSpanID {
 			t.Errorf("Expected finished span %d (span ID: %d) to have parent span ID %d, but got %d", idx, sctx.SpanID, expectedParentSpanID, span.ParentSpanID)
 		}
+		if span.SpanKind != sks[span.SpanContext().SpanID] {
+			t.Errorf("Expected finished span %d (span ID: %d) to have span.kind to be '%v' but was '%v'", idx, sctx.SpanID, sks[span.SpanContext().SpanID], span.SpanKind)
+		}
 	}
 }
 
@@ -535,15 +544,15 @@ func min(a, b int) int {
 
 func runOtelOTOtel(t *testing.T, ctx context.Context, name string, callback func(*testing.T, context.Context)) {
 	tr := oteltrace.GlobalProvider().GetTracer("")
-	ctx, span := tr.Start(ctx, fmt.Sprintf("%s_Otel_OTOtel", name))
+	ctx, span := tr.Start(ctx, fmt.Sprintf("%s_Otel_OTOtel", name), oteltrace.WithSpanKind(oteltrace.SpanKindClient))
 	defer span.End()
 	callback(t, ctx)
 	func(ctx2 context.Context) {
 		span, ctx2 := ot.StartSpanFromContext(ctx2, fmt.Sprintf("%sOtel_OT_Otel", name))
 		defer span.Finish()
 		callback(t, ctx2)
 		func(ctx3 context.Context) {
-			ctx3, span := tr.Start(ctx3, fmt.Sprintf("%sOtelOT_Otel_", name))
+			ctx3, span := tr.Start(ctx3, fmt.Sprintf("%sOtelOT_Otel_", name), oteltrace.WithSpanKind(oteltrace.SpanKindProducer))
 			defer span.End()
 			callback(t, ctx3)
 		}(ctx2)
@@ -552,15 +561,15 @@ func runOtelOTOtel(t *testing.T, ctx context.Context, name string, callback func
 
 func runOTOtelOT(t *testing.T, ctx context.Context, name string, callback func(*testing.T, context.Context)) {
 	tr := oteltrace.GlobalProvider().GetTracer("")
-	span, ctx := ot.StartSpanFromContext(ctx, fmt.Sprintf("%s_OT_OtelOT", name))
+	span, ctx := ot.StartSpanFromContext(ctx, fmt.Sprintf("%s_OT_OtelOT", name), ot.Tag{Key: "span.kind", Value: "client"})
 	defer span.Finish()
 	callback(t, ctx)
 	func(ctx2 context.Context) {
 		ctx2, span := tr.Start(ctx2, fmt.Sprintf("%sOT_Otel_OT", name))
 		defer span.End()
 		callback(t, ctx2)
 		func(ctx3 context.Context) {
-			span, ctx3 := ot.StartSpanFromContext(ctx3, fmt.Sprintf("%sOTOtel_OT_", name))
+			span, ctx3 := ot.StartSpanFromContext(ctx3, fmt.Sprintf("%sOTOtel_OT_", name), ot.Tag{Key: "span.kind", Value: "producer"})
 			defer span.Finish()
 			callback(t, ctx3)
 		}(ctx2)

exporter/trace/stackdriver/trace_proto.go

@@ -71,7 +71,7 @@ func protoFromSpanData(s *export.SpanData, projectID string) *tracepb.Span {
 	switch s.SpanKind {
 	// TODO(ymotongpoo): add cases for "Send" and "Recv".
 	default:
-		name = fmt.Sprintf("Span.%d-%s", s.SpanKind, name)
+		name = fmt.Sprintf("Span.%s-%s", s.SpanKind, name)
 	}
 
 	sp := &tracepb.Span{

exporter/trace/stdout/stdout_test.go

@@ -24,6 +24,7 @@ import (
 	"google.golang.org/grpc/codes"
 
 	"go.opentelemetry.io/api/core"
+	"go.opentelemetry.io/api/trace"
 	"go.opentelemetry.io/sdk/export"
 )
 
@@ -62,7 +63,8 @@ func TestExporter_ExportSpan(t *testing.T) {
 				Value: core.Value{Type: core.FLOAT64, Float64: doubleValue},
 			},
 		},
-		Status: codes.Unknown,
+		SpanKind: trace.SpanKindInternal,
+		Status:   codes.Unknown,
 	}
 	ex.ExportSpan(context.Background(), testSpan)
 
@@ -73,7 +75,7 @@ func TestExporter_ExportSpan(t *testing.T) {
 		`"TraceID":"0102030405060708090a0b0c0d0e0f10",` +
 		`"SpanID":"0102030405060708","TraceFlags":0},` +
 		`"ParentSpanID":0,` +
-		`"SpanKind":0,` +
+		`"SpanKind":"internal",` +
 		`"Name":"/foo",` +
 		`"StartTime":` + string(expectedSerializedNow) + "," +
 		`"EndTime":` + string(expectedSerializedNow) + "," +

plugin/httptrace/clienttrace.go

@@ -60,7 +60,7 @@ func newClientTracer(ctx context.Context) *clientTracer {
 }
 
 func (ct *clientTracer) open(name string, attrs ...core.KeyValue) {
-	_, sp := ct.tr.Start(ct.Context, name, trace.WithAttributes(attrs...))
+	_, sp := ct.tr.Start(ct.Context, name, trace.WithAttributes(attrs...), trace.WithSpanKind(trace.SpanKindClient))
 	ct.mtx.Lock()
 	defer ct.mtx.Unlock()
 	if ct.root == nil {

plugin/othttp/handler.go

@@ -131,6 +131,7 @@ func NewHandler(handler http.Handler, operation string, opts ...Option) http.Han
 	defaultOpts := []Option{
 		WithTracer(trace.GlobalProvider().GetTracer("go.opentelemtry.io/plugin/othttp")),
 		WithPropagator(prop.HTTPTraceContextPropagator{}),
+		WithSpanOptions(trace.WithSpanKind(trace.SpanKindServer)),
 	}
 
 	for _, opt := range append(defaultOpts, opts...) {

sdk/export/span.go

@@ -27,7 +27,7 @@ import (
 type SpanData struct {
 	SpanContext  core.SpanContext
 	ParentSpanID uint64
-	SpanKind     int
+	SpanKind     apitrace.SpanKind
 	Name         string
 	StartTime    time.Time
 	// The wall clock time of EndTime will be adjusted to always be offset

sdk/trace/span.go

@@ -330,11 +330,14 @@ 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,
-		// TODO;[rghetia] : fix spanKind
-		//SpanKind:        o.SpanKind,
+		SpanContext:     span.spanContext,
+		StartTime:       startTime,
+		SpanKind:        sk,
 		Name:            name,
 		HasRemoteParent: remoteParent,
 	}

sdk/trace/trace_test.go

@@ -242,6 +242,7 @@ func TestSetSpanAttributes(t *testing.T) {
 			Key:   core.Key("key1"),
 			Value: core.Value{Type: core.STRING, String: "value1"},
 		}},
+		SpanKind:        "internal",
 		HasRemoteParent: true,
 	}
 	if diff := cmp.Diff(got, want); diff != "" {
@@ -281,6 +282,7 @@ func TestSetSpanAttributesOverLimit(t *testing.T) {
 				Value: core.Value{Type: core.STRING, String: "value4"},
 			},
 		},
+		SpanKind:              "internal",
 		HasRemoteParent:       true,
 		DroppedAttributeCount: 1,
 	}
@@ -326,6 +328,7 @@ func TestEvents(t *testing.T) {
 			{Message: "foo", Attributes: []core.KeyValue{k1v1}},
 			{Message: "bar", Attributes: []core.KeyValue{k2v2, k3v3}},
 		},
+		SpanKind: "internal",
 	}
 	if diff := cmp.Diff(got, want, cmp.AllowUnexported(export.Event{})); diff != "" {
 		t.Errorf("Message Events: -got +want %s", diff)
@@ -376,6 +379,7 @@ func TestEventsOverLimit(t *testing.T) {
 		},
 		DroppedMessageEventCount: 2,
 		HasRemoteParent:          true,
+		SpanKind:                 "internal",
 	}
 	if diff := cmp.Diff(got, want, cmp.AllowUnexported(export.Event{})); diff != "" {
 		t.Errorf("Message Event over limit: -got +want %s", diff)
@@ -415,6 +419,7 @@ func TestAddLinks(t *testing.T) {
 			{SpanContext: sc1, Attributes: []core.KeyValue{k1v1}},
 			{SpanContext: sc2, Attributes: []core.KeyValue{k2v2}},
 		},
+		SpanKind: "internal",
 	}
 	if diff := cmp.Diff(got, want, cmp.AllowUnexported(export.Event{})); diff != "" {
 		t.Errorf("AddLink: -got +want %s", diff)
@@ -455,6 +460,7 @@ func TestLinks(t *testing.T) {
 			{SpanContext: sc1, Attributes: []core.KeyValue{k1v1}},
 			{SpanContext: sc2, Attributes: []core.KeyValue{k2v2, k3v3}},
 		},
+		SpanKind: "internal",
 	}
 	if diff := cmp.Diff(got, want, cmp.AllowUnexported(export.Event{})); diff != "" {
 		t.Errorf("Link: -got +want %s", diff)
@@ -497,6 +503,7 @@ func TestLinksOverLimit(t *testing.T) {
 		},
 		DroppedLinkCount: 1,
 		HasRemoteParent:  true,
+		SpanKind:         "internal",
 	}
 	if diff := cmp.Diff(got, want, cmp.AllowUnexported(export.Event{})); diff != "" {
 		t.Errorf("Link over limit: -got +want %s", diff)
@@ -543,6 +550,7 @@ func TestSetSpanStatus(t *testing.T) {
 		},
 		ParentSpanID:    sid,
 		Name:            "SpanStatus/span0",
+		SpanKind:        "internal",
 		Status:          codes.Canceled,
 		HasRemoteParent: true,
 	}
@@ -801,3 +809,41 @@ func TestCustomStartEndTime(t *testing.T) {
 		t.Errorf("expected end time to be %s, got %s", endTime, got.EndTime)
 	}
 }
+
+func TestWithSpanKind(t *testing.T) {
+	var te testExporter
+	tp, _ := NewProvider(WithSyncer(&te), WithConfig(Config{DefaultSampler: AlwaysSample()}))
+	tr := tp.GetTracer("withSpanKind")
+
+	_, span := tr.Start(context.Background(), "WithoutSpanKind")
+	spanData, err := endSpan(&te, span)
+	if err != nil {
+		t.Error(err.Error())
+	}
+
+	if spanData.SpanKind != apitrace.SpanKindInternal {
+		t.Errorf("Default value of Spankind should be Internal: got %+v, want %+v\n", spanData.SpanKind, apitrace.SpanKindInternal)
+	}
+
+	sks := []apitrace.SpanKind{
+		apitrace.SpanKindInternal,
+		apitrace.SpanKindServer,
+		apitrace.SpanKindClient,
+		apitrace.SpanKindProducer,
+		apitrace.SpanKindConsumer,
+	}
+
+	for _, sk := range sks {
+		te.spans = nil
+
+		_, span := tr.Start(context.Background(), fmt.Sprintf("SpanKind-%v", sk), apitrace.WithSpanKind(sk))
+		spanData, err := endSpan(&te, span)
+		if err != nil {
+			t.Error(err.Error())
+		}
+
+		if spanData.SpanKind != sk {
+			t.Errorf("WithSpanKind check: got %+v, want %+v\n", spanData.SpanKind, sks)
+		}
+	}
+}