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 9b97f91
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 41 deletions.
5 changes: 3 additions & 2 deletions plugin/ocgrpc/trace_common.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,11 @@ func (s *serverTraceHandler) TagRPC(ctx context.Context, rti *stats.RPCTagInfo)
name := "Recv" + strings.Replace(rti.FullMethodName, "/", ".", -1)
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{})
return trace.WithSpan(ctx, span)
}
}
// TODO(ramonza): should we ignore the in-process parent here?
ctx, _ = trace.StartSpan(ctx, name)
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 9b97f91

Please sign in to comment.