Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove resampling on span.SetName #1545

Merged
merged 1 commit into from
Feb 18, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
28 changes: 0 additions & 28 deletions sdk/trace/span.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
63 changes: 26 additions & 37 deletions sdk/trace/trace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
}

Expand Down