From 1b5b66213656691e3528909fe39fa030eae15423 Mon Sep 17 00:00:00 2001
From: Anthony Mirabella <a9@aneurysm9.com>
Date: Thu, 18 Feb 2021 12:49:10 -0500
Subject: [PATCH] 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            |  4 +++
 sdk/trace/span.go       | 28 ------------------
 sdk/trace/trace_test.go | 63 +++++++++++++++++------------------------
 3 files changed, 30 insertions(+), 65 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 077abba159d..147406ef4e2 100644
--- a/CHANGELOG.md
+++ b/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
diff --git a/sdk/trace/span.go b/sdk/trace/span.go
index 4fe4785d146..388238e25fa 100644
--- a/sdk/trace/span.go
+++ b/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.
diff --git a/sdk/trace/trace_test.go b/sdk/trace/trace_test.go
index fe19a638e6c..d3934574a91 100644
--- a/sdk/trace/trace_test.go
+++ b/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)
-		}
-		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)
-		}
-		span.SetName(tt.newName)
-		if fooSampler.callCount == 0 {
-			t.Errorf("%d: the sampler was not even called during span rename", 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 gotSampledAfter := span.SpanContext().IsSampled(); tt.sampledAfter != gotSampledAfter {
-			t.Errorf("%d: invalid sampling decision after rename, expected %v, got %v", idx, tt.sampledAfter, gotSampledAfter)
+		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.End()
+		sp.End()
 	}
 }