Skip to content

Commit

Permalink
Remove Tracer name prefix for Span names (#430)
Browse files Browse the repository at this point in the history
Spans should not have the Tracer name as a prefix for their names. This
removes the `spanNameWithPrefix` function and instead passes through the
span name unmodified wherever this had been called.

Tests that checked Span names are updated to have the non-prefix
expected names.
  • Loading branch information
MrAlias authored and rghetia committed Jan 14, 2020
1 parent d85178b commit 2d5ba32
Show file tree
Hide file tree
Showing 5 changed files with 62 additions and 39 deletions.
2 changes: 1 addition & 1 deletion api/global/internal/trace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ func TestTraceDefaultSDK(t *testing.T) {
t.Errorf("failed to wrap function with span post initialization with new tracer: %v", err)
}

expected := []string{"pre/span2", "pre/withSpan2", "post/span3", "post/withSpan3"}
expected := []string{"span2", "withSpan2", "span3", "withSpan3"}
require.Equal(t, tsp.spansStarted, expected)
require.Equal(t, tsp.spansEnded, expected)
}
14 changes: 7 additions & 7 deletions plugin/httptrace/clienttrace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,24 +91,24 @@ func TestHTTPRequestWithClientTrace(t *testing.T) {
attributes []core.KeyValue
}{
{
name: "go.opentelemetry.io/otel/plugin/httptrace/http.connect",
name: "http.connect",
attributes: []core.KeyValue{key.String("http.remote", address.String())},
},
{
name: "go.opentelemetry.io/otel/plugin/httptrace/http.getconn",
name: "http.getconn",
attributes: []core.KeyValue{
key.String("http.remote", address.String()),
key.String("http.host", address.String()),
},
},
{
name: "go.opentelemetry.io/otel/plugin/httptrace/http.receive",
name: "http.receive",
},
{
name: "go.opentelemetry.io/otel/plugin/httptrace/http.send",
name: "http.send",
},
{
name: "httptrace/client/test",
name: "test",
},
}
for _, tl := range testLen {
Expand All @@ -132,7 +132,7 @@ func TestHTTPRequestWithClientTrace(t *testing.T) {
expectedAttrs[attr.Key] = attr.Value.Emit()
}

if tl.name == "go.opentelemetry.io/otel/plugin/httptrace/http.getconn" {
if tl.name == "http.getconn" {
local := key.New("http.local")
// http.local attribute is not deterministic, just make sure it exists for `getconn`.
if _, ok := actualAttrs[local]; ok {
Expand Down Expand Up @@ -232,7 +232,7 @@ func TestConcurrentConnectionStart(t *testing.T) {
for _, tt := range tts {
t.Run(tt.name, func(t *testing.T) {
tt.run()
spans := exp.spanMap["go.opentelemetry.io/otel/plugin/httptrace/http.connect"]
spans := exp.spanMap["http.connect"]

if l := len(spans); l != 2 {
t.Fatalf("Expected 2 'http.connect' traces but found %d", l)
Expand Down
5 changes: 2 additions & 3 deletions sdk/trace/span.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,8 +159,7 @@ func (s *span) SetName(name string) {
// TODO: now what?
return
}
spanName := s.tracer.spanNameWithPrefix(name)
s.data.Name = spanName
s.data.Name = name
// SAMPLING
noParent := !s.data.ParentSpanID.IsValid()
var ctx core.SpanContext
Expand All @@ -175,7 +174,7 @@ func (s *span) SetName(name string) {
noParent: noParent,
remoteParent: s.data.HasRemoteParent,
parent: ctx,
name: spanName,
name: name,
cfg: s.tracer.provider.config.Load().(*Config),
span: s,
}
Expand Down
68 changes: 50 additions & 18 deletions sdk/trace/trace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ func TestSetName(t *testing.T) {
fooSampler := Sampler(func(p SamplingParameters) SamplingDecision {
samplerIsCalled = true
t.Logf("called sampler for name %q", p.Name)
return SamplingDecision{Sample: strings.HasPrefix(p.Name, "SetName/foo")}
return SamplingDecision{Sample: strings.HasPrefix(p.Name, "foo")}
})
tp, _ := NewProvider(WithConfig(Config{DefaultSampler: fooSampler}))

Expand Down Expand Up @@ -311,7 +311,7 @@ func TestSetSpanAttributesOnStart(t *testing.T) {
TraceFlags: 0x1,
},
ParentSpanID: sid,
Name: "StartSpanAttribute/span0",
Name: "span0",
Attributes: []core.KeyValue{
key.String("key1", "value1"),
key.String("key2", "value2"),
Expand Down Expand Up @@ -340,7 +340,7 @@ func TestSetSpanAttributes(t *testing.T) {
TraceFlags: 0x1,
},
ParentSpanID: sid,
Name: "SpanAttribute/span0",
Name: "span0",
Attributes: []core.KeyValue{
key.String("key1", "value1"),
},
Expand Down Expand Up @@ -375,7 +375,7 @@ func TestSetSpanAttributesOverLimit(t *testing.T) {
TraceFlags: 0x1,
},
ParentSpanID: sid,
Name: "SpanAttributesOverLimit/span0",
Name: "span0",
Attributes: []core.KeyValue{
key.Bool("key1", false),
key.Int64("key4", 4),
Expand Down Expand Up @@ -420,7 +420,7 @@ func TestEvents(t *testing.T) {
TraceFlags: 0x1,
},
ParentSpanID: sid,
Name: "Events/span0",
Name: "span0",
HasRemoteParent: true,
MessageEvents: []export.Event{
{Name: "foo", Attributes: []core.KeyValue{k1v1}},
Expand Down Expand Up @@ -470,7 +470,7 @@ func TestEventsOverLimit(t *testing.T) {
TraceFlags: 0x1,
},
ParentSpanID: sid,
Name: "EventsOverLimit/span0",
Name: "span0",
MessageEvents: []export.Event{
{Name: "foo", Attributes: []core.KeyValue{k1v1}},
{Name: "bar", Attributes: []core.KeyValue{k2v2, k3v3}},
Expand Down Expand Up @@ -514,7 +514,7 @@ func TestLinks(t *testing.T) {
TraceFlags: 0x1,
},
ParentSpanID: sid,
Name: "Links/span0",
Name: "span0",
HasRemoteParent: true,
Links: []apitrace.Link{
{SpanContext: sc1, Attributes: []core.KeyValue{k1v1}},
Expand Down Expand Up @@ -557,7 +557,7 @@ func TestLinksOverLimit(t *testing.T) {
TraceFlags: 0x1,
},
ParentSpanID: sid,
Name: "LinksOverLimit/span0",
Name: "span0",
Links: []apitrace.Link{
{SpanContext: sc2, Attributes: []core.KeyValue{k2v2}},
{SpanContext: sc3, Attributes: []core.KeyValue{k3v3}},
Expand All @@ -575,7 +575,7 @@ func TestSetSpanName(t *testing.T) {
te := &testExporter{}
tp, _ := NewProvider(WithSyncer(te))

want := "SetSpanName/SpanName-1"
want := "SpanName-1"
_, span := tp.Tracer("SetSpanName").Start(context.Background(), "SpanName-1",
apitrace.ChildOf(core.SpanContext{
TraceID: tid,
Expand Down Expand Up @@ -610,7 +610,7 @@ func TestSetSpanStatus(t *testing.T) {
TraceFlags: 0x1,
},
ParentSpanID: sid,
Name: "SpanStatus/span0",
Name: "span0",
SpanKind: apitrace.SpanKindInternal,
Status: codes.Canceled,
HasRemoteParent: true,
Expand Down Expand Up @@ -752,16 +752,30 @@ func TestStartSpanAfterEnd(t *testing.T) {
if got, want := len(spans), 3; got != want {
t.Fatalf("len(%#v) = %d; want %d", spans, got, want)
}
if got, want := spans["SpanAfterEnd/span-1"].SpanContext.TraceID, spans["SpanAfterEnd/parent"].SpanContext.TraceID; got != want {

gotParent, ok := spans["parent"]
if !ok {
t.Fatal("parent not recorded")
}
gotSpan1, ok := spans["span-1"]
if !ok {
t.Fatal("span-1 not recorded")
}
gotSpan2, ok := spans["span-2"]
if !ok {
t.Fatal("span-2 not recorded")
}

if got, want := gotSpan1.SpanContext.TraceID, gotParent.SpanContext.TraceID; got != want {
t.Errorf("span-1.TraceID=%q; want %q", got, want)
}
if got, want := spans["SpanAfterEnd/span-2"].SpanContext.TraceID, spans["SpanAfterEnd/parent"].SpanContext.TraceID; got != want {
if got, want := gotSpan2.SpanContext.TraceID, gotParent.SpanContext.TraceID; got != want {
t.Errorf("span-2.TraceID=%q; want %q", got, want)
}
if got, want := spans["SpanAfterEnd/span-1"].ParentSpanID, spans["SpanAfterEnd/parent"].SpanContext.SpanID; got != want {
if got, want := gotSpan1.ParentSpanID, gotParent.SpanContext.SpanID; got != want {
t.Errorf("span-1.ParentSpanID=%q; want %q (parent.SpanID)", got, want)
}
if got, want := spans["SpanAfterEnd/span-2"].ParentSpanID, spans["SpanAfterEnd/span-1"].SpanContext.SpanID; got != want {
if got, want := gotSpan2.ParentSpanID, gotSpan1.SpanContext.SpanID; got != want {
t.Errorf("span-2.ParentSpanID=%q; want %q (span1.SpanID)", got, want)
}
}
Expand All @@ -783,16 +797,34 @@ func TestChildSpanCount(t *testing.T) {
if got, want := len(spans), 4; got != want {
t.Fatalf("len(%#v) = %d; want %d", spans, got, want)
}
if got, want := spans["ChidSpanCount/span-3"].ChildSpanCount, 0; got != want {

gotParent, ok := spans["parent"]
if !ok {
t.Fatal("parent not recorded")
}
gotSpan1, ok := spans["span-1"]
if !ok {
t.Fatal("span-1 not recorded")
}
gotSpan2, ok := spans["span-2"]
if !ok {
t.Fatal("span-2 not recorded")
}
gotSpan3, ok := spans["span-3"]
if !ok {
t.Fatal("span-3 not recorded")
}

if got, want := gotSpan3.ChildSpanCount, 0; got != want {
t.Errorf("span-3.ChildSpanCount=%q; want %q", got, want)
}
if got, want := spans["ChidSpanCount/span-2"].ChildSpanCount, 0; got != want {
if got, want := gotSpan2.ChildSpanCount, 0; got != want {
t.Errorf("span-2.ChildSpanCount=%q; want %q", got, want)
}
if got, want := spans["ChidSpanCount/span-1"].ChildSpanCount, 1; got != want {
if got, want := gotSpan1.ChildSpanCount, 1; got != want {
t.Errorf("span-1.ChildSpanCount=%q; want %q", got, want)
}
if got, want := spans["ChidSpanCount/parent"].ChildSpanCount, 2; got != want {
if got, want := gotParent.ChildSpanCount, 2; got != want {
t.Errorf("parent.ChildSpanCount=%q; want %q", got, want)
}
}
Expand Down
12 changes: 2 additions & 10 deletions sdk/trace/tracer.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,7 @@ func (tr *tracer) Start(ctx context.Context, name string, o ...apitrace.StartOpt
parent = p.spanContext
}

spanName := tr.spanNameWithPrefix(name)
span := startSpanInternal(tr, spanName, parent, remoteParent, opts)
span := startSpanInternal(tr, name, parent, remoteParent, opts)
for _, l := range opts.Links {
span.addLink(l)
}
Expand All @@ -68,7 +67,7 @@ func (tr *tracer) Start(ctx context.Context, name string, o ...apitrace.StartOpt
}
}

ctx, end := startExecutionTracerTask(ctx, spanName)
ctx, end := startExecutionTracerTask(ctx, name)
span.executionTracerTaskEnd = end
return apitrace.ContextWithSpan(ctx, span), span
}
Expand All @@ -83,10 +82,3 @@ func (tr *tracer) WithSpan(ctx context.Context, name string, body func(ctx conte
}
return nil
}

func (tr *tracer) spanNameWithPrefix(name string) string {
if tr.name != "" {
return tr.name + "/" + name
}
return name
}

0 comments on commit 2d5ba32

Please sign in to comment.