diff --git a/tracing/opentracing/endpoint.go b/tracing/opentracing/endpoint.go index 9f626a79c..ef7fa63fe 100644 --- a/tracing/opentracing/endpoint.go +++ b/tracing/opentracing/endpoint.go @@ -37,10 +37,10 @@ func TraceClient(tracer opentracing.Tracer, operationName string) endpoint.Middl return func(next endpoint.Endpoint) endpoint.Endpoint { return func(ctx context.Context, request interface{}) (interface{}, error) { parentSpan := opentracing.SpanFromContext(ctx) - clientSpan := tracer.StartSpanWithOptions(opentracing.StartSpanOptions{ - OperationName: operationName, - Parent: parentSpan, // may be nil - }) + clientSpan := tracer.StartSpan( + operationName, + opentracing.ChildOf(parentSpan.Context()), + ) defer clientSpan.Finish() otext.SpanKind.Set(clientSpan, otext.SpanKindRPCClient) ctx = opentracing.ContextWithSpan(ctx, clientSpan) diff --git a/tracing/opentracing/endpoint_test.go b/tracing/opentracing/endpoint_test.go index d89136c93..f7727b441 100644 --- a/tracing/opentracing/endpoint_test.go +++ b/tracing/opentracing/endpoint_test.go @@ -33,9 +33,10 @@ func TestTraceServer(t *testing.T) { 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 := contextSpan.SpanID, endpointSpan.SpanID; want != have { + if want, have := contextContext.SpanID, endpointContext.SpanID; want != have { t.Errorf("Want SpanID %q, have %q", want, have) } } @@ -85,8 +86,11 @@ func TestTraceClient(t *testing.T) { 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 := parentSpan.SpanID, endpointSpan.ParentID; want != have { + if want, have := parentContext.SpanID, endpointContext.SpanID; want != have { t.Errorf("Want ParentID %q, have %q", want, have) } } diff --git a/tracing/opentracing/grpc.go b/tracing/opentracing/grpc.go index 258dff205..e678e301c 100644 --- a/tracing/opentracing/grpc.go +++ b/tracing/opentracing/grpc.go @@ -5,6 +5,7 @@ import ( "strings" "github.com/opentracing/opentracing-go" + "github.com/opentracing/opentracing-go/ext" "golang.org/x/net/context" "google.golang.org/grpc/metadata" @@ -20,7 +21,7 @@ func ToGRPCRequest(tracer opentracing.Tracer, logger log.Logger) func(ctx contex return func(ctx context.Context, md *metadata.MD) context.Context { if span := opentracing.SpanFromContext(ctx); span != nil { // There's nothing we can do with an error here. - if err := tracer.Inject(span, opentracing.TextMap, metadataReaderWriter{md}); err != nil { + if err := tracer.Inject(span.Context(), opentracing.TextMap, metadataReaderWriter{md}); err != nil { logger.Log("err", err) } } @@ -37,13 +38,12 @@ func ToGRPCRequest(tracer opentracing.Tracer, logger log.Logger) func(ctx contex // The logger is used to report errors and may be nil. func FromGRPCRequest(tracer opentracing.Tracer, operationName string, logger log.Logger) func(ctx context.Context, md *metadata.MD) context.Context { return func(ctx context.Context, md *metadata.MD) context.Context { - span, err := tracer.Join(operationName, opentracing.TextMap, metadataReaderWriter{md}) + var span opentracing.Span + wireContext, err := tracer.Extract(opentracing.TextMap, metadataReaderWriter{md}) if err != nil { - span = tracer.StartSpan(operationName) - if err != opentracing.ErrTraceNotFound { - logger.Log("err", err) - } + logger.Log("err", err) } + span = tracer.StartSpan(operationName, ext.RPCServerOption(wireContext)) return opentracing.ContextWithSpan(ctx, span) } } diff --git a/tracing/opentracing/grpc_test.go b/tracing/opentracing/grpc_test.go index 2db9db373..c23b63b5d 100644 --- a/tracing/opentracing/grpc_test.go +++ b/tracing/opentracing/grpc_test.go @@ -8,19 +8,21 @@ import ( "golang.org/x/net/context" "google.golang.org/grpc/metadata" + "github.com/go-kit/kit/log" kitot "github.com/go-kit/kit/tracing/opentracing" ) func TestTraceGRPCRequestRoundtrip(t *testing.T) { + logger := log.NewNopLogger() tracer := mocktracer.New() // Initialize the ctx with a Span to inject. beforeSpan := tracer.StartSpan("to_inject").(*mocktracer.MockSpan) defer beforeSpan.Finish() - beforeSpan.SetBaggageItem("baggage", "check") + beforeSpan.Context().SetBaggageItem("baggage", "check") beforeCtx := opentracing.ContextWithSpan(context.Background(), beforeSpan) - toGRPCFunc := kitot.ToGRPCRequest(tracer, nil) + toGRPCFunc := kitot.ToGRPCRequest(tracer, logger) md := metadata.Pairs() // Call the RequestFunc. afterCtx := toGRPCFunc(beforeCtx, &md) @@ -38,22 +40,25 @@ func TestTraceGRPCRequestRoundtrip(t *testing.T) { } // Use FromGRPCRequest to verify that we can join with the trace given MD. - fromGRPCFunc := kitot.FromGRPCRequest(tracer, "joined", nil) + fromGRPCFunc := kitot.FromGRPCRequest(tracer, "joined", logger) joinCtx := fromGRPCFunc(afterCtx, &md) joinedSpan := opentracing.SpanFromContext(joinCtx).(*mocktracer.MockSpan) - if joinedSpan.SpanID == beforeSpan.SpanID { - t.Error("SpanID should have changed", joinedSpan.SpanID, beforeSpan.SpanID) + joinedContext := joinedSpan.Context().(*mocktracer.MockSpanContext) + beforeContext := beforeSpan.Context().(*mocktracer.MockSpanContext) + + if joinedContext.SpanID == beforeContext.SpanID { + t.Error("SpanID should have changed", joinedContext.SpanID, beforeContext.SpanID) } // Check that the parent/child relationship is as expected for the joined span. - if want, have := beforeSpan.SpanID, joinedSpan.ParentID; want != have { + if want, have := beforeContext.SpanID, joinedSpan.ParentID; want != have { t.Errorf("Want ParentID %q, have %q", want, have) } if want, have := "joined", joinedSpan.OperationName; want != have { t.Errorf("Want %q, have %q", want, have) } - if want, have := "check", joinedSpan.BaggageItem("baggage"); want != have { + if want, have := "check", joinedSpan.Context().BaggageItem("baggage"); want != have { t.Errorf("Want %q, have %q", want, have) } } diff --git a/tracing/opentracing/http.go b/tracing/opentracing/http.go index 63c59f8c7..49fb7113f 100644 --- a/tracing/opentracing/http.go +++ b/tracing/opentracing/http.go @@ -36,7 +36,7 @@ func ToHTTPRequest(tracer opentracing.Tracer, logger log.Logger) kithttp.Request // There's nothing we can do with any errors here. if err = tracer.Inject( - span, + span.Context(), opentracing.TextMap, opentracing.HTTPHeaderTextMapCarrier(req.Header), ); err != nil { @@ -57,17 +57,15 @@ func ToHTTPRequest(tracer opentracing.Tracer, logger log.Logger) kithttp.Request func FromHTTPRequest(tracer opentracing.Tracer, operationName string, logger log.Logger) kithttp.RequestFunc { return func(ctx context.Context, req *http.Request) context.Context { // Try to join to a trace propagated in `req`. - span, err := tracer.Join( - operationName, + var span opentracing.Span + wireContext, err := tracer.Extract( opentracing.TextMap, opentracing.HTTPHeaderTextMapCarrier(req.Header), ) if err != nil { - span = tracer.StartSpan(operationName) - if err != opentracing.ErrTraceNotFound { - logger.Log("err", err) - } + logger.Log("err", err) } + span = tracer.StartSpan(operationName, ext.RPCServerOption(wireContext)) return opentracing.ContextWithSpan(ctx, span) } } diff --git a/tracing/opentracing/http_test.go b/tracing/opentracing/http_test.go index a6f22bf78..5035db828 100644 --- a/tracing/opentracing/http_test.go +++ b/tracing/opentracing/http_test.go @@ -8,19 +8,21 @@ import ( "github.com/opentracing/opentracing-go/mocktracer" "golang.org/x/net/context" + "github.com/go-kit/kit/log" kitot "github.com/go-kit/kit/tracing/opentracing" ) func TestTraceHTTPRequestRoundtrip(t *testing.T) { + logger := log.NewNopLogger() tracer := mocktracer.New() // Initialize the ctx with a Span to inject. beforeSpan := tracer.StartSpan("to_inject").(*mocktracer.MockSpan) defer beforeSpan.Finish() - beforeSpan.SetBaggageItem("baggage", "check") + beforeSpan.Context().SetBaggageItem("baggage", "check") beforeCtx := opentracing.ContextWithSpan(context.Background(), beforeSpan) - toHTTPFunc := kitot.ToHTTPRequest(tracer, nil) + toHTTPFunc := kitot.ToHTTPRequest(tracer, logger) req, _ := http.NewRequest("GET", "http://test.biz/url", nil) // Call the RequestFunc. afterCtx := toHTTPFunc(beforeCtx, req) @@ -38,22 +40,25 @@ func TestTraceHTTPRequestRoundtrip(t *testing.T) { } // Use FromHTTPRequest to verify that we can join with the trace given a req. - fromHTTPFunc := kitot.FromHTTPRequest(tracer, "joined", nil) + fromHTTPFunc := kitot.FromHTTPRequest(tracer, "joined", logger) joinCtx := fromHTTPFunc(afterCtx, req) joinedSpan := opentracing.SpanFromContext(joinCtx).(*mocktracer.MockSpan) - if joinedSpan.SpanID == beforeSpan.SpanID { - t.Error("SpanID should have changed", joinedSpan.SpanID, beforeSpan.SpanID) + joinedContext := joinedSpan.Context().(*mocktracer.MockSpanContext) + beforeContext := beforeSpan.Context().(*mocktracer.MockSpanContext) + + if joinedContext.SpanID == beforeContext.SpanID { + t.Error("SpanID should have changed", joinedContext.SpanID, beforeContext.SpanID) } // Check that the parent/child relationship is as expected for the joined span. - if want, have := beforeSpan.SpanID, joinedSpan.ParentID; want != have { + if want, have := beforeContext.SpanID, joinedSpan.ParentID; want != have { t.Errorf("Want ParentID %q, have %q", want, have) } if want, have := "joined", joinedSpan.OperationName; want != have { t.Errorf("Want %q, have %q", want, have) } - if want, have := "check", joinedSpan.BaggageItem("baggage"); want != have { + if want, have := "check", joinedSpan.Context().BaggageItem("baggage"); want != have { t.Errorf("Want %q, have %q", want, have) } }