Skip to content
This repository has been archived by the owner on Jul 31, 2023. It is now read-only.

Commit

Permalink
Remove deprecated trace.StartSpanWithXXX convenience methods
Browse files Browse the repository at this point in the history
  • Loading branch information
Ramon Nogueira committed Mar 9, 2018
1 parent 2ecd8d9 commit a250d24
Show file tree
Hide file tree
Showing 7 changed files with 41 additions and 52 deletions.
2 changes: 1 addition & 1 deletion examples/grpc/helloworld_server/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func main() {

// Set up a new server with the OpenCensus
// stats handler to enable stats and tracing.
s := grpc.NewServer(grpc.StatsHandler(ocgrpc.NewServerStatsHandler()))
s := grpc.NewServer(grpc.StatsHandler(new(ocgrpc.ServerHandler)))
pb.RegisterGreeterServer(s, &server{})
// Register reflection service on gRPC server.
reflection.Register(s)
Expand Down
7 changes: 4 additions & 3 deletions plugin/ocgrpc/grpc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,20 +72,21 @@ func TestNewClientStatsHandler(t *testing.T) {
}

func TestNewServerStatsHandler(t *testing.T) {
ctx := context.Background()

handler := NewServerStatsHandler()

ctx := context.Background()
te := &traceExporter{}
trace.RegisterExporter(te)
if err := ServerRequestCountView.Subscribe(); err != nil {
t.Fatal(err)
}

// start a span here to ensure there is a sampled trace in the context
span := trace.NewSpan("/foo", nil, trace.StartOptions{
Sampler: trace.AlwaysSample(),
})
ctx = trace.WithSpan(ctx, span)

var handler ServerHandler
ctx = handler.TagRPC(ctx, &stats.RPCTagInfo{
FullMethodName: "/service.foo/method",
})
Expand Down
6 changes: 0 additions & 6 deletions plugin/ocgrpc/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,6 @@ import (
"google.golang.org/grpc/stats"
)

// NewServerStatsHandler enables OpenCensus stats and trace
// for gRPC servers. Deprecated, construct a ServerHandler directly.
func NewServerStatsHandler() stats.Handler {
return &ServerHandler{}
}

// ServerHandler implements gRPC stats.Handler recording OpenCensus stats and
// traces. Use with gRPC servers.
type ServerHandler struct {
Expand Down
11 changes: 8 additions & 3 deletions plugin/ocgrpc/trace_common.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,13 +58,18 @@ func (c *clientTraceHandler) TagRPC(ctx context.Context, rti *stats.RPCTagInfo)
func (s *serverTraceHandler) TagRPC(ctx context.Context, rti *stats.RPCTagInfo) context.Context {
md, _ := metadata.FromIncomingContext(ctx)
name := "Recv" + strings.Replace(rti.FullMethodName, "/", ".", -1)
var span *trace.Span
if s := md[traceContextKey]; len(s) > 0 {
if parent, ok := propagation.FromBinary([]byte(s[0])); ok {
ctx, _ = trace.StartSpanWithRemoteParent(ctx, name, parent, trace.StartOptions{})
return ctx
span = trace.NewSpanWithRemoteParent(name, parent, trace.StartOptions{})
}
}
ctx, _ = trace.StartSpan(ctx, name)
if span == nil {
// TODO(ramonza): should we ignore the in-process parent here?
ctx, span = trace.StartSpan(ctx, name)
} else {
ctx = trace.WithSpan(ctx, span)
}
return ctx
}

Expand Down
6 changes: 3 additions & 3 deletions plugin/ochttp/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,11 +80,11 @@ func (h *Handler) startTrace(w http.ResponseWriter, r *http.Request) (*http.Requ
ctx := r.Context()
var span *trace.Span
if sc, ok := p.SpanContextFromRequest(r); ok {
ctx, span = trace.StartSpanWithRemoteParent(ctx, name, sc, trace.StartOptions{})
span = trace.NewSpanWithRemoteParent(name, sc, trace.StartOptions{})
} else {
ctx, span = trace.StartSpan(ctx, name)
span = trace.NewSpan(name, nil, trace.StartOptions{})
}

ctx = trace.WithSpan(ctx, span)
span.SetAttributes(requestAttrs(r)...)
return r.WithContext(trace.WithSpan(r.Context(), span)), span.End
}
Expand Down
22 changes: 0 additions & 22 deletions trace/trace.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,28 +127,6 @@ func StartSpan(ctx context.Context, name string) (context.Context, *Span) {
return WithSpan(ctx, span), span
}

// StartSpanWithOptions 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.
//
// Deprecated: Use StartSpan(...), or WithSpan(ctx, NewSpan(...)).
func StartSpanWithOptions(ctx context.Context, name string, o StartOptions) (context.Context, *Span) {
parentSpan, _ := ctx.Value(contextKey{}).(*Span)
span := NewSpan(name, parentSpan, o)
return WithSpan(ctx, span), span
}

// StartSpanWithRemoteParent starts a new child span with the given parent SpanContext.
//
// If there is an existing span in ctx, it is ignored -- the returned Span is a
// child of the span specified by parent.
//
// Deprecated: Use WithSpan(ctx, NewSpanWithRemoteParent(...)).
func StartSpanWithRemoteParent(ctx context.Context, name string, parent SpanContext, o StartOptions) (context.Context, *Span) {
span := NewSpanWithRemoteParent(name, parent, o)
return WithSpan(ctx, span), span
}

// NewSpan returns a new span.
//
// If parent is not nil, created span will be a child of the parent.
Expand Down
39 changes: 25 additions & 14 deletions trace/trace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,20 +109,20 @@ func TestSampling(t *testing.T) {
SpanID: sid,
TraceOptions: test.parentTraceOptions,
}
ctx, _ = StartSpanWithRemoteParent(context.Background(), "foo", sc, StartOptions{
ctx, _ = startSpanWithRemoteParent(context.Background(), "foo", sc, StartOptions{
Sampler: test.sampler,
})
} else if test.localParent {
sampler := NeverSample()
if test.parentTraceOptions == 1 {
sampler = AlwaysSample()
}
ctx2, _ := StartSpanWithOptions(context.Background(), "foo", StartOptions{Sampler: sampler})
ctx, _ = StartSpanWithOptions(ctx2, "foo", StartOptions{
ctx2, _ := startSpanWithOptions(context.Background(), "foo", StartOptions{Sampler: sampler})
ctx, _ = startSpanWithOptions(ctx2, "foo", StartOptions{
Sampler: test.sampler,
})
} else {
ctx, _ = StartSpanWithOptions(context.Background(), "foo", StartOptions{
ctx, _ = startSpanWithOptions(context.Background(), "foo", StartOptions{
Sampler: test.sampler,
})
}
Expand Down Expand Up @@ -159,7 +159,7 @@ func TestSampling(t *testing.T) {
if test.parentTraceOptions == 1 {
sampler = AlwaysSample()
}
ctx2, _ := StartSpanWithOptions(context.Background(), "foo", StartOptions{Sampler: sampler})
ctx2, _ := startSpanWithOptions(context.Background(), "foo", StartOptions{Sampler: sampler})
ctx, _ := StartSpan(ctx2, "foo")
sc := FromContext(ctx).SpanContext()
if (sc == SpanContext{}) {
Expand All @@ -180,7 +180,7 @@ func TestSampling(t *testing.T) {
func TestProbabilitySampler(t *testing.T) {
exported := 0
for i := 0; i < 1000; i++ {
_, span := StartSpanWithOptions(context.Background(), "foo", StartOptions{
_, span := startSpanWithOptions(context.Background(), "foo", StartOptions{
Sampler: ProbabilitySampler(0.3),
})
if span.SpanContext().IsSampled() {
Expand All @@ -192,18 +192,23 @@ func TestProbabilitySampler(t *testing.T) {
}
}

func startSpanWithRemoteParent(ctx context.Context, name string, parent SpanContext, o StartOptions) (context.Context, *Span) {
span := NewSpanWithRemoteParent(name, parent, o)
return WithSpan(ctx, span), span
}

func TestStartSpanWithRemoteParent(t *testing.T) {
sc := SpanContext{
TraceID: tid,
SpanID: sid,
TraceOptions: 0x0,
}
ctx, _ := StartSpanWithRemoteParent(context.Background(), "StartSpanWithRemoteParent", sc, StartOptions{})
ctx, _ := startSpanWithRemoteParent(context.Background(), "startSpanWithRemoteParent", sc, StartOptions{})
if err := checkChild(sc, FromContext(ctx)); err != nil {
t.Error(err)
}

ctx, _ = StartSpanWithRemoteParent(context.Background(), "StartSpanWithRemoteParent", sc, StartOptions{})
ctx, _ = startSpanWithRemoteParent(context.Background(), "startSpanWithRemoteParent", sc, StartOptions{})
if err := checkChild(sc, FromContext(ctx)); err != nil {
t.Error(err)
}
Expand All @@ -213,12 +218,12 @@ func TestStartSpanWithRemoteParent(t *testing.T) {
SpanID: sid,
TraceOptions: 0x1,
}
ctx, _ = StartSpanWithRemoteParent(context.Background(), "StartSpanWithRemoteParent", sc, StartOptions{})
ctx, _ = startSpanWithRemoteParent(context.Background(), "startSpanWithRemoteParent", sc, StartOptions{})
if err := checkChild(sc, FromContext(ctx)); err != nil {
t.Error(err)
}

ctx, _ = StartSpanWithRemoteParent(context.Background(), "StartSpanWithRemoteParent", sc, StartOptions{})
ctx, _ = startSpanWithRemoteParent(context.Background(), "startSpanWithRemoteParent", sc, StartOptions{})
if err := checkChild(sc, FromContext(ctx)); err != nil {
t.Error(err)
}
Expand Down Expand Up @@ -510,12 +515,18 @@ func (e exporter) ExportSpan(s *SpanData) {
e[s.Name] = s
}

func startSpanWithOptions(ctx context.Context, name string, o StartOptions) (context.Context, *Span) {
parentSpan, _ := ctx.Value(contextKey{}).(*Span)
span := NewSpan(name, parentSpan, o)
return WithSpan(ctx, span), span
}

func Test_Issue328_EndSpanTwice(t *testing.T) {
spans := make(exporter)
RegisterExporter(&spans)
defer UnregisterExporter(&spans)
ctx := context.Background()
ctx, span := StartSpanWithOptions(ctx, "span-1", StartOptions{Sampler: AlwaysSample()})
ctx, span := startSpanWithOptions(ctx, "span-1", StartOptions{Sampler: AlwaysSample()})
span.End()
span.End()
UnregisterExporter(&spans)
Expand All @@ -528,12 +539,12 @@ func TestStartSpanAfterEnd(t *testing.T) {
spans := make(exporter)
RegisterExporter(&spans)
defer UnregisterExporter(&spans)
ctx, span0 := StartSpanWithOptions(context.Background(), "parent", StartOptions{Sampler: AlwaysSample()})
ctx1, span1 := StartSpanWithOptions(ctx, "span-1", StartOptions{Sampler: AlwaysSample()})
ctx, span0 := startSpanWithOptions(context.Background(), "parent", StartOptions{Sampler: AlwaysSample()})
ctx1, span1 := startSpanWithOptions(ctx, "span-1", StartOptions{Sampler: AlwaysSample()})
span1.End()
// Start a new span with the context containing span-1
// even though span-1 is ended, we still add this as a new child of span-1
_, span2 := StartSpanWithOptions(ctx1, "span-2", StartOptions{Sampler: AlwaysSample()})
_, span2 := startSpanWithOptions(ctx1, "span-2", StartOptions{Sampler: AlwaysSample()})
span2.End()
span0.End()
UnregisterExporter(&spans)
Expand Down

0 comments on commit a250d24

Please sign in to comment.