Merge pull request #30 from unistack-org/spanfix

fix span tags

Author: vtolstov

go.mod

@@ -4,6 +4,6 @@ go 1.16
 
 require (
 	github.com/opentracing/opentracing-go v1.2.0
-	go.unistack.org/micro/v3 v3.10.1
+	go.unistack.org/micro/v3 v3.10.4
 	gopkg.in/yaml.v2 v2.4.0 // indirect
 )

go.sum

@@ -73,8 +73,8 @@ github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415/go.mod h1:
 github.com/xeipuuv/gojsonschema v1.2.0/go.mod h1:anYRn/JVcOK2ZgGU+IjEV4nwlhoK5sQluxsYJ78Id3Y=
 go.opentelemetry.io/proto/otlp v0.7.0/go.mod h1:PqfVotwruBrMGOCsRd/89rSnXhoiJIqeYNgFYFoEGnI=
 go.unistack.org/micro-proto/v3 v3.3.1/go.mod h1:cwRyv8uInM2I7EbU7O8Fx2Ls3N90Uw9UCCcq4olOdfE=
-go.unistack.org/micro/v3 v3.10.1 h1:MbtPj7ueTw4x6lL2Ok2ez70ORyGXBWhxizO5fQsnAA4=
-go.unistack.org/micro/v3 v3.10.1/go.mod h1:gI4RkJKHLPW7KV6h4+ZBOZD997MRvFRXMPQIHpozikI=
+go.unistack.org/micro/v3 v3.10.4 h1:8HneC2t7oteTwwkFLmSg5bs62h/OqEzevx/IbXG1vRo=
+go.unistack.org/micro/v3 v3.10.4/go.mod h1:gI4RkJKHLPW7KV6h4+ZBOZD997MRvFRXMPQIHpozikI=
 golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
 golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
 golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA=

opentracing.go

@@ -10,18 +10,18 @@ import (
 	"go.unistack.org/micro/v3/tracer"
 )
 
-var _ tracer.Tracer = &opentracingTracer{}
+var _ tracer.Tracer = &otTracer{}
 
