From 38babff9859b35e5434a234e51d6e18ce6ee0a75 Mon Sep 17 00:00:00 2001 From: Srikanth Chekuri Date: Sun, 3 Oct 2021 17:39:31 +0530 Subject: [PATCH 1/3] Skip links with invalid span context --- sdk/trace/trace_test.go | 9 +++++++++ sdk/trace/tracer.go | 4 +++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/sdk/trace/trace_test.go b/sdk/trace/trace_test.go index c4ab8443c2c..36ed7165cc2 100644 --- a/sdk/trace/trace_test.go +++ b/sdk/trace/trace_test.go @@ -670,6 +670,15 @@ func TestLinks(t *testing.T) { if diff := cmpDiff(got, want); diff != "" { t.Errorf("Link: -got +want %s", diff) } + sc1 = trace.NewSpanContext(trace.SpanContextConfig{TraceID: trace.TraceID([16]byte{1, 1}), SpanID: trace.SpanID{3}}) + + span1 := startSpan(tp, "name", trace.WithLinks([]trace.Link{ + {SpanContext: trace.SpanContext{}}, + {SpanContext: sc1}, + }...)) + + sdkspan, _ := span1.(*recordingSpan) + require.Len(t, sdkspan.Links(), 1) } func TestLinksOverLimit(t *testing.T) { diff --git a/sdk/trace/tracer.go b/sdk/trace/tracer.go index afd4b5b5d5b..6c545b960ce 100644 --- a/sdk/trace/tracer.go +++ b/sdk/trace/tracer.go @@ -138,7 +138,9 @@ func (tr *tracer) newRecordingSpan(psc, sc trace.SpanContext, name string, sr Sa } for _, l := range config.Links() { - s.addLink(l) + if l.SpanContext.IsValid() { + s.addLink(l) + } } s.SetAttributes(sr.Attributes...) From ceb720724f2462594c955200ec9aa654a854b957 Mon Sep 17 00:00:00 2001 From: Srikanth Chekuri Date: Sun, 3 Oct 2021 17:47:57 +0530 Subject: [PATCH 2/3] Add CHANGELOG entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index fb122786042..e1257405d04 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - The Metric SDK `Export()` function takes a new two-level reader interface for iterating over results one instrumentation library at a time. (#2197) - The former `"go.opentelemetry.io/otel/sdk/export/metric".CheckpointSet` is renamed `Reader`. - The new interface is named `"go.opentelemetry.io/otel/sdk/export/metric".InstrumentationLibraryReader`. +- Skip links with invalid span context. (#2275) ## [1.0.0] - 2021-09-20 From d0fed6c2ea601adbd2ba0d7f1891f64480c1f1fc Mon Sep 17 00:00:00 2001 From: Srikanth Chekuri Date: Tue, 12 Oct 2021 03:47:43 +0530 Subject: [PATCH 3/3] Review suggestions --- CHANGELOG.md | 5 ++++- sdk/trace/span.go | 2 +- sdk/trace/tracer.go | 4 +--- trace/config.go | 2 +- 4 files changed, 7 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e1257405d04..9a510106ae4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ## [Unreleased] +## Changed + +- Skip links with invalid span context. (#2275) + ## [1.0.1] - 2021-10-01 ### Fixed @@ -22,7 +26,6 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - The Metric SDK `Export()` function takes a new two-level reader interface for iterating over results one instrumentation library at a time. (#2197) - The former `"go.opentelemetry.io/otel/sdk/export/metric".CheckpointSet` is renamed `Reader`. - The new interface is named `"go.opentelemetry.io/otel/sdk/export/metric".InstrumentationLibraryReader`. -- Skip links with invalid span context. (#2275) ## [1.0.0] - 2021-09-20 diff --git a/sdk/trace/span.go b/sdk/trace/span.go index cda1644409b..e4ed7fbc0d9 100644 --- a/sdk/trace/span.go +++ b/sdk/trace/span.go @@ -453,7 +453,7 @@ func (s *recordingSpan) Resource() *resource.Resource { } func (s *recordingSpan) addLink(link trace.Link) { - if !s.IsRecording() { + if !s.IsRecording() || !link.SpanContext.IsValid() { return } s.mu.Lock() diff --git a/sdk/trace/tracer.go b/sdk/trace/tracer.go index 6c545b960ce..afd4b5b5d5b 100644 --- a/sdk/trace/tracer.go +++ b/sdk/trace/tracer.go @@ -138,9 +138,7 @@ func (tr *tracer) newRecordingSpan(psc, sc trace.SpanContext, name string, sr Sa } for _, l := range config.Links() { - if l.SpanContext.IsValid() { - s.addLink(l) - } + s.addLink(l) } s.SetAttributes(sr.Attributes...) diff --git a/trace/config.go b/trace/config.go index b1b74cb722d..8461a15ccd4 100644 --- a/trace/config.go +++ b/trace/config.go @@ -259,7 +259,7 @@ func WithStackTrace(b bool) SpanEndEventOption { } // WithLinks adds links to a Span. The links are added to the existing Span -// links, i.e. this does not overwrite. +// links, i.e. this does not overwrite. Links with invalid span context are ignored. func WithLinks(links ...Link) SpanStartOption { return spanOptionFunc(func(cfg *SpanConfig) { cfg.links = append(cfg.links, links...)