Deprecate NewSpanXXX in the favor of StartSpan

Required to support the Go execution tracer spans.

Fixes #694.

exporter/stackdriver/stackdriver_test.go

@@ -53,14 +53,13 @@ func TestExport(t *testing.T) {
 
 	trace.ApplyConfig(trace.Config{DefaultSampler: trace.AlwaysSample()})
 
-	span := trace.NewSpan("custom-span", nil, trace.StartOptions{})
+	_, span := trace.StartSpan(context.Background(), "custom-span")
 	time.Sleep(10 * time.Millisecond)
 	span.End()
 
 	// Test HTTP spans
 
 	handler := http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
-
 		_, backgroundSpan := trace.StartSpan(context.Background(), "BackgroundWork")
 		spanContext := backgroundSpan.SpanContext()
 		time.Sleep(10 * time.Millisecond)

exporter/stackdriver/stats.go

@@ -155,12 +155,11 @@ func (e *statsExporter) Flush() {
 }
 
 func (e *statsExporter) uploadStats(vds []*view.Data) error {
-	span := trace.NewSpan(
+	ctx, span := trace.StartSpan(
+		context.Background(),
 		"go.opencensus.io/exporter/stackdriver.uploadStats",
-		nil,
-		trace.StartOptions{Sampler: trace.NeverSample()},
+		trace.WithSampler(trace.NeverSample()),
 	)
-	ctx := trace.WithSpan(context.Background(), span)
 	defer span.End()
 
 	for _, vd := range vds {

exporter/stackdriver/trace.go

@@ -117,11 +117,14 @@ func (e *traceExporter) uploadSpans(spans []*trace.SpanData) {
 		req.Spans = append(req.Spans, protoFromSpanData(span, e.projectID))
 	}
 	// Create a never-sampled span to prevent traces associated with exporter.
-	span := trace.NewSpan("go.opencensus.io/exporter/stackdriver.uploadSpans", nil, trace.StartOptions{Sampler: trace.NeverSample()})
+	ctx, span := trace.StartSpan( // TODO: add timeouts
+		context.Background(),
+		"go.opencensus.io/exporter/stackdriver.uploadSpans",
+		trace.WithSampler(trace.NeverSample()),
+	)
 	defer span.End()
 	span.AddAttributes(trace.Int64Attribute("num_spans", int64(len(spans))))
 
-	ctx := trace.WithSpan(context.Background(), span) // TODO: add timeouts
 	err := e.client.BatchWriteSpans(ctx, &req)
 	if err != nil {
 		span.SetStatus(trace.Status{Code: 2, Message: err.Error()})

exporter/stackdriver/trace_proto_test.go

@@ -57,19 +57,21 @@ func (t *testExporter) ExportSpan(s *trace.SpanData) {
 }
 
 func TestExportTrace(t *testing.T) {
+	ctx := context.Background()
+
 	var te testExporter
 	trace.RegisterExporter(&te)
 	defer trace.UnregisterExporter(&te)
 
-	span0 := trace.NewSpanWithRemoteParent(
+	ctx, span0 := trace.StartSpan(
+		ctx,
 		"span0",
-		trace.SpanContext{
+		trace.WithRemoteParent(trace.SpanContext{
 			TraceID:      traceID,
 			SpanID:       spanID,
 			TraceOptions: 1,
-		},
-		trace.StartOptions{})
-	ctx := trace.WithSpan(context.Background(), span0)
+		}),
+	)
 	{
 		ctx1, span1 := trace.StartSpan(ctx, "span1")
 		{

exporter/stackdriver/trace_test.go

@@ -35,9 +35,8 @@ func TestBundling(t *testing.T) {
 	}
 	trace.RegisterExporter(exporter)
 
-	trace.ApplyConfig(trace.Config{DefaultSampler: trace.AlwaysSample()})
 	for i := 0; i < 35; i++ {
-		_, span := trace.StartSpan(context.Background(), "span")
+		_, span := trace.StartSpan(context.Background(), "span", trace.WithSampler(trace.AlwaysSample()))
 		span.End()
 	}
 

plugin/ocgrpc/grpc_test.go

@@ -35,10 +35,7 @@ func TestClientHandler(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	span := trace.NewSpan("/foo", nil, trace.StartOptions{
-		Sampler: trace.AlwaysSample(),
-	})
-	ctx = trace.WithSpan(ctx, span)
+	ctx, _ = trace.StartSpan(ctx, "/foo", trace.WithSampler(trace.AlwaysSample()))
 
 	var handler ClientHandler
 	ctx = handler.TagRPC(ctx, &stats.RPCTagInfo{

plugin/ocgrpc/trace_common.go

@@ -36,11 +36,9 @@ const traceContextKey = "grpc-trace-bin"
 func (c *ClientHandler) traceTagRPC(ctx context.Context, rti *stats.RPCTagInfo) context.Context {
 	name := strings.TrimPrefix(rti.FullMethodName, "/")
 	name = strings.Replace(name, "/", ".", -1)
-	span := trace.NewSpan(name, trace.FromContext(ctx), trace.StartOptions{
-		Sampler:  c.StartOptions.Sampler,
-		SpanKind: trace.SpanKindClient,
-	}) // span is ended by traceHandleRPC
-	ctx = trace.WithSpan(ctx, span)
+	ctx, span := trace.StartSpan(ctx, name,
+		trace.WithSampler(c.StartOptions.Sampler),
+		trace.WithSpanKind(trace.SpanKindClient)) // span is ended by traceHandleRPC
 	traceContextBinary := propagation.Binary(span.SpanContext())
 	return metadata.AppendToOutgoingContext(ctx, traceContextKey, string(traceContextBinary))
 }
@@ -52,11 +50,6 @@ func (c *ClientHandler) traceTagRPC(ctx context.Context, rti *stats.RPCTagInfo)
 //
 // It returns ctx, with the new trace span added.
 func (s *ServerHandler) traceTagRPC(ctx context.Context, rti *stats.RPCTagInfo) context.Context {
-	opts := trace.StartOptions{
-		Sampler:  s.StartOptions.Sampler,
-		SpanKind: trace.SpanKindServer,
-	}
-
 	md, _ := metadata.FromIncomingContext(ctx)
 	name := strings.TrimPrefix(rti.FullMethodName, "/")
 	name = strings.Replace(name, "/", ".", -1)
@@ -72,15 +65,21 @@ func (s *ServerHandler) traceTagRPC(ctx context.Context, rti *stats.RPCTagInfo)
 		traceContextBinary := []byte(traceContext[0])
 		parent, haveParent = propagation.FromBinary(traceContextBinary)
 		if haveParent && !s.IsPublicEndpoint {
-			span := trace.NewSpanWithRemoteParent(name, parent, opts)
-			return trace.WithSpan(ctx, span)
+			ctx, _ := trace.StartSpan(ctx, name,
+				trace.WithRemoteParent(parent),
+				trace.WithSpanKind(trace.SpanKindServer),
+				trace.WithSampler(s.StartOptions.Sampler),
+			)
+			return ctx
 		}
 	}
-	span := trace.NewSpan(name, nil, opts)
+	ctx, span := trace.StartSpan(ctx, name,
+		trace.WithSpanKind(trace.SpanKindServer),
+		trace.WithSampler(s.StartOptions.Sampler))
 	if haveParent {
 		span.AddLink(trace.Link{TraceID: parent.TraceID, SpanID: parent.SpanID, Type: trace.LinkTypeChild})
 	}
-	return trace.WithSpan(ctx, span)
+	return ctx
 }
 
 func traceHandleRPC(ctx context.Context, rs stats.RPCStats) {

plugin/ochttp/server.go

@@ -76,20 +76,20 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 }
 
 func (h *Handler) startTrace(w http.ResponseWriter, r *http.Request) (*http.Request, func()) {
-	opts := trace.StartOptions{
-		Sampler:  h.StartOptions.Sampler,
-		SpanKind: trace.SpanKindServer,
-	}
-
 	name := spanNameFromURL(r.URL)
 	ctx := r.Context()
 	var span *trace.Span
 	sc, ok := h.extractSpanContext(r)
 	if ok && !h.IsPublicEndpoint {
-		span = trace.NewSpanWithRemoteParent(name, sc, opts)
-		ctx = trace.WithSpan(ctx, span)
+		ctx, span = trace.StartSpan(ctx, name,
+			trace.WithRemoteParent(sc),
+			trace.WithSampler(h.StartOptions.Sampler),
+			trace.WithSpanKind(trace.SpanKindServer))
 	} else {
-		span = trace.NewSpan(name, nil, opts)
+		ctx, span = trace.StartSpan(ctx, name,
+			trace.WithSampler(h.StartOptions.Sampler),
+			trace.WithSpanKind(trace.SpanKindServer),
+		)
 		if ok {
 			span.AddLink(trace.Link{
 				TraceID:    sc.TraceID,
@@ -99,9 +99,8 @@ func (h *Handler) startTrace(w http.ResponseWriter, r *http.Request) (*http.Requ
 			})
 		}
 	}
-	ctx = trace.WithSpan(ctx, span)
 	span.AddAttributes(requestAttrs(r)...)
-	return r.WithContext(trace.WithSpan(r.Context(), span)), span.End
+	return r.WithContext(ctx), span.End
 }
 
 func (h *Handler) extractSpanContext(r *http.Request) (trace.SpanContext, bool) {

plugin/ochttp/trace.go

@@ -53,10 +53,11 @@ func (t *traceTransport) RoundTrip(req *http.Request) (*http.Response, error) {
 	name := spanNameFromURL(req.URL)
 	// TODO(jbd): Discuss whether we want to prefix
 	// outgoing requests with Sent.
-	parent := trace.FromContext(req.Context())
-	span := trace.NewSpan(name, parent, t.startOptions)
-	req = req.WithContext(trace.WithSpan(req.Context(), span))
+	_, span := trace.StartSpan(req.Context(), name,
+		trace.WithSampler(t.startOptions.Sampler),
+		trace.WithSpanKind(trace.SpanKindClient))
 
+	req = req.WithContext(trace.WithSpan(req.Context(), span))
 	if t.format != nil {
 		t.format.SpanContextToRequest(span.SpanContext(), req)
 	}

plugin/ochttp/trace_test.go

@@ -73,7 +73,8 @@ func (t testPropagator) SpanContextToRequest(sc trace.SpanContext, req *http.Req
 }
 
 func TestTransport_RoundTrip(t *testing.T) {
-	parent := trace.NewSpan("parent", nil, trace.StartOptions{})
+	ctx := context.Background()
+	ctx, parent := trace.StartSpan(ctx, "parent")
 	tests := []struct {
 		name   string
 		parent *trace.Span
@@ -221,12 +222,9 @@ func TestEndToEnd(t *testing.T) {
 			url := serveHTTP(tt.handler, serverDone, serverReturn)
 
 			// Start a root Span in the client.
-			root := trace.NewSpan(
-				"top-level",
-				nil,
-				trace.StartOptions{})
-			ctx := trace.WithSpan(context.Background(), root)
-
+			ctx, root := trace.StartSpan(
+				context.Background(),
+				"top-level")
 			// Make the request.
 			req, err := http.NewRequest(
 				http.MethodPost,
@@ -278,7 +276,7 @@ func TestEndToEnd(t *testing.T) {
 						t.Errorf("Span name: %q; want %q", got, want)
 					}
 				default:
-					t.Fatalf("server or client span missing")
+					t.Fatalf("server or client span missing; kind = %v", sp.SpanKind)
 				}
 			}
 

trace/trace.go

@@ -123,34 +123,76 @@ type StartOptions struct {
 	// SpanKind represents the kind of a span. If none is set,
 	// SpanKindUnspecified is used.
 	SpanKind int
+
+	remoteParent SpanContext
+}
+
+// StartOption apply changes to StartOptions.
+type StartOption func(*StartOptions)
+
+// WithSpanKind makes new spans to be created with the given kind.
+func WithSpanKind(spanKind int) StartOption {
+	return func(o *StartOptions) {
+		o.SpanKind = spanKind
+	}
+}
+
+// WithSampler makes new spans to be be created with a custom sampler.
+// Otherwise, the global sampler is used.
+func WithSampler(sampler Sampler) StartOption {
+	return func(o *StartOptions) {
+		o.Sampler = sampler
+	}
+}
+
+// WithRemoteParent makes new spans to be the child of the given part.
+func WithRemoteParent(parent SpanContext) StartOption {
+	return func(o *StartOptions) {
+		o.remoteParent = parent
+	}
 }
 
 // StartSpan starts a new child span of the current span in the context. If
 // there is no span in the context, creates a new trace and span.
 //
-// This is provided as a convenience for WithSpan(ctx, NewSpan(...)). Use it
-// if you require custom spans in addition to the default spans provided by
-// ocgrpc, ochttp or similar framework integration.
-func StartSpan(ctx context.Context, name string) (context.Context, *Span) {
-	parentSpan, _ := ctx.Value(contextKey{}).(*Span)
-	span := NewSpan(name, parentSpan, StartOptions{})
+// If WithRemoteParent option is provided, started span will be a child
+// of the given remote parent.
+func StartSpan(ctx context.Context, name string, o ...StartOption) (context.Context, *Span) {
+	var opts StartOptions
+
+	var parent SpanContext
+	if p := FromContext(ctx); p != nil {
+		parent = p.spanContext
+	}
+	for _, op := range o {
+		op(&opts)
+	}
+
+	var span *Span
+	if opts.remoteParent != (SpanContext{}) {
+		span = startSpanInternal(name, true, opts.remoteParent, true, opts)
+	} else {
+		span = startSpanInternal(name, parent != SpanContext{}, parent, false, opts)
+	}
 	return WithSpan(ctx, span), span
 }
 
 // NewSpan returns a new span.
 //
 // If parent is not nil, created span will be a child of the parent.
+//
+// Deprecated: Use StartSpan.
 func NewSpan(name string, parent *Span, o StartOptions) *Span {
-	hasParent := false
 	var parentSpanContext SpanContext
 	if parent != nil {
-		hasParent = true
 		parentSpanContext = parent.SpanContext()
 	}
-	return startSpanInternal(name, hasParent, parentSpanContext, false, o)
+	return startSpanInternal(name, parent != nil, parentSpanContext, false, o)
 }
 
 // NewSpanWithRemoteParent returns a new span with the given parent SpanContext.
+//
+// Deprecated: Use StartSpan with WithRemoteParent.
 func NewSpanWithRemoteParent(name string, parent SpanContext, o StartOptions) *Span {
 	return startSpanInternal(name, true, parent, true, o)
 }