Remove resampling on span.SetName (#1545)

The spec makes it optional to attempt resampling when changing the name
of a span and we're not sure whether it can be done in an appropriate
manner, so it's best to not do it at all for now.  We can try again
later if we find a good way to do it.

CHANGELOG.md

@@ -18,6 +18,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 - Added `resource.Default()` for use with meter and tracer providers. (#1507)
 - Added `Keys()` method to `propagation.TextMapCarrier` and `propagation.HeaderCarrier` to adapt `http.Header` to this interface. (#1544)
 
+### Removed
+
+- Removed attempt to resample spans upon changing the span name with `span.SetName()`. (#1545)
+
 ## [0.17.0] - 2020-02-12
 
 ### Changed

sdk/trace/span.go

@@ -302,35 +302,7 @@ func (s *span) SetName(name string) {
 
 	s.mu.Lock()
 	defer s.mu.Unlock()
-
 	s.name = name
-	// SAMPLING
-	noParent := !s.parent.SpanID.IsValid()
-	var ctx trace.SpanContext
-	if noParent {
-		ctx = trace.SpanContext{}
-	} else {
-		// FIXME: Where do we get the parent context from?
-		ctx = s.spanContext
-	}
-	data := samplingData{
-		noParent:     noParent,
-		remoteParent: s.hasRemoteParent,
-		parent:       ctx,
-		name:         name,
-		cfg:          s.tracer.provider.config.Load().(*Config),
-		span:         s,
-		attributes:   s.attributes.toKeyValue(),
-		links:        s.interfaceArrayToLinksArray(),
-		kind:         s.spanKind,
-	}
-	sampled := makeSamplingDecision(data)
-
-	// Adding attributes directly rather than using s.SetAttributes()
-	// as s.mu is already locked and attempting to do so would deadlock.
-	for _, a := range sampled.Attributes {
-		s.attributes.add(a)
-	}
 }
 
 // Name returns the name of this span.

sdk/trace/trace_test.go

@@ -158,58 +158,47 @@ func (ts testSampler) Description() string {
 }
 
 func TestSetName(t *testing.T) {
-	fooSampler := &testSampler{prefix: "foo", t: t}
-	tp := NewTracerProvider(WithConfig(Config{DefaultSampler: fooSampler}))
+	tp := NewTracerProvider()
 
 	type testCase struct {
-		name          string
-		newName       string
-		sampledBefore bool
-		sampledAfter  bool
+		name    string
+		newName string
 	}
 	for idx, tt := range []testCase{
 		{ // 0
-			name:          "foobar",
-			newName:       "foobaz",
-			sampledBefore: true,
-			sampledAfter:  true,
+			name:    "foobar",
+			newName: "foobaz",
 		},
 		{ // 1
-			name:          "foobar",
-			newName:       "barbaz",
-			sampledBefore: true,
-			sampledAfter:  false,
+			name:    "foobar",
+			newName: "barbaz",
 		},
 		{ // 2
-			name:          "barbar",
-			newName:       "barbaz",
-			sampledBefore: false,
-			sampledAfter:  false,
+			name:    "barbar",
+			newName: "barbaz",
 		},
 		{ // 3
-			name:          "barbar",
-			newName:       "foobar",
-			sampledBefore: false,
-			sampledAfter:  true,
+			name:    "barbar",
+			newName: "foobar",
 		},
 	} {
-		span := startNamedSpan(tp, "SetName", tt.name)
-		if fooSampler.callCount == 0 {
-			t.Errorf("%d: the sampler was not even called during span creation", idx)
+		sp := startNamedSpan(tp, "SetName", tt.name)
+		if sdkspan, ok := sp.(*span); ok {
+			if sdkspan.Name() != tt.name {
+				t.Errorf("%d: invalid name at span creation, expected %v, got %v", idx, tt.name, sdkspan.Name())
+			}
+		} else {
+			t.Errorf("%d: unable to coerce span to SDK span, is type %T", idx, sp)
 		}
-		fooSampler.callCount = 0
-		if gotSampledBefore := span.SpanContext().IsSampled(); tt.sampledBefore != gotSampledBefore {
-			t.Errorf("%d: invalid sampling decision before rename, expected %v, got %v", idx, tt.sampledBefore, gotSampledBefore)
+		sp.SetName(tt.newName)
+		if sdkspan, ok := sp.(*span); ok {
+			if sdkspan.Name() != tt.newName {
+				t.Errorf("%d: span name not changed, expected %v, got %v", idx, tt.newName, sdkspan.Name())
+			}
+		} else {
+			t.Errorf("%d: unable to coerce span to SDK span, is type %T", idx, sp)
 		}
-		span.SetName(tt.newName)
-		if fooSampler.callCount == 0 {
-			t.Errorf("%d: the sampler was not even called during span rename", idx)
-		}
-		fooSampler.callCount = 0
-		if gotSampledAfter := span.SpanContext().IsSampled(); tt.sampledAfter != gotSampledAfter {
-			t.Errorf("%d: invalid sampling decision after rename, expected %v, got %v", idx, tt.sampledAfter, gotSampledAfter)
-		}
-		span.End()
+		sp.End()
 	}
 }