diff --git a/tracer/src/Datadog.Trace/Activity/OtlpHelpers.cs b/tracer/src/Datadog.Trace/Activity/OtlpHelpers.cs index 443135368ded..27958ce5f711 100644 --- a/tracer/src/Datadog.Trace/Activity/OtlpHelpers.cs +++ b/tracer/src/Datadog.Trace/Activity/OtlpHelpers.cs @@ -254,8 +254,8 @@ private static void ExtractActivityLinks(Span span, IActivity5? activity spanContext.LastParentId = traceState.LastParent; spanContext.PropagatedTags = traceTags; - var extractedSpan = new Span(spanContext, DateTimeOffset.Now, new CommonTags()); - var spanLink = span.AddSpanLink(extractedSpan); + var spanLink = new SpanLink(spanContext); + span.AddLink(spanLink); if (duckLink.Tags is not null) { diff --git a/tracer/src/Datadog.Trace/Span.cs b/tracer/src/Datadog.Trace/Span.cs index fc4c5d2d8a8c..89cec0e664c7 100644 --- a/tracer/src/Datadog.Trace/Span.cs +++ b/tracer/src/Datadog.Trace/Span.cs @@ -538,21 +538,19 @@ private void WriteCloseDebugMessage() /// /// Adds a SpanLink to the current Span if the Span is active. /// - /// The Span to add as a SpanLink - /// List of KeyValue pairings of attributes to add to the SpanLink. Defaults to null - /// returns the SpanLink on success or null on failure (span is closed already) - internal SpanLink AddSpanLink(Span spanLinkToAdd, List> attributes = null) + /// The SpanLink to add + /// This span to allow method chaining. + internal Span AddLink(SpanLink spanLink) { if (IsFinished) { - Log.Warning("AddSpanLink should not be called after the span was closed"); - return null; + Log.Warning("AddLink should not be called after the span was closed"); + return this; } SpanLinks ??= new List(); - var spanLink = new SpanLink(spanLinkToAdd, this, attributes); SpanLinks.Add(spanLink); - return spanLink; + return this; } } } diff --git a/tracer/src/Datadog.Trace/SpanLink.cs b/tracer/src/Datadog.Trace/SpanLink.cs index 627536e90693..fc6bc00b6e9f 100644 --- a/tracer/src/Datadog.Trace/SpanLink.cs +++ b/tracer/src/Datadog.Trace/SpanLink.cs @@ -25,17 +25,15 @@ internal class SpanLink /// in OpenTelemetry that's called a Span Context, which may also include tracestate and trace flags. /// /// The context of the spanlink to extract attributes from - /// Reference to the span you're adding SpanLinks to /// Optional dictionary of attributes to take for the spanlink. - public SpanLink(SpanContext spanLinkContext, Span decoratedSpan, List>? attributes = null) + public SpanLink(SpanContext spanLinkContext, List>? attributes = null) { Context = spanLinkContext; - DecoratedSpan = decoratedSpan; Attributes = attributes; } - public SpanLink(Span spanToLink, Span decoratedSpan, List>? attributes = null) - : this(spanToLink.Context, decoratedSpan, attributes) + public SpanLink(Span spanToLink, List>? attributes = null) + : this(spanToLink.Context, attributes) { } @@ -43,8 +41,6 @@ public SpanLink(Span spanToLink, Span decoratedSpan, List /// Adds an Attribute to the SpanLink. /// @@ -52,12 +48,6 @@ public SpanLink(Span spanToLink, Span decoratedSpan, Listvalue of attribute public void AddAttribute(string name, string value) { - if (DecoratedSpan.IsFinished) - { - Log.Warning("AddAttribute should not be called after the decorated span was closed"); - return; - } - var newAttribute = new KeyValuePair(name, value); Attributes ??= []; diff --git a/tracer/test/Datadog.Trace.Tests/Agent/MessagePack/SpanMessagePackFormatterTests.cs b/tracer/test/Datadog.Trace.Tests/Agent/MessagePack/SpanMessagePackFormatterTests.cs index 76928a95c333..2ed0d18d7335 100644 --- a/tracer/test/Datadog.Trace.Tests/Agent/MessagePack/SpanMessagePackFormatterTests.cs +++ b/tracer/test/Datadog.Trace.Tests/Agent/MessagePack/SpanMessagePackFormatterTests.cs @@ -141,11 +141,14 @@ public void SpanLink_Tag_Serialization() new("pair", "false"), new("arbitrary", "56709") }; - spans[0].AddSpanLink(spans[1], attributesToAdd); - var tmpSpanLink = spans[1].AddSpanLink(spans[2]); + spans[0].AddLink(new SpanLink(spans[1].Context, attributesToAdd)); + + var tmpSpanLink = new SpanLink(spans[2].Context); + spans[1].AddLink(tmpSpanLink); tmpSpanLink.AddAttribute("attribute1", "value1"); tmpSpanLink.AddAttribute("attribute2", "value2"); - spans[1].AddSpanLink(spans[0]); + + spans[1].AddLink(new SpanLink(spans[0].Context)); foreach (var span in spans) { diff --git a/tracer/test/Datadog.Trace.Tests/SpanLinksTests.cs b/tracer/test/Datadog.Trace.Tests/SpanLinksTests.cs index ce2904f34d41..55aeb08bbe0f 100644 --- a/tracer/test/Datadog.Trace.Tests/SpanLinksTests.cs +++ b/tracer/test/Datadog.Trace.Tests/SpanLinksTests.cs @@ -3,10 +3,12 @@ // This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc. // +using System.Collections.Generic; using Castle.Core.Internal; using Datadog.Trace.Agent; using Datadog.Trace.Configuration; using Datadog.Trace.Sampling; +using FluentAssertions; using Moq; using Xunit; @@ -26,30 +28,36 @@ public SpanLinksTests() } [Fact] - public void AddLink_InCloseSpan() + public void AddLink_AfterSpanFinished_IsNoOp() { var parentScope = (Scope)_tracer.StartActive("Parent"); var childScope = (Scope)_tracer.StartActive("Child"); var parentSpan = parentScope.Span; var childSpan = childScope.Span; + var spanLink = new SpanLink(parentSpan.Context); childSpan.Finish(); - Assert.Null(childSpan.AddSpanLink(parentSpan)); + + childSpan.AddLink(spanLink); + childSpan.SpanLinks.Should().BeNullOrEmpty(); } [Fact] - public void AddAttribute_ToLink_InCloseSpan() + public void AddAttribute_BeforeSpanFinished_IsSuccessful() { var parentScope = (Scope)_tracer.StartActive("Parent"); var childScope = (Scope)_tracer.StartActive("Child"); var parentSpan = parentScope.Span; var childSpan = childScope.Span; - var spanLink = childSpan.AddSpanLink(parentSpan); + var spanLink = new SpanLink(parentSpan.Context); + + childSpan.AddLink(spanLink); + spanLink.AddAttribute("key", "value"); + + spanLink.Attributes.Should().BeEquivalentTo(new[] { new KeyValuePair("key", "value") }); childSpan.Finish(); - spanLink.AddAttribute("should", "return null"); - Assert.Null(spanLink.Attributes); } } }