diff --git a/api/trace/api.go b/api/trace/api.go index b364af44a24..1172587aba0 100644 --- a/api/trace/api.go +++ b/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. diff --git a/bridge/opentracing/bridge.go b/bridge/opentracing/bridge.go index d76745789cd..034907bfd89 100644 --- a/bridge/opentracing/bridge.go +++ b/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 { diff --git a/bridge/opentracing/internal/mock.go b/bridge/opentracing/internal/mock.go index 20fd44d323e..19dde5f5d87 100644 --- a/bridge/opentracing/internal/mock.go +++ 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) diff --git a/bridge/opentracing/mix_test.go b/bridge/opentracing/mix_test.go index f623de2a555..70001099821 100644 --- a/bridge/opentracing/mix_test.go +++ b/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" diff --git a/exporter/trace/stdout/stdout_test.go b/exporter/trace/stdout/stdout_test.go index d54af337275..566c39e6c46 100644 --- a/exporter/trace/stdout/stdout_test.go +++ b/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) + "," + diff --git a/sdk/trace/span.go b/sdk/trace/span.go index 6ca5d5b601a..9607804f703 100644 --- a/sdk/trace/span.go +++ b/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, } diff --git a/sdk/trace/trace_test.go b/sdk/trace/trace_test.go index a66b6cf9a49..6584a95d8a9 100644 --- a/sdk/trace/trace_test.go +++ b/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, }