-type opentracingTracer struct {
+type otTracer struct {
 	opts   tracer.Options
 	tracer opentracing.Tracer
 }
 
-func (ot *opentracingTracer) Name() string {
+func (ot *otTracer) Name() string {
 	return ot.opts.Name
 }
 
-func (ot *opentracingTracer) Init(opts ...tracer.Option) error {
+func (ot *otTracer) Init(opts ...tracer.Option) error {
 	for _, o := range opts {
 		o(&ot.opts)
 	}
@@ -35,69 +35,109 @@ func (ot *opentracingTracer) Init(opts ...tracer.Option) error {
 	return nil
 }
 
-func (ot *opentracingTracer) Start(ctx context.Context, name string, opts ...tracer.SpanOption) (context.Context, tracer.Span) {
-	ctx, span, _ := startSpanFromIncomingContext(ctx, ot.tracer, name)
-	return ctx, &opentracingSpan{span: span}
+func (ot *otTracer) Start(ctx context.Context, name string, opts ...tracer.SpanOption) (context.Context, tracer.Span) {
+	options := tracer.NewSpanOptions(opts...)
+	var span opentracing.Span
+	switch options.Kind {
+	case tracer.SpanKindInternal, tracer.SpanKindUnspecified:
+		ctx, span = ot.startSpanFromContext(ctx, name)
+	case tracer.SpanKindClient, tracer.SpanKindProducer:
+		ctx, span = ot.startSpanFromOutgoingContext(ctx, name)
+	case tracer.SpanKindServer, tracer.SpanKindConsumer:
+		ctx, span = ot.startSpanFromIncomingContext(ctx, ot.tracer, name)
+	}
+	return ctx, &otSpan{span: span, opts: options}
+}
+
+type otSpan struct {
+	span      opentracing.Span
+	opts      tracer.SpanOptions
+	status    tracer.SpanStatus
+	statusMsg string
 }
 
-type opentracingSpan struct {
-	span   opentracing.Span
-	labels []interface{}
+func (os *otSpan) SetStatus(st tracer.SpanStatus, msg string) {
+	switch st {
+	case tracer.SpanStatusError:
+		os.span.SetTag("error", true)
+	}
+	os.status = st
+	os.statusMsg = msg
 }
 
-func (os *opentracingSpan) Tracer() tracer.Tracer {
-	return &opentracingTracer{tracer: os.span.Tracer()}
+func (os *otSpan) Status() (tracer.SpanStatus, string) {
+	return os.status, os.statusMsg
 }
 
-func (os *opentracingSpan) Finish(opts ...tracer.SpanOption) {
-	if len(os.labels) > 0 {
-		os.span.LogKV(os.labels...)
+func (os *otSpan) Tracer() tracer.Tracer {
+	return &otTracer{tracer: os.span.Tracer()}
+}
+
+func (os *otSpan) Finish(opts ...tracer.SpanOption) {
+	if len(os.opts.Labels) > 0 {
+		os.span.LogKV(os.opts.Labels...)
 	}
 	os.span.Finish()
 }
 
-func (os *opentracingSpan) AddEvent(name string, opts ...tracer.EventOption) {
+func (os *otSpan) AddEvent(name string, opts ...tracer.EventOption) {
 	os.span.LogFields(log.Event(name))
 }
 
-func (os *opentracingSpan) Context() context.Context {
-	return tracer.NewSpanContext(context.Background(), os)
+func (os *otSpan) Context() context.Context {
+	return opentracing.ContextWithSpan(context.Background(), os.span)
 }
 
-func (os *opentracingSpan) SetName(name string) {
+func (os *otSpan) SetName(name string) {
 	os.span = os.span.SetOperationName(name)
 }
 
-func (os *opentracingSpan) SetLabels(labels ...interface{}) {
-	os.labels = labels
+func (os *otSpan) SetLabels(labels ...interface{}) {
+	os.opts.Labels = labels
 }
 
-func (os *opentracingSpan) AddLabels(labels ...interface{}) {
-	os.labels = append(os.labels, labels...)
+func (os *otSpan) Kind() tracer.SpanKind {
+	return os.opts.Kind
 }
 
-func NewTracer(opts ...tracer.Option) *opentracingTracer {
+func (os *otSpan) AddLabels(labels ...interface{}) {
+	os.opts.Labels = append(os.opts.Labels, labels...)
+}
+
+func NewTracer(opts ...tracer.Option) *otTracer {
 	options := tracer.NewOptions(opts...)
-	return &opentracingTracer{opts: options}
+	return &otTracer{opts: options}
 }
 
 func spanFromContext(ctx context.Context) opentracing.Span {
 	return opentracing.SpanFromContext(ctx)
 }
 
-// startSpanFromOutgoingContext returns a new span with the given operation name and options. If a span
-// is found in the context, it will be used as the parent of the resulting span.
-func startSpanFromOutgoingContext(ctx context.Context, tracer opentracing.Tracer, name string, opts ...opentracing.StartSpanOption) (context.Context, opentracing.Span, error) {
+func (ot *otTracer) startSpanFromContext(ctx context.Context, name string, opts ...opentracing.StartSpanOption) (context.Context, opentracing.Span) {
+	if parentSpan := opentracing.SpanFromContext(ctx); parentSpan != nil {
+		opts = append(opts, opentracing.ChildOf(parentSpan.Context()))
+	}
+
+	md := metadata.New(1)
+
+	sp := ot.tracer.StartSpan(name, opts...)
+	if err := sp.Tracer().Inject(sp.Context(), opentracing.TextMap, opentracing.TextMapCarrier(md)); err != nil {
+		return nil, nil
+	}
+
+	ctx = opentracing.ContextWithSpan(ctx, sp)
+
+	return ctx, sp
+}
+
+func (ot *otTracer) startSpanFromOutgoingContext(ctx context.Context, name string, opts ...opentracing.StartSpanOption) (context.Context, opentracing.Span) {
 	var parentCtx opentracing.SpanContext
 
-	md, ok := metadata.FromIncomingContext(ctx)
-	// Find parent span.
-	if parentSpan := opentracing.SpanFromContext(ctx); parentSpan != nil {
-		// First try to get span within current service boundary.
-		parentCtx = parentSpan.Context()
-	} else if spanCtx, err := tracer.Extract(opentracing.TextMap, opentracing.TextMapCarrier(md)); err == nil && ok {
-		// If there doesn't exist, try to get it from metadata(which is cross boundary)
-		parentCtx = spanCtx
+	md, ok := metadata.FromOutgoingContext(ctx)
+	if ok && md != nil {
+		if spanCtx, err := ot.tracer.Extract(opentracing.TextMap, opentracing.TextMapCarrier(md)); err == nil && ok {
+			parentCtx = spanCtx
+		}
 	}
 
 	if parentCtx != nil {
@@ -106,52 +146,38 @@ func startSpanFromOutgoingContext(ctx context.Context, tracer opentracing.Tracer
 
 	nmd := metadata.Copy(md)
 
-	sp := tracer.StartSpan(name, opts...)
+	sp := ot.tracer.StartSpan(name, opts...)
 	if err := sp.Tracer().Inject(sp.Context(), opentracing.TextMap, opentracing.TextMapCarrier(nmd)); err != nil {
-		return nil, nil, err
+		return nil, nil
 	}
 
 	ctx = metadata.NewOutgoingContext(opentracing.ContextWithSpan(ctx, sp), nmd)
 
-	return ctx, sp, nil
+	return ctx, sp
 }
 
-// startSpanFromIncomingContext returns a new span with the given operation name and options. If a span
-// is found in the context, it will be used as the parent of the resulting span.
-func startSpanFromIncomingContext(ctx context.Context, tracer opentracing.Tracer, name string, opts ...opentracing.StartSpanOption) (context.Context, opentracing.Span, error) {
+func (ot *otTracer) startSpanFromIncomingContext(ctx context.Context, tracer opentracing.Tracer, name string, opts ...opentracing.StartSpanOption) (context.Context, opentracing.Span) {
 	var parentCtx opentracing.SpanContext
 
-	// Find parent span.
 	md, ok := metadata.FromIncomingContext(ctx)
-	if parentSpan := opentracing.SpanFromContext(ctx); parentSpan != nil {
-		// First try to get span within current service boundary.
-		parentCtx = parentSpan.Context()
-	} else if spanCtx, err := tracer.Extract(opentracing.TextMap, opentracing.TextMapCarrier(md)); err == nil && ok {
-		// If there doesn't exist, try to get it from metadata(which is cross boundary)
-		parentCtx = spanCtx
+	if ok && md != nil {
+		if spanCtx, err := tracer.Extract(opentracing.TextMap, opentracing.TextMapCarrier(md)); err == nil {
+			parentCtx = spanCtx
+		}
 	}
 
 	if parentCtx != nil {
 		opts = append(opts, opentracing.ChildOf(parentCtx))
 	}
 
-	var nmd metadata.Metadata
-	if ok {
-		nmd = metadata.New(len(md))
-	} else {
-		nmd = metadata.New(0)
-	}
+	nmd := metadata.Copy(md)
 
 	sp := tracer.StartSpan(name, opts...)
 	if err := sp.Tracer().Inject(sp.Context(), opentracing.TextMap, opentracing.TextMapCarrier(nmd)); err != nil {
-		return nil, nil, err
-	}
-
-	for k, v := range md {
-		nmd.Set(k, v)
+		return nil, nil
 	}
 
 	ctx = metadata.NewIncomingContext(opentracing.ContextWithSpan(ctx, sp), nmd)
 
-	return ctx, sp, nil
+	return ctx, sp
 }

opentracing_test.go

@@ -20,16 +20,14 @@ func TestStartSpanFromIncomingContext(t *testing.T) {
 	ctx = metadata.NewIncomingContext(ctx, md)
 
 	tracer := opentracing.GlobalTracer()
+	ot := &otTracer{tracer: tracer}
 
 	g.Add(8000)
 	cherr := make(chan error)
 	for i := 0; i < 8000; i++ {
 		go func() {
 			defer g.Done()
-			_, sp, err := startSpanFromIncomingContext(ctx, tracer, "test")
-			if err != nil {
-				cherr <- err
-			}
+			_, sp := ot.startSpanFromIncomingContext(ctx, tracer, "test")
 			sp.Finish()
 		}()
 	}