From f4cfe820a3bfbe633efd2b0964be0d975afeeebd Mon Sep 17 00:00:00 2001 From: Anton Manakin Date: Wed, 8 May 2024 06:57:08 +0300 Subject: [PATCH 1/3] record links with empty span context --- CHANGELOG.md | 1 + sdk/trace/span.go | 6 ++- sdk/trace/trace_test.go | 88 ++++++++++++++++++++++++++++++++++++++++- 3 files changed, 93 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0db3cf6ce6f..98915b58e26 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - De-duplicate map attributes added to a `Record` in `go.opentelemetry.io/otel/sdk/log`. (#5230) - The `go.opentelemetry.io/otel/exporters/stdout/stdoutlog` exporter won't print `AttributeValueLengthLimit` and `AttributeCountLimit` fields now, instead it prints the `DroppedAttributes` field. (#5272) - Improved performance in the `Stringer` implementation of `go.opentelemetry.io/otel/baggage.Member` by reducing the number of allocations. (#5286) +- Span in `go.opentelemetry.io/otel/sdk/trace` will record links without span context if either non-empty `TraceState` or attributes are provided. (#5315) ## [1.26.0/0.48.0/0.2.0-alpha] 2024-04-24 diff --git a/sdk/trace/span.go b/sdk/trace/span.go index c44f6b926aa..7acfd3fe9f4 100644 --- a/sdk/trace/span.go +++ b/sdk/trace/span.go @@ -630,7 +630,11 @@ func (s *recordingSpan) Resource() *resource.Resource { } func (s *recordingSpan) AddLink(link trace.Link) { - if !s.IsRecording() || !link.SpanContext.IsValid() { + if !s.IsRecording() { + return + } + if !link.SpanContext.IsValid() && len(link.Attributes) == 0 && + link.SpanContext.TraceState().Len() == 0 { return } diff --git a/sdk/trace/trace_test.go b/sdk/trace/trace_test.go index 2ec6cbb57a6..471c1731fad 100644 --- a/sdk/trace/trace_test.go +++ b/sdk/trace/trace_test.go @@ -1923,6 +1923,9 @@ func TestEmptyRecordingSpanDroppedAttributes(t *testing.T) { } func TestSpanAddLink(t *testing.T) { + ts, err := trace.ParseTraceState("k=v") + require.NoError(t, err) + tests := []struct { name string attrLinkCountLimit int @@ -1934,7 +1937,6 @@ func TestSpanAddLink(t *testing.T) { attrLinkCountLimit: 128, link: trace.Link{ SpanContext: trace.NewSpanContext(trace.SpanContextConfig{TraceID: trace.TraceID([16]byte{}), SpanID: [8]byte{}}), - Attributes: []attribute.KeyValue{{Key: "k1", Value: attribute.StringValue("v1")}}, }, want: &snapshot{ name: "span0", @@ -2002,6 +2004,50 @@ func TestSpanAddLink(t *testing.T) { instrumentationScope: instrumentation.Scope{Name: "AddLinkWithMoreAttributesThanLimit"}, }, }, + { + name: "AddLinkWithAttributesEmptySpanContext", + attrLinkCountLimit: 128, + link: trace.Link{ + Attributes: []attribute.KeyValue{{Key: "k1", Value: attribute.StringValue("v1")}}, + }, + want: &snapshot{ + name: "span0", + spanContext: trace.NewSpanContext(trace.SpanContextConfig{ + TraceID: tid, + TraceFlags: 0x1, + }), + parent: sc.WithRemote(true), + links: []Link{ + { + Attributes: []attribute.KeyValue{{Key: "k1", Value: attribute.StringValue("v1")}}, + }, + }, + spanKind: trace.SpanKindInternal, + instrumentationScope: instrumentation.Scope{Name: "AddLinkWithAttributesEmptySpanContext"}, + }, + }, + { + name: "AddLinkWithTraceStateEmptySpanContext", + attrLinkCountLimit: 128, + link: trace.Link{ + SpanContext: trace.SpanContext{}.WithTraceState(ts), + }, + want: &snapshot{ + name: "span0", + spanContext: trace.NewSpanContext(trace.SpanContextConfig{ + TraceID: tid, + TraceFlags: 0x1, + }), + parent: sc.WithRemote(true), + links: []Link{ + { + SpanContext: trace.SpanContext{}.WithTraceState(ts), + }, + }, + spanKind: trace.SpanKindInternal, + instrumentationScope: instrumentation.Scope{Name: "AddLinkWithTraceStateEmptySpanContext"}, + }, + }, } for _, tc := range tests { @@ -2026,3 +2072,43 @@ func TestSpanAddLink(t *testing.T) { }) } } + +func TestAddLinkToNonRecordingSpan(t *testing.T) { + te := NewTestExporter() + sl := NewSpanLimits() + tp := NewTracerProvider( + WithSpanLimits(sl), + WithSyncer(te), + WithResource(resource.Empty()), + ) + + attrs := []attribute.KeyValue{{Key: "k", Value: attribute.StringValue("v")}} + + span := startSpan(tp, "AddLinkToNonRecordingSpan") + _, err := endSpan(te, span) + require.NoError(t, err) + + // Add link after ending span + span.AddLink(trace.Link{ + SpanContext: sc, + Attributes: attrs, + }) + + require.Equal(t, 1, te.Len()) + got := te.Spans()[0] + want := &snapshot{ + name: "span0", + spanContext: trace.NewSpanContext(trace.SpanContextConfig{ + TraceID: tid, + TraceFlags: 0x1, + }), + parent: sc.WithRemote(true), + links: nil, + spanKind: trace.SpanKindInternal, + instrumentationScope: instrumentation.Scope{Name: "AddLinkToNonRecordingSpan"}, + } + + if diff := cmpDiff(got, want); diff != "" { + t.Errorf("AddLinkToNonRecordingSpan: -got +want %s", diff) + } +} From 6ac89762adeb447e238f8b20a580217e4016d064 Mon Sep 17 00:00:00 2001 From: Anton Manakin Date: Thu, 9 May 2024 13:33:06 +0300 Subject: [PATCH 2/3] add global trace state --- sdk/trace/trace_test.go | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/sdk/trace/trace_test.go b/sdk/trace/trace_test.go index 471c1731fad..8ae44d17b82 100644 --- a/sdk/trace/trace_test.go +++ b/sdk/trace/trace_test.go @@ -47,6 +47,7 @@ var ( tid trace.TraceID sid trace.SpanID sc trace.SpanContext + ts trace.TraceState handler = &storingHandler{} ) @@ -59,6 +60,7 @@ func init() { SpanID: sid, TraceFlags: 0x1, }) + ts, _ = trace.ParseTraceState("k=v") otel.SetErrorHandler(handler) } @@ -330,10 +332,6 @@ func TestStartSpanWithParent(t *testing.T) { t.Error(err) } - ts, err := trace.ParseTraceState("k=v") - if err != nil { - t.Error(err) - } sc2 := sc.WithTraceState(ts) _, s3 := tr.Start(trace.ContextWithRemoteSpanContext(ctx, sc2), "span3-sampled-parent2") if err := checkChild(t, sc2, s3); err != nil { @@ -1923,9 +1921,6 @@ func TestEmptyRecordingSpanDroppedAttributes(t *testing.T) { } func TestSpanAddLink(t *testing.T) { - ts, err := trace.ParseTraceState("k=v") - require.NoError(t, err) - tests := []struct { name string attrLinkCountLimit int From b7782d4d04bd0278154484da21fdaaf1b3e70b83 Mon Sep 17 00:00:00 2001 From: Anton Manakin Date: Thu, 9 May 2024 21:44:42 +0300 Subject: [PATCH 3/3] fix test comments and changelog --- CHANGELOG.md | 2 +- sdk/trace/trace_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 98915b58e26..eb8eac9e238 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,7 +24,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - De-duplicate map attributes added to a `Record` in `go.opentelemetry.io/otel/sdk/log`. (#5230) - The `go.opentelemetry.io/otel/exporters/stdout/stdoutlog` exporter won't print `AttributeValueLengthLimit` and `AttributeCountLimit` fields now, instead it prints the `DroppedAttributes` field. (#5272) - Improved performance in the `Stringer` implementation of `go.opentelemetry.io/otel/baggage.Member` by reducing the number of allocations. (#5286) -- Span in `go.opentelemetry.io/otel/sdk/trace` will record links without span context if either non-empty `TraceState` or attributes are provided. (#5315) +- The `Span` in `go.opentelemetry.io/otel/sdk/trace` will record links without span context if either non-empty `TraceState` or attributes are provided. (#5315) ## [1.26.0/0.48.0/0.2.0-alpha] 2024-04-24 diff --git a/sdk/trace/trace_test.go b/sdk/trace/trace_test.go index 8ae44d17b82..e7ef786a1b3 100644 --- a/sdk/trace/trace_test.go +++ b/sdk/trace/trace_test.go @@ -2083,7 +2083,7 @@ func TestAddLinkToNonRecordingSpan(t *testing.T) { _, err := endSpan(te, span) require.NoError(t, err) - // Add link after ending span + // Add link to ended, non-recording, span. The link should be dropped. span.AddLink(trace.Link{ SpanContext: sc, Attributes: attrs,