Span interface should only allow LinkedTo at creation. (#349)

* Remove AddLink & Link from Span Interface I have remove AddLink and Link from the interface and all it refereneces and replaced AddLink with addlink, Also Removed respective unit tests Signed-off-by: vineeth <[email protected]> * removing the unused code from unit tests Signed-off-by: VineethReddy02 <[email protected]>
open-telemetry · Nov 26, 2019 · a99f872 · a99f872
1 parent 3d78564
commit a99f872
Show file tree

Hide file tree

Showing 9 changed files with 16 additions and 121 deletions.
diff --git a/api/testharness/harness.go b/api/testharness/harness.go
@@ -304,12 +304,6 @@ func (h *Harness) testSpan(tracerFactory func() trace.Tracer) {
 		"#AddEventWithTimestamp": func(span trace.Span) {
 			span.AddEventWithTimestamp(context.Background(), time.Now(), "test event")
 		},
-		"#AddLink": func(span trace.Span) {
-			span.AddLink(trace.Link{})
-		},
-		"#Link": func(span trace.Span) {
-			span.Link(core.SpanContext{})
-		},
 		"#SetStatus": func(span trace.Span) {
 			span.SetStatus(codes.Internal)
 		},

diff --git a/api/trace/api.go b/api/trace/api.go
@@ -72,13 +72,6 @@ type Span interface {
 	// IsRecording returns true if the span is active and recording events is enabled.
 	IsRecording() bool
 
-	// AddLink adds a link to the span.
-	AddLink(link Link)
-
-	// Link creates a link between this span and the other span specified by the SpanContext.
-	// It then adds the newly created Link to the span.
-	Link(sc core.SpanContext, attrs ...core.KeyValue)
-
 	// SpanContext returns span context of the span. Returned SpanContext is usable
 	// even after the span ends.
 	SpanContext() core.SpanContext

diff --git a/api/trace/current_test.go b/api/trace/current_test.go
@@ -103,11 +103,3 @@ func (mockSpan) AddEvent(ctx context.Context, msg string, attrs ...core.KeyValue
 // AddEventWithTimestamp does nothing.
 func (mockSpan) AddEventWithTimestamp(ctx context.Context, timestamp time.Time, msg string, attrs ...core.KeyValue) {
 }
-
-// AddLink does nothing.
-func (mockSpan) AddLink(link trace.Link) {
-}
-
-// Link does nothing.
-func (mockSpan) Link(sc core.SpanContext, attrs ...core.KeyValue) {
-}
diff --git a/api/trace/noop_span.go b/api/trace/noop_span.go
@@ -73,12 +73,4 @@ func (NoopSpan) AddEventWithTimestamp(ctx context.Context, timestamp time.Time,
 
 // SetName does nothing.
 func (NoopSpan) SetName(name string) {
-}
-
-// AddLink does nothing.
-func (NoopSpan) AddLink(link Link) {
-}
-
-// Link does nothing.
-func (NoopSpan) Link(sc core.SpanContext, attrs ...core.KeyValue) {
-}
+}
diff --git a/bridge/opentracing/internal/mock.go b/bridge/opentracing/internal/mock.go
@@ -289,14 +289,6 @@ func (s *MockSpan) AddEventWithTimestamp(ctx context.Context, timestamp time.Tim
 	})
 }
 
-func (s *MockSpan) AddLink(link oteltrace.Link) {
-	// TODO
-}
-
-func (s *MockSpan) Link(sc otelcore.SpanContext, attrs ...otelcore.KeyValue) {
-	// TODO
-}
-
 func (s *MockSpan) OverrideTracer(tracer oteltrace.Tracer) {
 	s.officialTracer = tracer
 }
diff --git a/internal/trace/mock_span.go b/internal/trace/mock_span.go
@@ -82,11 +82,3 @@ func (ms *MockSpan) AddEvent(ctx context.Context, msg string, attrs ...core.KeyV
 // AddEvent does nothing.
 func (ms *MockSpan) AddEventWithTimestamp(ctx context.Context, timestamp time.Time, msg string, attrs ...core.KeyValue) {
 }
-
-// AddLink does nothing.
-func (ms *MockSpan) AddLink(link apitrace.Link) {
-}
-
-// Link does nothing.
-func (ms *MockSpan) Link(sc core.SpanContext, attrs ...core.KeyValue) {
-}
diff --git a/sdk/trace/span.go b/sdk/trace/span.go
@@ -189,33 +189,10 @@ func (s *span) SetName(name string) {
 	makeSamplingDecision(data)
 }
 
-// AddLink implements Span interface. Specified link is added to the span.
-// If the total number of links associated with the span exceeds the limit
-// then the oldest link is removed to create space for the link being added.
-func (s *span) AddLink(link apitrace.Link) {
-	if !s.IsRecording() {
-		return
-	}
-	s.addLink(link)
-}
-
-// Link implements Span interface. It is similar to AddLink but it excepts
-// SpanContext and attributes as arguments instead of Link. It first creates
-// a Link object and then adds to the span.
-func (s *span) Link(sc core.SpanContext, attrs ...core.KeyValue) {
+func (s *span) addLink(link apitrace.Link) {
 	if !s.IsRecording() {
 		return
 	}
-	attrsCopy := attrs
-	if attrs != nil {
-		attrsCopy = make([]core.KeyValue, len(attrs))
-		copy(attrsCopy, attrs)
-	}
-	link := apitrace.Link{SpanContext: sc, Attributes: attrsCopy}
-	s.addLink(link)
-}
-
-func (s *span) addLink(link apitrace.Link) {
 	s.mu.Lock()
 	defer s.mu.Unlock()
 	s.links.add(link)

diff --git a/sdk/trace/trace_test.go b/sdk/trace/trace_test.go
@@ -482,63 +482,25 @@ func TestEventsOverLimit(t *testing.T) {
 	}
 }
 
-func TestAddLinks(t *testing.T) {
-	te := &testExporter{}
-	tp, _ := NewProvider(WithSyncer(te))
-
-	span := startSpan(tp, "AddLinks")
-	k1v1 := key.New("key1").String("value1")
-	k2v2 := key.New("key2").String("value2")
-
-	sc1 := core.SpanContext{TraceID: core.TraceID([16]byte{1, 1}), SpanID: core.SpanID{3}}
-	sc2 := core.SpanContext{TraceID: core.TraceID([16]byte{1, 1}), SpanID: core.SpanID{3}}
-
-	link1 := apitrace.Link{SpanContext: sc1, Attributes: []core.KeyValue{k1v1}}
-	link2 := apitrace.Link{SpanContext: sc2, Attributes: []core.KeyValue{k2v2}}
-	span.AddLink(link1)
-	span.AddLink(link2)
-
-	got, err := endSpan(te, span)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	want := &export.SpanData{
-		SpanContext: core.SpanContext{
-			TraceID:    tid,
-			TraceFlags: 0x1,
-		},
-		ParentSpanID:    sid,
-		Name:            "AddLinks/span0",
-		HasRemoteParent: true,
-		Links: []apitrace.Link{
-			{SpanContext: sc1, Attributes: []core.KeyValue{k1v1}},
-			{SpanContext: sc2, Attributes: []core.KeyValue{k2v2}},
-		},
-		SpanKind: apitrace.SpanKindInternal,
-	}
-	if diff := cmpDiff(got, want); diff != "" {
-		t.Errorf("AddLink: -got +want %s", diff)
-	}
-}
-
 func TestLinks(t *testing.T) {
 	te := &testExporter{}
 	tp, _ := NewProvider(WithSyncer(te))
 
-	span := startSpan(tp, "Links")
 	k1v1 := key.New("key1").String("value1")
 	k2v2 := key.New("key2").String("value2")
 	k3v3 := key.New("key3").String("value3")
 
 	sc1 := core.SpanContext{TraceID: core.TraceID([16]byte{1, 1}), SpanID: core.SpanID{3}}
 	sc2 := core.SpanContext{TraceID: core.TraceID([16]byte{1, 1}), SpanID: core.SpanID{3}}
 
-	span.Link(sc1, key.New("key1").String("value1"))
-	span.Link(sc2,
-		key.New("key2").String("value2"),
-		key.New("key3").String("value3"),
+	span := startSpan(tp, "Links",
+		apitrace.LinkedTo(sc1, key.New("key1").String("value1")),
+		apitrace.LinkedTo(sc2,
+			key.New("key2").String("value2"),
+			key.New("key3").String("value3"),
+		),
 	)
+
 	got, err := endSpan(te, span)
 	if err != nil {
 		t.Fatal(err)
@@ -572,15 +534,16 @@ func TestLinksOverLimit(t *testing.T) {
 	sc3 := core.SpanContext{TraceID: core.TraceID([16]byte{1, 1}), SpanID: core.SpanID{3}}
 
 	tp, _ := NewProvider(WithConfig(cfg), WithSyncer(te))
-	span := startSpan(tp, "LinksOverLimit")
+
+	span := startSpan(tp, "LinksOverLimit",
+		apitrace.LinkedTo(sc1, key.New("key1").String("value1")),
+		apitrace.LinkedTo(sc2, key.New("key2").String("value2")),
+		apitrace.LinkedTo(sc3, key.New("key3").String("value3")),
+	)
 
 	k2v2 := key.New("key2").String("value2")
 	k3v3 := key.New("key3").String("value3")
 
-	span.Link(sc1, key.New("key1").String("value1"))
-	span.Link(sc2, key.New("key2").String("value2"))
-	span.Link(sc3, key.New("key3").String("value3"))
-
 	got, err := endSpan(te, span)
 	if err != nil {
 		t.Fatal(err)

diff --git a/sdk/trace/tracer.go b/sdk/trace/tracer.go
@@ -59,7 +59,7 @@ func (tr *tracer) Start(ctx context.Context, name string, o ...apitrace.SpanOpti
 	spanName := tr.spanNameWithPrefix(name)
 	span := startSpanInternal(tr, spanName, parent, remoteParent, opts)
 	for _, l := range opts.Links {
-		span.AddLink(l)
+		span.addLink(l)
 	}
 	span.SetAttributes(opts.Attributes...)