Repository: go-kit/kit (go)

Issue:
Add error tag to spans on OT endpoint middleware
Hi!
I think it would be useful for the OpenTracing endpoint middleware (https://github.com/go-kit/kit/blob/master/tracing/opentracing/endpoint.go) to inspect the `error` returned by the next endpoint and set the `error` tag to true/false accordingly (it could also optionally set the `error.object` log field).
In addition to just inspecting the returned `error` it could also check if the `response` struct implements `endpoint.Failer` and apply the same logic.

If you think this would be useful I'd be happy to create a PR, just wanted to get your thoughts first.

Pull request description:
tracing.opentracing endpoint middleware options
# what

- Add `TraceEndpoint` function to not rely on client/server kinds.
- TraceServer and TraceClient should differ only in span kind tag value
- Extend `tracing.opentracing` endpoint middleware API with some options (like it was done for Opencensus):
  - optionally trace business errors
  - add option to set static/dynamic tags
  - add option to set dynamic name based on context and previous name
  - bump opentracing lib (to have `ext.LogError` function)

# why

closes #804 

Patch:
diff --git a/go.mod b/go.mod
index 1d66e82..e1669ca 100644
--- a/go.mod
+++ b/go.mod
@@ -35,7 +35,7 @@ require (
 	github.com/oklog/run v1.0.0 // indirect
 	github.com/op/go-logging v0.0.0-20160315200505-970db520ece7 // indirect
 	github.com/opentracing/basictracer-go v1.0.0 // indirect
-	github.com/opentracing/opentracing-go v1.1.0
+	github.com/opentracing/opentracing-go v1.2.0
 	github.com/openzipkin-contrib/zipkin-go-opentracing v0.4.5
 	github.com/openzipkin/zipkin-go v0.2.2
 	github.com/pact-foundation/pact-go v1.0.4
diff --git a/go.sum b/go.sum
index 9a80e3e..b41a5f3 100644
--- a/go.sum
+++ b/go.sum
@@ -106,7 +106,6 @@ github.com/golang/protobuf v1.3.1/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5y
 github.com/golang/protobuf v1.3.2 h1:6nsPYzhq5kReh6QImI3k5qWzO4PEbvbIW2cwSfR/6xs=
 github.com/golang/protobuf v1.3.2/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U=
 github.com/golang/snappy v0.0.0-20180518054509-2e65f85255db/go.mod h1:/XxbfmMg8lxefKM7IXC3fBNl/7bRcc72aCRzEWrmP2Q=
-github.com/google/btree v0.0.0-20180813153112-4030bb1f1f0c h1:964Od4U6p2jUkFxvCydnIczKteheJEzHRToSGK3Bnlw=
 github.com/google/btree v0.0.0-20180813153112-4030bb1f1f0c/go.mod h1:lNA+9X1NB3Zf8V7Ke586lFgjr2dZNuvo3lPJSGZ5JPQ=
 github.com/google/btree v1.0.0 h1:0udJVsspx3VBr5FwtLhQQtuAsVc79tTq0ocGIPAU6qo=
 github.com/google/btree v1.0.0/go.mod h1:lNA+9X1NB3Zf8V7Ke586lFgjr2dZNuvo3lPJSGZ5JPQ=
@@ -254,8 +253,9 @@ github.com/opentracing-contrib/go-observer v0.0.0-20170622124052-a52f23424492/go
 github.com/opentracing/basictracer-go v1.0.0 h1:YyUAhaEfjoWXclZVJ9sGoNct7j4TVk7lZWlQw5UXuoo=
 github.com/opentracing/basictracer-go v1.0.0/go.mod h1:QfBfYuafItcjQuMwinw9GhYKwFXS9KnPs5lxoYwgW74=
 github.com/opentracing/opentracing-go v1.0.2/go.mod h1:UkNAQd3GIcIGf0SeVgPpRdFStlNbqXla1AfSYxPUl2o=
-github.com/opentracing/opentracing-go v1.1.0 h1:pWlfV3Bxv7k65HYwkikxat0+s3pV4bsqf19k25Ur8rU=
 github.com/opentracing/opentracing-go v1.1.0/go.mod h1:UkNAQd3GIcIGf0SeVgPpRdFStlNbqXla1AfSYxPUl2o=
+github.com/opentracing/opentracing-go v1.2.0 h1:uEJPy/1a5RIPAJ0Ov+OIO8OxWu77jEv+1B0VhjKrZUs=
+github.com/opentracing/opentracing-go v1.2.0/go.mod h1:GxEUsuufX4nBwe+T+Wl9TAgYrxe9dPLANfrWvHYVTgc=
 github.com/openzipkin-contrib/zipkin-go-opentracing v0.4.5 h1:ZCnq+JUrvXcDVhX/xRolRBZifmabN1HcS1wrPSvxhrU=
 github.com/openzipkin-contrib/zipkin-go-opentracing v0.4.5/go.mod h1:/wsWhb9smxSfWAKL3wpBW7V8scJMt8N8gnaMCS9E/cA=
 github.com/openzipkin/zipkin-go v0.2.1/go.mod h1:NaW6tEwdmWMaCDZzg8sh+IBNOxHMPnhQw8ySjnjRyN4=
@@ -380,7 +380,6 @@ golang.org/x/net v0.0.0-20190311183353-d8887717615a/go.mod h1:t9HGtf8HONx5eT2rtn
 golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg=
 golang.org/x/net v0.0.0-20190603091049-60506f45cf65/go.mod h1:HSz+uSET+XFnRR8LxR5pz3Of3rY3CfYBVs4xY44aLks=
 golang.org/x/net v0.0.0-20190613194153-d28f0bde5980/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
-golang.org/x/net v0.0.0-20190620200207-3b0461eec859 h1:R/3boaszxrf1GEUWTVDzSKVwLmSJpwZ1yqXm8j0v2QI=
 golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
 golang.org/x/net v0.0.0-20190813141303-74dc4d7220e7 h1:fHDIZ2oxGnUZRN6WgWFCbYBjH9uqVPRCUVUDhs0wnbA=
 golang.org/x/net v0.0.0-20190813141303-74dc4d7220e7/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
diff --git a/tracing/opentracing/endpoint.go b/tracing/opentracing/endpoint.go
index 0482e9c..4df1ef2 100644
--- a/tracing/opentracing/endpoint.go
+++ b/tracing/opentracing/endpoint.go
@@ -2,54 +2,123 @@ package opentracing
 
 import (
 	"context"
+	"strconv"
 
 	"github.com/opentracing/opentracing-go"
 	otext "github.com/opentracing/opentracing-go/ext"
+	otlog "github.com/opentracing/opentracing-go/log"
 
 	"github.com/go-kit/kit/endpoint"
+	"github.com/go-kit/kit/sd/lb"
 )
 
-// TraceServer returns a Middleware that wraps the `next` Endpoint in an
+// TraceEndpoint returns a Middleware that wraps the `next` Endpoint in an
 // OpenTracing Span called `operationName`.
 //
-// If `ctx` already has a Span, it is re-used and the operation name is
-// overwritten. If `ctx` does not yet have a Span, one is created here.
-func TraceServer(tracer opentracing.Tracer, operationName string) endpoint.Middleware {
-	return func(next endpoint.Endpoint) endpoint.Endpoint {
-		return func(ctx context.Context, request interface{}) (interface{}, error) {
-			serverSpan := opentracing.SpanFromContext(ctx)
-			if serverSpan == nil {
-				// All we can do is create a new root span.
-				serverSpan = tracer.StartSpan(operationName)
-			} else {
-				serverSpan.SetOperationName(operationName)
-			}
-			defer serverSpan.Finish()
-			otext.SpanKindRPCServer.Set(serverSpan)
-			ctx = opentracing.ContextWithSpan(ctx, serverSpan)
-			return next(ctx, request)
-		}
+// If `ctx` already has a Span, child span is created from it.
+// If `ctx` doesn't yet have a Span, the new one is created.
+func TraceEndpoint(tracer opentracing.Tracer, operationName string, opts ...EndpointOption) endpoint.Middleware {
+	cfg := &EndpointOptions{
+		Tags: make(opentracing.Tags),
+	}
+
+	for _, opt := range opts {
+		opt(cfg)
 	}
-}
 
-// TraceClient returns a Middleware that wraps the `next` Endpoint in an
-// OpenTracing Span called `operationName`.
-func TraceClient(tracer opentracing.Tracer, operationName string) endpoint.Middleware {
 	return func(next endpoint.Endpoint) endpoint.Endpoint {
-		return func(ctx context.Context, request interface{}) (interface{}, error) {
-			var clientSpan opentracing.Span
+		return func(ctx context.Context, request interface{}) (response interface{}, err error) {
+			if cfg.GetOperationName != nil {
+				if newOperationName := cfg.GetOperationName(ctx, operationName); newOperationName != "" {
+					operationName = newOperationName
+				}
+			}
+
+			var span opentracing.Span
 			if parentSpan := opentracing.SpanFromContext(ctx); parentSpan != nil {
-				clientSpan = tracer.StartSpan(
+				span = tracer.StartSpan(
 					operationName,
 					opentracing.ChildOf(parentSpan.Context()),
 				)
 			} else {
-				clientSpan = tracer.StartSpan(operationName)
+				span = tracer.StartSpan(operationName)
+			}
+			defer span.Finish()
+
+			applyTags(span, cfg.Tags)
+			if cfg.GetTags != nil {
+				extraTags := cfg.GetTags(ctx)
+				applyTags(span, extraTags)
 			}
-			defer clientSpan.Finish()
-			otext.SpanKindRPCClient.Set(clientSpan)
-			ctx = opentracing.ContextWithSpan(ctx, clientSpan)
+
+			ctx = opentracing.ContextWithSpan(ctx, span)
+
+			defer func() {
+				if err != nil {
+					if lbErr, ok := err.(lb.RetryError); ok {
+						// handle errors originating from lb.Retry
+						fields := make([]otlog.Field, 0, len(lbErr.RawErrors))
+						for idx, rawErr := range lbErr.RawErrors {
+							fields = append(fields, otlog.String(
+								"gokit.retry.error."+strconv.Itoa(idx+1), rawErr.Error(),
+							))
+						}
+
+						otext.LogError(span, lbErr, fields...)
+
+						return
+					}
+
+					// generic error
+					otext.LogError(span, err)
+
+					return
+				}
+
+				// test for business error
+				if res, ok := response.(endpoint.Failer); ok && res.Failed() != nil {
+					span.LogFields(
+						otlog.String("gokit.business.error", res.Failed().Error()),
+					)
+
+					if cfg.IgnoreBusinessError {
+						return
+					}
+
+					// treating business error as real error in span.
+					otext.LogError(span, res.Failed())
+
+					return
+				}
+			}()
+
 			return next(ctx, request)
 		}
 	}
 }
+
+// TraceServer returns a Middleware that wraps the `next` Endpoint in an
+// OpenTracing Span called `operationName` with server span.kind tag..
+func TraceServer(tracer opentracing.Tracer, operationName string, opts ...EndpointOption) endpoint.Middleware {
+	opts = append(opts, WithTags(map[string]interface{}{
+		otext.SpanKindRPCServer.Key: otext.SpanKindRPCServer.Value,
+	}))
+
+	return TraceEndpoint(tracer, operationName, opts...)
+}
+
+// TraceClient returns a Middleware that wraps the `next` Endpoint in an
+// OpenTracing Span called `operationName` with client span.kind tag.
+func TraceClient(tracer opentracing.Tracer, operationName string, opts ...EndpointOption) endpoint.Middleware {
+	opts = append(opts, WithTags(map[string]interface{}{
+		otext.SpanKindRPCClient.Key: otext.SpanKindRPCClient.Value,
+	}))
+
+	return TraceEndpoint(tracer, operationName, opts...)
+}
+
+func applyTags(span opentracing.Span, tags opentracing.Tags) {
+	for key, value := range tags {
+		span.SetTag(key, value)
+	}
+}
diff --git a/tracing/opentracing/endpoint_options.go b/tracing/opentracing/endpoint_options.go
new file mode 100644
index 0000000..6854271
--- /dev/null
+++ b/tracing/opentracing/endpoint_options.go
@@ -0,0 +1,74 @@
+package opentracing
+
+import (
+	"context"
+
+	"github.com/opentracing/opentracing-go"
+)
+
+// EndpointOptions holds the options for tracing an endpoint
+type EndpointOptions struct {
+	// IgnoreBusinessError if set to true will not treat a business error
+	// identified through the endpoint.Failer interface as a span error.
+	IgnoreBusinessError bool
+
+	// GetOperationName is an optional function that can set the span operation name based on the existing one
+	// for the endpoint and information in the context.
+	//
+	// If the function is nil, or the returned name is empty, the existing name for the endpoint is used.
+	GetOperationName func(ctx context.Context, name string) string
+
+	// Tags holds the default tags which will be set on span
+	// creation by our Endpoint middleware.
+	Tags opentracing.Tags
+
+	// GetTags is an optional function that can extract tags
+	// from the context and add them to the span.
+	GetTags func(ctx context.Context) opentracing.Tags
+}
+
+// EndpointOption allows for functional options to endpoint tracing middleware.
+type EndpointOption func(*EndpointOptions)
+
+// WithOptions sets all configuration options at once by use of the EndpointOptions struct.
+func WithOptions(options EndpointOptions) EndpointOption {
+	return func(o *EndpointOptions) {
+		*o = options
+	}
+}
+
+// WithIgnoreBusinessError if set to true will not treat a business error
+// identified through the endpoint.Failer interface as a span error.
+func WithIgnoreBusinessError(ignoreBusinessError bool) EndpointOption {
+	return func(o *EndpointOptions) {
+		o.IgnoreBusinessError = ignoreBusinessError
+	}
+}
+
+// WithOperationNameFunc allows to set function that can set the span operation name based on the existing one
+// for the endpoint and information in the context.
+func WithOperationNameFunc(getOperationName func(ctx context.Context, name string) string) EndpointOption {
+	return func(o *EndpointOptions) {
+		o.GetOperationName = getOperationName
+	}
+}
+
+// WithTags adds default tags for the spans created by the Endpoint tracer.
+func WithTags(tags opentracing.Tags) EndpointOption {
+	return func(o *EndpointOptions) {
+		if o.Tags == nil {
+			o.Tags = make(opentracing.Tags)
+		}
+
+		for key, value := range tags {
+			o.Tags[key] = value
+		}
+	}
+}
+
+// WithTagsFunc set the func to extracts additional tags from the context.
+func WithTagsFunc(getTags func(ctx context.Context) opentracing.Tags) EndpointOption {
+	return func(o *EndpointOptions) {
+		o.GetTags = getTags
+	}
+}


Test patch:
diff --git a/tracing/opentracing/endpoint_test.go b/tracing/opentracing/endpoint_test.go
index a26f7ba..267415b 100644
--- a/tracing/opentracing/endpoint_test.go
+++ b/tracing/opentracing/endpoint_test.go
@@ -2,50 +2,89 @@ package opentracing_test
 
 import (
 	"context"
+	"errors"
+	"fmt"
+	"reflect"
 	"testing"
+	"time"
 
 	"github.com/opentracing/opentracing-go"
+	otext "github.com/opentracing/opentracing-go/ext"
+	otlog "github.com/opentracing/opentracing-go/log"
 	"github.com/opentracing/opentracing-go/mocktracer"
 
 	"github.com/go-kit/kit/endpoint"
+	"github.com/go-kit/kit/sd"
+	"github.com/go-kit/kit/sd/lb"
 	kitot "github.com/go-kit/kit/tracing/opentracing"
 )
 
-func TestTraceServer(t *testing.T) {
+const (
+	span1 = "SPAN-1"
+	span2 = "SPAN-2"
+	span3 = "SPAN-3"
+	span4 = "SPAN-4"
+	span5 = "SPAN-5"
+	span6 = "SPAN-6"
+	span7 = "SPAN-7"
+	span8 = "SPAN-8"
+)
+
+var (
+	err1 = errors.New("some error")
+	err2 = errors.New("some business error")
+	err3 = errors.New("other business error")
+)
+
+// compile time assertion
+var _ endpoint.Failer = failedResponse{}
+
+type failedResponse struct {
+	err error
+}
+
+func (r failedResponse) Failed() error {
+	return r.err
+}
+
+func TestTraceEndpoint(t *testing.T) {
 	tracer := mocktracer.New()
 
-	// Initialize the ctx with a nameless Span.
-	contextSpan := tracer.StartSpan("").(*mocktracer.MockSpan)
-	ctx := opentracing.ContextWithSpan(context.Background(), contextSpan)
+	// Initialize the ctx with a parent Span.
+	parentSpan := tracer.StartSpan("parent").(*mocktracer.MockSpan)
+	defer parentSpan.Finish()
+	ctx := opentracing.ContextWithSpan(context.Background(), parentSpan)
 
-	tracedEndpoint := kitot.TraceServer(tracer, "testOp")(endpoint.Nop)
+	tracedEndpoint := kitot.TraceEndpoint(tracer, "testOp")(endpoint.Nop)
 	if _, err := tracedEndpoint(ctx, struct{}{}); err != nil {
 		t.Fatal(err)
 	}
 
+	// tracedEndpoint created a new Span.
 	finishedSpans := tracer.FinishedSpans()
 	if want, have := 1, len(finishedSpans); want != have {
 		t.Fatalf("Want %v span(s), found %v", want, have)
 	}
 
-	// Test that the op name is updated
 	endpointSpan := finishedSpans[0]
 	if want, have := "testOp", endpointSpan.OperationName; want != have {
 		t.Fatalf("Want %q, have %q", want, have)
 	}
-	contextContext := contextSpan.Context().(mocktracer.MockSpanContext)
-	endpointContext := endpointSpan.Context().(mocktracer.MockSpanContext)
-	// ...and that the ID is unmodified.
-	if want, have := contextContext.SpanID, endpointContext.SpanID; want != have {
-		t.Errorf("Want SpanID %q, have %q", want, have)
+
+	parentContext := parentSpan.Context().(mocktracer.MockSpanContext)
+	endpointContext := parentSpan.Context().(mocktracer.MockSpanContext)
+
+	// ... and that the parent ID is set appropriately.
+	if want, have := parentContext.SpanID, endpointContext.SpanID; want != have {
+		t.Errorf("Want ParentID %q, have %q", want, have)
 	}
 }
 
-func TestTraceServerNoContextSpan(t *testing.T) {
+func TestTraceEndpointNoContextSpan(t *testing.T) {
 	tracer := mocktracer.New()
 
 	// Empty/background context.
-	tracedEndpoint := kitot.TraceServer(tracer, "testOp")(endpoint.Nop)
+	tracedEndpoint := kitot.TraceEndpoint(tracer, "testOp")(endpoint.Nop)
 	if _, err := tracedEndpoint(context.Background(), struct{}{}); err != nil {
 		t.Fatal(err)
 	}
@@ -57,21 +96,264 @@ func TestTraceServerNoContextSpan(t *testing.T) {
 	}
 
 	endpointSpan := finishedSpans[0]
+
 	if want, have := "testOp", endpointSpan.OperationName; want != have {
 		t.Fatalf("Want %q, have %q", want, have)
 	}
 }
 
-func TestTraceClient(t *testing.T) {
+func TestTraceEndpointWithOptions(t *testing.T) {
 	tracer := mocktracer.New()
 
-	// Initialize the ctx with a parent Span.
-	parentSpan := tracer.StartSpan("parent").(*mocktracer.MockSpan)
-	defer parentSpan.Finish()
-	ctx := opentracing.ContextWithSpan(context.Background(), parentSpan)
+	// span 1 without options
+	mw := kitot.TraceEndpoint(tracer, span1)
+	tracedEndpoint := mw(endpoint.Nop)
+	_, _ = tracedEndpoint(context.Background(), struct{}{})
 
-	tracedEndpoint := kitot.TraceClient(tracer, "testOp")(endpoint.Nop)
-	if _, err := tracedEndpoint(ctx, struct{}{}); err != nil {
+	// span 2 with options
+	mw = kitot.TraceEndpoint(
+		tracer,
+		span2,
+		kitot.WithOptions(kitot.EndpointOptions{}),
+	)
+	tracedEndpoint = mw(func(context.Context, interface{}) (interface{}, error) {
+		return nil, err1
+	})
+	_, _ = tracedEndpoint(context.Background(), struct{}{})
+
+	// span 3 with lb error
+	mw = kitot.TraceEndpoint(
+		tracer,
+		span3,
+		kitot.WithOptions(kitot.EndpointOptions{}),
+	)
+	tracedEndpoint = mw(
+		lb.Retry(
+			5,
+			1*time.Second,
+			lb.NewRoundRobin(
+				sd.FixedEndpointer{
+					func(context.Context, interface{}) (interface{}, error) {
+						return nil, err1
+					},
+				},
+			),
+		),
+	)
+	_, _ = tracedEndpoint(context.Background(), struct{}{})
+
+	// span 4 with disabled IgnoreBusinessError option
+	mw = kitot.TraceEndpoint(
+		tracer,
+		span4,
+		kitot.WithIgnoreBusinessError(false),
+	)
+	tracedEndpoint = mw(func(context.Context, interface{}) (interface{}, error) {
+		return failedResponse{
+			err: err2,
+		}, nil
+	})
+	_, _ = tracedEndpoint(context.Background(), struct{}{})
+
+	// span 5 with enabled IgnoreBusinessError option
+	mw = kitot.TraceEndpoint(tracer, span5, kitot.WithIgnoreBusinessError(true))
+	tracedEndpoint = mw(func(context.Context, interface{}) (interface{}, error) {
+		return failedResponse{
+			err: err3,
+		}, nil
+	})
+	_, _ = tracedEndpoint(context.Background(), struct{}{})
+
+	// span 6 with OperationNameFunc option
+	mw = kitot.TraceEndpoint(
+		tracer,
+		span6,
+		kitot.WithOperationNameFunc(func(ctx context.Context, name string) string {
+			return fmt.Sprintf("%s-%s", "new", name)
+		}),
+	)
+	tracedEndpoint = mw(endpoint.Nop)
+	_, _ = tracedEndpoint(context.Background(), struct{}{})
+
+	// span 7 with Tags options
+	mw = kitot.TraceEndpoint(
+		tracer,
+		span7,
+		kitot.WithTags(map[string]interface{}{
+			"tag1": "tag1",
+			"tag2": "tag2",
+		}),
+		kitot.WithTags(map[string]interface{}{
+			"tag3": "tag3",
+		}),
+	)
+	tracedEndpoint = mw(endpoint.Nop)
+	_, _ = tracedEndpoint(context.Background(), struct{}{})
+
+	// span 8 with TagsFunc options
+	mw = kitot.TraceEndpoint(
+		tracer,
+		span8,
+		kitot.WithTags(map[string]interface{}{
+			"tag1": "tag1",
+			"tag2": "tag2",
+		}),
+		kitot.WithTags(map[string]interface{}{
+			"tag3": "tag3",
+		}),
+		kitot.WithTagsFunc(func(ctx context.Context) opentracing.Tags {
+			return map[string]interface{}{
+				"tag4": "tag4",
+			}
+		}),
+	)
+	tracedEndpoint = mw(endpoint.Nop)
+	_, _ = tracedEndpoint(context.Background(), struct{}{})
+
+	finishedSpans := tracer.FinishedSpans()
+	if want, have := 8, len(finishedSpans); want != have {
+		t.Fatalf("Want %v span(s), found %v", want, have)
+	}
+
+	// test span 1
+	span := finishedSpans[0]
+
+	if want, have := span1, span.OperationName; want != have {
+		t.Fatalf("Want %q, have %q", want, have)
+	}
+
+	// test span 2
+	span = finishedSpans[1]
+
+	if want, have := span2, span.OperationName; want != have {
+		t.Fatalf("Want %q, have %q", want, have)
+	}
+
+	if want, have := true, span.Tag("error"); want != have {
+		t.Fatalf("Want %v, have %v", want, have)
+	}
+
+	// test span 3
+	span = finishedSpans[2]
+
+	if want, have := span3, span.OperationName; want != have {
+		t.Fatalf("Want %q, have %q", want, have)
+	}
+
+	if want, have := true, span.Tag("error"); want != have {
+		t.Fatalf("Want %v, have %v", want, have)
+	}
+
+	if want, have := 1, len(span.Logs()); want != have {
+		t.Fatalf("incorrect logs count, wanted %d, got %d", want, have)
+	}
+
+	if want, have := []otlog.Field{
+		otlog.String("event", "error"),
+		otlog.String("error.object", "some error (previously: some error; some error; some error; some error)"),
+		otlog.String("gokit.retry.error.1", "some error"),
+		otlog.String("gokit.retry.error.2", "some error"),
+		otlog.String("gokit.retry.error.3", "some error"),
+		otlog.String("gokit.retry.error.4", "some error"),
+		otlog.String("gokit.retry.error.5", "some error"),
+	}, span.Logs()[0].Fields; reflect.DeepEqual(want, have) {
+		t.Fatalf("Want %q, have %q", want, have)
+	}
+
+	// test span 4
+	span = finishedSpans[3]
+
+	if want, have := span4, span.OperationName; want != have {
+		t.Fatalf("Want %q, have %q", want, have)
+	}
+
+	if want, have := true, span.Tag("error"); want != have {
+		t.Fatalf("Want %v, have %v", want, have)
+	}
+
+	if want, have := 2, len(span.Logs()); want != have {
+		t.Fatalf("incorrect logs count, wanted %d, got %d", want, have)
+	}
+
+	if want, have := []otlog.Field{
+		otlog.String("gokit.business.error", "some business error"),
+	}, span.Logs()[0].Fields; reflect.DeepEqual(want, have) {
+		t.Fatalf("Want %q, have %q", want, have)
+	}
+
+	if want, have := []otlog.Field{
+		otlog.String("event", "error"),
+		otlog.String("error.object", "some business error"),
+	}, span.Logs()[1].Fields; reflect.DeepEqual(want, have) {
+		t.Fatalf("Want %q, have %q", want, have)
+	}
+
+	// test span 5
+	span = finishedSpans[4]
+
+	if want, have := span5, span.OperationName; want != have {
+		t.Fatalf("Want %q, have %q", want, have)
+	}
+
+	if want, have := (interface{})(nil), span.Tag("error"); want != have {
+		t.Fatalf("Want %q, have %q", want, have)
+	}
+
+	if want, have := 1, len(span.Logs()); want != have {
+		t.Fatalf("incorrect logs count, wanted %d, got %d", want, have)
+	}
+
+	if want, have := []otlog.Field{
+		otlog.String("gokit.business.error", "some business error"),
+	}, span.Logs()[0].Fields; reflect.DeepEqual(want, have) {
+		t.Fatalf("Want %q, have %q", want, have)
+	}
+
+	// test span 6
+	span = finishedSpans[5]
+
+	if want, have := fmt.Sprintf("%s-%s", "new", span6), span.OperationName; want != have {
+		t.Fatalf("Want %q, have %q", want, have)
+	}
+
+	// test span 7
+	span = finishedSpans[6]
+
+	if want, have := span7, span.OperationName; want != have {
+		t.Fatalf("Want %q, have %q", want, have)
+	}
+
+	if want, have := map[string]interface{}{
+		"tag1": "tag1",
+		"tag2": "tag2",
+		"tag3": "tag3",
+	}, span.Tags(); fmt.Sprint(want) != fmt.Sprint(have) {
+		t.Fatalf("Want %q, have %q", want, have)
+	}
+
+	// test span 8
+	span = finishedSpans[7]
+
+	if want, have := span8, span.OperationName; want != have {
+		t.Fatalf("Want %q, have %q", want, have)
+	}
+
+	if want, have := map[string]interface{}{
+		"tag1": "tag1",
+		"tag2": "tag2",
+		"tag3": "tag3",
+		"tag4": "tag4",
+	}, span.Tags(); fmt.Sprint(want) != fmt.Sprint(have) {
+		t.Fatalf("Want %q, have %q", want, have)
+	}
+}
+
+func TestTraceServer(t *testing.T) {
+	tracer := mocktracer.New()
+
+	// Empty/background context.
+	tracedEndpoint := kitot.TraceServer(tracer, "testOp")(endpoint.Nop)
+	if _, err := tracedEndpoint(context.Background(), struct{}{}); err != nil {
 		t.Fatal(err)
 	}
 
@@ -81,21 +363,20 @@ func TestTraceClient(t *testing.T) {
 		t.Fatalf("Want %v span(s), found %v", want, have)
 	}
 
-	endpointSpan := finishedSpans[0]
-	if want, have := "testOp", endpointSpan.OperationName; want != have {
+	span := finishedSpans[0]
+
+	if want, have := "testOp", span.OperationName; want != have {
 		t.Fatalf("Want %q, have %q", want, have)
 	}
 
-	parentContext := parentSpan.Context().(mocktracer.MockSpanContext)
-	endpointContext := parentSpan.Context().(mocktracer.MockSpanContext)
-
-	// ... and that the parent ID is set appropriately.
-	if want, have := parentContext.SpanID, endpointContext.SpanID; want != have {
-		t.Errorf("Want ParentID %q, have %q", want, have)
+	if want, have := map[string]interface{}{
+		otext.SpanKindRPCServer.Key: otext.SpanKindRPCServer.Value,
+	}, span.Tags(); fmt.Sprint(want) != fmt.Sprint(have) {
+		t.Fatalf("Want %q, have %q", want, have)
 	}
 }
 
-func TestTraceClientNoContextSpan(t *testing.T) {
+func TestTraceClient(t *testing.T) {
 	tracer := mocktracer.New()
 
 	// Empty/background context.
@@ -110,8 +391,15 @@ func TestTraceClientNoContextSpan(t *testing.T) {
 		t.Fatalf("Want %v span(s), found %v", want, have)
 	}
 
-	endpointSpan := finishedSpans[0]
-	if want, have := "testOp", endpointSpan.OperationName; want != have {
+	span := finishedSpans[0]
+
+	if want, have := "testOp", span.OperationName; want != have {
+		t.Fatalf("Want %q, have %q", want, have)
+	}
+
+	if want, have := map[string]interface{}{
+		otext.SpanKindRPCClient.Key: otext.SpanKindRPCClient.Value,
+	}, span.Tags(); fmt.Sprint(want) != fmt.Sprint(have) {
 		t.Fatalf("Want %q, have %q", want, have)
 	}
 }