Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add WithSpanKind option to span creation #234

Merged
merged 7 commits into from
Oct 23, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 21 additions & 1 deletion api/trace/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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
}
}
15 changes: 7 additions & 8 deletions experimental/bridge/opentracing/bridge.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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
Expand Down
8 changes: 7 additions & 1 deletion experimental/bridge/opentracing/internal/mock.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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(),
Expand All @@ -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)
Expand Down Expand Up @@ -209,6 +214,7 @@ type MockSpan struct {
mockTracer *MockTracer
officialTracer oteltrace.Tracer
spanContext otelcore.SpanContext
SpanKind oteltrace.SpanKind
recording bool

Attributes oteldctx.Map
Expand Down
17 changes: 13 additions & 4 deletions experimental/bridge/opentracing/mix_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
Expand All @@ -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)
}
}
}

Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion exporter/trace/stackdriver/trace_proto.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
6 changes: 4 additions & 2 deletions exporter/trace/stdout/stdout_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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)

Expand All @@ -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) + "," +
Expand Down
2 changes: 1 addition & 1 deletion plugin/httptrace/clienttrace.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
1 change: 1 addition & 0 deletions plugin/othttp/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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...) {
Expand Down
2 changes: 1 addition & 1 deletion sdk/export/span.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 7 additions & 4 deletions sdk/trace/span.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
Expand Down
46 changes: 46 additions & 0 deletions sdk/trace/trace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 != "" {
Expand Down Expand Up @@ -281,6 +282,7 @@ func TestSetSpanAttributesOverLimit(t *testing.T) {
Value: core.Value{Type: core.STRING, String: "value4"},
},
},
SpanKind: "internal",
HasRemoteParent: true,
DroppedAttributeCount: 1,
}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -543,6 +550,7 @@ func TestSetSpanStatus(t *testing.T) {
},
ParentSpanID: sid,
Name: "SpanStatus/span0",
SpanKind: "internal",
Status: codes.Canceled,
HasRemoteParent: true,
}
Expand Down Expand Up @@ -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)
}
}
}