[release/9.0] Reject duplicate span ids (#6471)

* [release/9.0] Reject duplicate span ids * WIP * Update
dotnet · Oct 24, 2024 · 23bfd12 · 23bfd12
1 parent b1e81d7
commit 23bfd12
Show file tree

Hide file tree

Showing 4 changed files with 88 additions and 0 deletions.
diff --git a/playground/Stress/Stress.ApiService/Program.cs b/playground/Stress/Stress.ApiService/Program.cs
@@ -210,4 +210,19 @@ async IAsyncEnumerable<string> WriteOutput()
     return "Log with formatted data";
 });
 
+app.MapGet("/duplicate-spanid", async () =>
+{
+    var traceCreator = new TraceCreator();
+
+    var span1 = traceCreator.CreateActivity("Test 1", "0485b1947fe788bb");
+    await Task.Delay(1000);
+    span1?.Stop();
+
+    var span2 = traceCreator.CreateActivity("Test 2", "0485b1947fe788bb");
+    await Task.Delay(1000);
+    span2?.Stop();
+
+    return $"Created duplicate span IDs.";
+});
+
 app.Run();
diff --git a/playground/Stress/Stress.ApiService/TraceCreator.cs b/playground/Stress/Stress.ApiService/TraceCreator.cs
@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System.Diagnostics;
+using System.Reflection;
 
 namespace Stress.ApiService;
 
@@ -13,6 +14,22 @@ public class TraceCreator
 
     private readonly List<Activity> _allActivities = new List<Activity>();
 
+    public Activity? CreateActivity(string name, string? spandId)
+    {
+        var activity = s_activitySource.StartActivity(name, ActivityKind.Client);
+        if (activity != null)
+        {
+            if (spandId != null)
+            {
+                // Gross but it's the only way.
+                typeof(Activity).GetField("_spanId", BindingFlags.Instance | BindingFlags.NonPublic)!.SetValue(activity, spandId);
+                typeof(Activity).GetField("_traceId", BindingFlags.Instance | BindingFlags.NonPublic)!.SetValue(activity, activity.TraceId.ToString());
+            }
+        }
+
+        return activity;
+    }
+
     public async Task CreateTraceAsync(int count, bool createChildren)
     {
         var activityStack = new Stack<Activity>();

diff --git a/src/Aspire.Dashboard/Otlp/Model/OtlpTrace.cs b/src/Aspire.Dashboard/Otlp/Model/OtlpTrace.cs
@@ -52,6 +52,11 @@ public int CalculateDepth(OtlpSpan span)
 
     public void AddSpan(OtlpSpan span)
     {
+        if (Spans.Any(s => s.SpanId == span.SpanId))
+        {
+            throw new InvalidOperationException($"Duplicate span id '{span.SpanId}' detected.");
+        }
+
         var added = false;
         for (var i = Spans.Count - 1; i >= 0; i--)
         {

diff --git a/tests/Aspire.Dashboard.Tests/TelemetryRepositoryTests/TraceTests.cs b/tests/Aspire.Dashboard.Tests/TelemetryRepositoryTests/TraceTests.cs
@@ -90,6 +90,57 @@ public void AddTraces()
             });
     }
 
+    [Fact]
+    public void AddTraces_DuplicateTraceIds_Reject()
+    {
+        // Arrange
+        var repository = CreateRepository();
+        // Act
+        var addContext = new AddContext();
+        repository.AddTraces(addContext, new RepeatedField<ResourceSpans>()
+        {
+            new ResourceSpans
+            {
+                Resource = CreateResource(),
+                ScopeSpans =
+                {
+                    new ScopeSpans
+                    {
+                        Scope = CreateScope(),
+                        Spans =
+                        {
+                            CreateSpan(traceId: "1", spanId: "1-1", startTime: s_testTime.AddMinutes(1), endTime: s_testTime.AddMinutes(10)),
+                            CreateSpan(traceId: "1", spanId: "1-1", startTime: s_testTime.AddMinutes(5), endTime: s_testTime.AddMinutes(10)),
+                            CreateSpan(traceId: "1", spanId: "1-2", startTime: s_testTime.AddMinutes(1), endTime: s_testTime.AddMinutes(10), parentSpanId: "1-1"),
+                        }
+                    }
+                }
+            }
+        });
+        // Assert
+        Assert.Equal(1, addContext.FailureCount);
+        var applications = repository.GetApplications();
+        Assert.Collection(applications,
+            app =>
+            {
+                Assert.Equal("TestService", app.ApplicationName);
+                Assert.Equal("TestId", app.InstanceId);
+            });
+        var traces = repository.GetTraces(new GetTracesRequest
+        {
+            ApplicationKey = applications[0].ApplicationKey,
+            FilterText = string.Empty,
+            StartIndex = 0,
+            Count = 10,
+            Filters = []
+        });
+        Assert.Collection(traces.PagedResult.Items,
+            trace =>
+            {
+                Assert.Equal(2, trace.Spans.Count);
+            });
+    }
+
     [Fact]
     public void AddTraces_Scope_Multiple()
     {