From 2a77217a1b4898309b11ae4f5a6dd43be0a1ffe0 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 1 Aug 2025 19:25:50 +0000 Subject: [PATCH 01/31] Initial plan From 8609dea6c989c2896261b8fb468cc6db82387bea Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 1 Aug 2025 19:40:25 +0000 Subject: [PATCH 02/31] Fix InvalidOperationException in CosmosDiagnostics.ToString() due to concurrent access Co-authored-by: kirankumarkolli <6880899+kirankumarkolli@users.noreply.github.com> --- Microsoft.Azure.Cosmos/src/Tracing/Trace.cs | 36 ++++++++- .../Tracing/TraceData/SummaryDiagnostics.cs | 21 +++++- .../Tracing/TraceWriter.TraceJsonWriter.cs | 30 +++++++- .../CosmosDiagnosticsUnitTests.cs | 73 +++++++++++++++++++ 4 files changed, 150 insertions(+), 10 deletions(-) diff --git a/Microsoft.Azure.Cosmos/src/Tracing/Trace.cs b/Microsoft.Azure.Cosmos/src/Tracing/Trace.cs index cb644f1ff0..3ae9d18556 100644 --- a/Microsoft.Azure.Cosmos/src/Tracing/Trace.cs +++ b/Microsoft.Azure.Cosmos/src/Tracing/Trace.cs @@ -124,18 +124,48 @@ public static Trace GetRootTrace( public void AddDatum(string key, TraceDatum traceDatum) { - this.data.Value.Add(key, traceDatum); + lock (this.children) + { + this.data.Value.Add(key, traceDatum); + } this.Summary.UpdateRegionContacted(traceDatum); } public void AddDatum(string key, object value) { - this.data.Value.Add(key, value); + lock (this.children) + { + this.data.Value.Add(key, value); + } } public void AddOrUpdateDatum(string key, object value) { - this.data.Value[key] = value; + lock (this.children) + { + this.data.Value[key] = value; + } + } + + internal ITrace[] GetChildrenSnapshot() + { + lock (this.children) + { + return this.children.ToArray(); + } + } + + internal KeyValuePair[] GetDataSnapshot() + { + if (!this.data.IsValueCreated) + { + return new KeyValuePair[0]; + } + + lock (this.children) + { + return this.data.Value.ToArray(); + } } } } diff --git a/Microsoft.Azure.Cosmos/src/Tracing/TraceData/SummaryDiagnostics.cs b/Microsoft.Azure.Cosmos/src/Tracing/TraceData/SummaryDiagnostics.cs index 64f3414529..e04003412d 100644 --- a/Microsoft.Azure.Cosmos/src/Tracing/TraceData/SummaryDiagnostics.cs +++ b/Microsoft.Azure.Cosmos/src/Tracing/TraceData/SummaryDiagnostics.cs @@ -38,9 +38,24 @@ public SummaryDiagnostics(ITrace trace) private void CollectSummaryFromTraceTree(ITrace currentTrace) { - foreach (object datums in currentTrace.Data.Values) + // Create snapshots to avoid concurrency issues + KeyValuePair[] dataSnapshot = null; + ITrace[] childrenSnapshot = null; + + if (currentTrace is Trace concreteTrace) + { + dataSnapshot = concreteTrace.GetDataSnapshot(); + childrenSnapshot = concreteTrace.GetChildrenSnapshot(); + } + else + { + dataSnapshot = currentTrace.Data.ToArray(); + childrenSnapshot = currentTrace.Children.ToArray(); + } + + foreach (KeyValuePair datum in dataSnapshot) { - if (datums is ClientSideRequestStatisticsTraceDatum clientSideRequestStatisticsTraceDatum) + if (datum.Value is ClientSideRequestStatisticsTraceDatum clientSideRequestStatisticsTraceDatum) { this.AggregateStatsFromStoreResults(clientSideRequestStatisticsTraceDatum.StoreResponseStatisticsList); this.AggregateGatewayStatistics(clientSideRequestStatisticsTraceDatum.HttpResponseStatisticsList); @@ -48,7 +63,7 @@ private void CollectSummaryFromTraceTree(ITrace currentTrace) } } - foreach (ITrace childTrace in currentTrace.Children) + foreach (ITrace childTrace in childrenSnapshot) { this.CollectSummaryFromTraceTree(childTrace); } diff --git a/Microsoft.Azure.Cosmos/src/Tracing/TraceWriter.TraceJsonWriter.cs b/Microsoft.Azure.Cosmos/src/Tracing/TraceWriter.TraceJsonWriter.cs index f3880a517c..1317efa1a5 100644 --- a/Microsoft.Azure.Cosmos/src/Tracing/TraceWriter.TraceJsonWriter.cs +++ b/Microsoft.Azure.Cosmos/src/Tracing/TraceWriter.TraceJsonWriter.cs @@ -53,12 +53,23 @@ public static void WriteTrace( writer.WriteFieldName("duration in milliseconds"); writer.WriteNumberValue(trace.Duration.TotalMilliseconds); - if (trace.Data.Any()) + // Create a snapshot of the data to avoid concurrency issues + KeyValuePair[] dataSnapshot = null; + if (trace is Trace concreteTrace) + { + dataSnapshot = concreteTrace.GetDataSnapshot(); + } + else if (trace.Data.Any()) + { + dataSnapshot = trace.Data.ToArray(); + } + + if (dataSnapshot != null && dataSnapshot.Any()) { writer.WriteFieldName("data"); writer.WriteObjectStart(); - foreach (KeyValuePair kvp in trace.Data) + foreach (KeyValuePair kvp in dataSnapshot) { string key = kvp.Key; object value = kvp.Value; @@ -70,12 +81,23 @@ public static void WriteTrace( writer.WriteObjectEnd(); } - if (trace.Children.Any()) + // Create a snapshot of the children to avoid concurrency issues + ITrace[] childrenSnapshot = null; + if (trace is Trace concreteTraceForChildren) + { + childrenSnapshot = concreteTraceForChildren.GetChildrenSnapshot(); + } + else if (trace.Children.Any()) + { + childrenSnapshot = trace.Children.ToArray(); + } + + if (childrenSnapshot != null && childrenSnapshot.Any()) { writer.WriteFieldName("children"); writer.WriteArrayStart(); - foreach (ITrace child in trace.Children) + foreach (ITrace child in childrenSnapshot) { WriteTrace(writer, child, diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/CosmosDiagnosticsUnitTests.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/CosmosDiagnosticsUnitTests.cs index 404955997d..a59d6057ae 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/CosmosDiagnosticsUnitTests.cs +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/CosmosDiagnosticsUnitTests.cs @@ -12,6 +12,7 @@ namespace Microsoft.Azure.Cosmos.Tests using System.Net.Http; using System.Threading; using System.Threading.Tasks; + using Microsoft.Azure.Cosmos.Diagnostics; using Microsoft.Azure.Cosmos.Tracing; using Microsoft.Azure.Cosmos.Tracing.TraceData; using Microsoft.Azure.Documents; @@ -137,6 +138,78 @@ public async Task ValidateActivityIdWithSynchronizationContext() } } + [TestMethod] + public void ValidateConcurrentToStringCalls() + { + // Create a trace and simulate concurrent access while calling ToString() + using ITrace trace = Microsoft.Azure.Cosmos.Tracing.Trace.GetRootTrace("concurrency-test"); + + Exception caughtException = null; + const int numThreads = 10; + const int numIterations = 100; + using CountdownEvent countdown = new CountdownEvent(numThreads); + + // Start multiple threads that will concurrently modify the trace + for (int i = 0; i < numThreads; i++) + { + int threadId = i; + Task.Run(() => + { + try + { + for (int j = 0; j < numIterations; j++) + { + // Add children and data concurrently + using ITrace child = trace.StartChild($"child-{threadId}-{j}"); + child.AddDatum($"key-{threadId}-{j}", $"value-{threadId}-{j}"); + + // Simulate some work + Thread.Sleep(1); + } + } + catch (Exception ex) + { + Interlocked.CompareExchange(ref caughtException, ex, null); + } + finally + { + countdown.Signal(); + } + }); + } + + // Concurrently call ToString() which caused the original issue + Task toStringTask = Task.Run(() => + { + try + { + for (int i = 0; i < numIterations; i++) + { + CosmosTraceDiagnostics diagnostics = new CosmosTraceDiagnostics(trace); + string diagnosticsString = diagnostics.ToString(); + Assert.IsNotNull(diagnosticsString); + + // Simulate some work + Thread.Sleep(1); + } + } + catch (Exception ex) + { + Interlocked.CompareExchange(ref caughtException, ex, null); + } + }); + + // Wait for all threads to complete + countdown.Wait(TimeSpan.FromSeconds(30)); + toStringTask.Wait(TimeSpan.FromSeconds(30)); + + // Verify no exceptions occurred + if (caughtException != null) + { + Assert.Fail($"Concurrent access caused exception: {caughtException}"); + } + } + private Task ValidateActivityIdHelper() { Guid activityId = System.Diagnostics.Trace.CorrelationManager.ActivityId; From ba096bd2d36e17b00575685e2ad178a0870f1770 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 1 Aug 2025 19:47:10 +0000 Subject: [PATCH 03/31] Complete fix for InvalidOperationException - handle all trace enumeration points Co-authored-by: kirankumarkolli <6880899+kirankumarkolli@users.noreply.github.com> --- .../src/Diagnostics/CosmosTraceDiagnostics.cs | 27 ++++++++- .../ServerSideMetricsTraceExtractor.cs | 55 +++++++++++++++++-- 2 files changed, 76 insertions(+), 6 deletions(-) diff --git a/Microsoft.Azure.Cosmos/src/Diagnostics/CosmosTraceDiagnostics.cs b/Microsoft.Azure.Cosmos/src/Diagnostics/CosmosTraceDiagnostics.cs index ef74ad3d6e..38d4280527 100644 --- a/Microsoft.Azure.Cosmos/src/Diagnostics/CosmosTraceDiagnostics.cs +++ b/Microsoft.Azure.Cosmos/src/Diagnostics/CosmosTraceDiagnostics.cs @@ -70,7 +70,19 @@ private bool WalkTraceTreeForGoneException(ITrace currentTrace) return false; } - foreach (object datum in currentTrace.Data.Values) + // Use thread-safe enumeration to avoid concurrency issues + object[] dataSnapshot = null; + if (currentTrace is Tracing.Trace concreteTrace) + { + var dataSnapshotPairs = concreteTrace.GetDataSnapshot(); + dataSnapshot = dataSnapshotPairs.Select(kvp => kvp.Value).ToArray(); + } + else + { + dataSnapshot = currentTrace.Data.Values.ToArray(); + } + + foreach (object datum in dataSnapshot) { if (datum is ClientSideRequestStatisticsTraceDatum clientSideRequestStatisticsTraceDatum) { @@ -84,7 +96,18 @@ private bool WalkTraceTreeForGoneException(ITrace currentTrace) } } - foreach (ITrace childTrace in currentTrace.Children) + // Use thread-safe enumeration to avoid concurrency issues + ITrace[] childrenSnapshot = null; + if (currentTrace is Tracing.Trace concreteTraceForChildren) + { + childrenSnapshot = concreteTraceForChildren.GetChildrenSnapshot(); + } + else + { + childrenSnapshot = currentTrace.Children.ToArray(); + } + + foreach (ITrace childTrace in childrenSnapshot) { if (this.WalkTraceTreeForGoneException(childTrace)) { diff --git a/Microsoft.Azure.Cosmos/src/Query/Core/Metrics/ServerSideMetricsTraceExtractor.cs b/Microsoft.Azure.Cosmos/src/Query/Core/Metrics/ServerSideMetricsTraceExtractor.cs index 0bfafabab9..889a822107 100644 --- a/Microsoft.Azure.Cosmos/src/Query/Core/Metrics/ServerSideMetricsTraceExtractor.cs +++ b/Microsoft.Azure.Cosmos/src/Query/Core/Metrics/ServerSideMetricsTraceExtractor.cs @@ -5,6 +5,7 @@ namespace Microsoft.Azure.Cosmos.Query.Core.Metrics { using System; + using System.Linq; using Microsoft.Azure.Cosmos.Tracing; using Microsoft.Azure.Cosmos.Tracing.TraceData; @@ -17,7 +18,19 @@ public static void WalkTraceTreeForQueryMetrics(ITrace currentTrace, ServerSideM return; } - foreach (object datum in currentTrace.Data.Values) + // Use thread-safe enumeration to avoid concurrency issues + object[] dataSnapshot = null; + if (currentTrace is Tracing.Trace concreteTrace) + { + var dataSnapshotPairs = concreteTrace.GetDataSnapshot(); + dataSnapshot = dataSnapshotPairs.Select(kvp => kvp.Value).ToArray(); + } + else + { + dataSnapshot = currentTrace.Data.Values.ToArray(); + } + + foreach (object datum in dataSnapshot) { if (datum is QueryMetricsTraceDatum queryMetricsTraceDatum) { @@ -28,7 +41,18 @@ public static void WalkTraceTreeForQueryMetrics(ITrace currentTrace, ServerSideM } } - foreach (ITrace childTrace in currentTrace.Children) + // Use thread-safe enumeration to avoid concurrency issues + ITrace[] childrenSnapshot = null; + if (currentTrace is Tracing.Trace concreteTraceForChildren) + { + childrenSnapshot = concreteTraceForChildren.GetChildrenSnapshot(); + } + else + { + childrenSnapshot = currentTrace.Children.ToArray(); + } + + foreach (ITrace childTrace in childrenSnapshot) { ServerSideMetricsTraceExtractor.WalkTraceTreeForQueryMetrics(childTrace, accumulator); } @@ -41,7 +65,19 @@ private static void WalkTraceTreeForPartitionInfo(ITrace currentTrace, ServerSid return; } - foreach (Object datum in currentTrace.Data.Values) + // Use thread-safe enumeration to avoid concurrency issues + object[] dataSnapshot = null; + if (currentTrace is Tracing.Trace concreteTrace) + { + var dataSnapshotPairs = concreteTrace.GetDataSnapshot(); + dataSnapshot = dataSnapshotPairs.Select(kvp => kvp.Value).ToArray(); + } + else + { + dataSnapshot = currentTrace.Data.Values.ToArray(); + } + + foreach (Object datum in dataSnapshot) { if (datum is ClientSideRequestStatisticsTraceDatum clientSideRequestStatisticsTraceDatum) { @@ -61,7 +97,18 @@ private static void WalkTraceTreeForPartitionInfo(ITrace currentTrace, ServerSid } } - foreach (ITrace childTrace in currentTrace.Children) + // Use thread-safe enumeration to avoid concurrency issues + ITrace[] childrenSnapshot2 = null; + if (currentTrace is Tracing.Trace concreteTraceForChildren2) + { + childrenSnapshot2 = concreteTraceForChildren2.GetChildrenSnapshot(); + } + else + { + childrenSnapshot2 = currentTrace.Children.ToArray(); + } + + foreach (ITrace childTrace in childrenSnapshot2) { ServerSideMetricsTraceExtractor.WalkTraceTreeForPartitionInfo(childTrace, serverSideMetrics); } From 9a2c674378a3874866b6d3d7f3a6c124d204309f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 14 Aug 2025 21:37:42 +0000 Subject: [PATCH 04/31] Update trace walking logic to prevent modifications during walk operations Co-authored-by: kirankumarkolli <6880899+kirankumarkolli@users.noreply.github.com> --- .../src/Diagnostics/CosmosTraceDiagnostics.cs | 55 ++++----- .../ServerSideMetricsTraceExtractor.cs | 75 ++++-------- Microsoft.Azure.Cosmos/src/Tracing/Trace.cs | 42 +++++++ .../Tracing/TraceData/SummaryDiagnostics.cs | 40 +++---- .../Tracing/TraceWriter.TraceJsonWriter.cs | 107 +++++++++--------- 5 files changed, 162 insertions(+), 157 deletions(-) diff --git a/Microsoft.Azure.Cosmos/src/Diagnostics/CosmosTraceDiagnostics.cs b/Microsoft.Azure.Cosmos/src/Diagnostics/CosmosTraceDiagnostics.cs index 38d4280527..fbaf56a0fb 100644 --- a/Microsoft.Azure.Cosmos/src/Diagnostics/CosmosTraceDiagnostics.cs +++ b/Microsoft.Azure.Cosmos/src/Diagnostics/CosmosTraceDiagnostics.cs @@ -70,52 +70,47 @@ private bool WalkTraceTreeForGoneException(ITrace currentTrace) return false; } - // Use thread-safe enumeration to avoid concurrency issues - object[] dataSnapshot = null; - if (currentTrace is Tracing.Trace concreteTrace) + // Set walking state to prevent modifications during the walk + bool isRootTrace = currentTrace == this.Value; + if (isRootTrace && currentTrace is Tracing.Trace rootConcreteTrace) { - var dataSnapshotPairs = concreteTrace.GetDataSnapshot(); - dataSnapshot = dataSnapshotPairs.Select(kvp => kvp.Value).ToArray(); - } - else - { - dataSnapshot = currentTrace.Data.Values.ToArray(); + rootConcreteTrace.SetWalkingStateRecursively(true); } - foreach (object datum in dataSnapshot) + try { - if (datum is ClientSideRequestStatisticsTraceDatum clientSideRequestStatisticsTraceDatum) + foreach (object datum in currentTrace.Data.Values) { - foreach (StoreResponseStatistics responseStatistics in clientSideRequestStatisticsTraceDatum.StoreResponseStatisticsList) + if (datum is ClientSideRequestStatisticsTraceDatum clientSideRequestStatisticsTraceDatum) { - if (responseStatistics.StoreResult != null && responseStatistics.StoreResult.StatusCode == Documents.StatusCodes.Gone) + foreach (StoreResponseStatistics responseStatistics in clientSideRequestStatisticsTraceDatum.StoreResponseStatisticsList) { - return true; + if (responseStatistics.StoreResult != null && responseStatistics.StoreResult.StatusCode == Documents.StatusCodes.Gone) + { + return true; + } } } } - } - // Use thread-safe enumeration to avoid concurrency issues - ITrace[] childrenSnapshot = null; - if (currentTrace is Tracing.Trace concreteTraceForChildren) - { - childrenSnapshot = concreteTraceForChildren.GetChildrenSnapshot(); - } - else - { - childrenSnapshot = currentTrace.Children.ToArray(); - } + foreach (ITrace childTrace in currentTrace.Children) + { + if (this.WalkTraceTreeForGoneException(childTrace)) + { + return true; + } + } - foreach (ITrace childTrace in childrenSnapshot) + return false; + } + finally { - if (this.WalkTraceTreeForGoneException(childTrace)) + // Clear walking state when done + if (isRootTrace && currentTrace is Tracing.Trace rootConcreteTraceFinally) { - return true; + rootConcreteTraceFinally.SetWalkingStateRecursively(false); } } - - return false; } private string ToJsonString() diff --git a/Microsoft.Azure.Cosmos/src/Query/Core/Metrics/ServerSideMetricsTraceExtractor.cs b/Microsoft.Azure.Cosmos/src/Query/Core/Metrics/ServerSideMetricsTraceExtractor.cs index 889a822107..602ce4754e 100644 --- a/Microsoft.Azure.Cosmos/src/Query/Core/Metrics/ServerSideMetricsTraceExtractor.cs +++ b/Microsoft.Azure.Cosmos/src/Query/Core/Metrics/ServerSideMetricsTraceExtractor.cs @@ -18,43 +18,37 @@ public static void WalkTraceTreeForQueryMetrics(ITrace currentTrace, ServerSideM return; } - // Use thread-safe enumeration to avoid concurrency issues - object[] dataSnapshot = null; + // Set walking state to prevent modifications during the walk if (currentTrace is Tracing.Trace concreteTrace) { - var dataSnapshotPairs = concreteTrace.GetDataSnapshot(); - dataSnapshot = dataSnapshotPairs.Select(kvp => kvp.Value).ToArray(); - } - else - { - dataSnapshot = currentTrace.Data.Values.ToArray(); + concreteTrace.SetWalkingStateRecursively(true); } - foreach (object datum in dataSnapshot) + try { - if (datum is QueryMetricsTraceDatum queryMetricsTraceDatum) + foreach (object datum in currentTrace.Data.Values) { - ServerSideMetricsInternal serverSideMetrics = queryMetricsTraceDatum.QueryMetrics.ServerSideMetrics; - serverSideMetrics.FeedRange = currentTrace.Name; - ServerSideMetricsTraceExtractor.WalkTraceTreeForPartitionInfo(currentTrace, serverSideMetrics); - accumulator.Accumulate(serverSideMetrics); + if (datum is QueryMetricsTraceDatum queryMetricsTraceDatum) + { + ServerSideMetricsInternal serverSideMetrics = queryMetricsTraceDatum.QueryMetrics.ServerSideMetrics; + serverSideMetrics.FeedRange = currentTrace.Name; + ServerSideMetricsTraceExtractor.WalkTraceTreeForPartitionInfo(currentTrace, serverSideMetrics); + accumulator.Accumulate(serverSideMetrics); + } } - } - // Use thread-safe enumeration to avoid concurrency issues - ITrace[] childrenSnapshot = null; - if (currentTrace is Tracing.Trace concreteTraceForChildren) - { - childrenSnapshot = concreteTraceForChildren.GetChildrenSnapshot(); - } - else - { - childrenSnapshot = currentTrace.Children.ToArray(); + foreach (ITrace childTrace in currentTrace.Children) + { + ServerSideMetricsTraceExtractor.WalkTraceTreeForQueryMetrics(childTrace, accumulator); + } } - - foreach (ITrace childTrace in childrenSnapshot) + finally { - ServerSideMetricsTraceExtractor.WalkTraceTreeForQueryMetrics(childTrace, accumulator); + // Clear walking state when done + if (currentTrace is Tracing.Trace concreteTraceFinally) + { + concreteTraceFinally.SetWalkingStateRecursively(false); + } } } @@ -65,19 +59,7 @@ private static void WalkTraceTreeForPartitionInfo(ITrace currentTrace, ServerSid return; } - // Use thread-safe enumeration to avoid concurrency issues - object[] dataSnapshot = null; - if (currentTrace is Tracing.Trace concreteTrace) - { - var dataSnapshotPairs = concreteTrace.GetDataSnapshot(); - dataSnapshot = dataSnapshotPairs.Select(kvp => kvp.Value).ToArray(); - } - else - { - dataSnapshot = currentTrace.Data.Values.ToArray(); - } - - foreach (Object datum in dataSnapshot) + foreach (Object datum in currentTrace.Data.Values) { if (datum is ClientSideRequestStatisticsTraceDatum clientSideRequestStatisticsTraceDatum) { @@ -97,18 +79,7 @@ private static void WalkTraceTreeForPartitionInfo(ITrace currentTrace, ServerSid } } - // Use thread-safe enumeration to avoid concurrency issues - ITrace[] childrenSnapshot2 = null; - if (currentTrace is Tracing.Trace concreteTraceForChildren2) - { - childrenSnapshot2 = concreteTraceForChildren2.GetChildrenSnapshot(); - } - else - { - childrenSnapshot2 = currentTrace.Children.ToArray(); - } - - foreach (ITrace childTrace in childrenSnapshot2) + foreach (ITrace childTrace in currentTrace.Children) { ServerSideMetricsTraceExtractor.WalkTraceTreeForPartitionInfo(childTrace, serverSideMetrics); } diff --git a/Microsoft.Azure.Cosmos/src/Tracing/Trace.cs b/Microsoft.Azure.Cosmos/src/Tracing/Trace.cs index 3ae9d18556..190a7a0f53 100644 --- a/Microsoft.Azure.Cosmos/src/Tracing/Trace.cs +++ b/Microsoft.Azure.Cosmos/src/Tracing/Trace.cs @@ -18,6 +18,7 @@ internal sealed class Trace : ITrace private readonly List children; private readonly Lazy> data; private ValueStopwatch stopwatch; + private volatile bool isBeingWalked; private Trace( string name, @@ -95,6 +96,11 @@ public ITrace StartChild( public void AddChild(ITrace child) { + if (this.isBeingWalked) + { + return; // Ignore modifications while being walked + } + lock (this.children) { this.children.Add(child); @@ -124,6 +130,11 @@ public static Trace GetRootTrace( public void AddDatum(string key, TraceDatum traceDatum) { + if (this.isBeingWalked) + { + return; // Ignore modifications while being walked + } + lock (this.children) { this.data.Value.Add(key, traceDatum); @@ -133,6 +144,11 @@ public void AddDatum(string key, TraceDatum traceDatum) public void AddDatum(string key, object value) { + if (this.isBeingWalked) + { + return; // Ignore modifications while being walked + } + lock (this.children) { this.data.Value.Add(key, value); @@ -141,6 +157,11 @@ public void AddDatum(string key, object value) public void AddOrUpdateDatum(string key, object value) { + if (this.isBeingWalked) + { + return; // Ignore modifications while being walked + } + lock (this.children) { this.data.Value[key] = value; @@ -167,5 +188,26 @@ internal KeyValuePair[] GetDataSnapshot() return this.data.Value.ToArray(); } } + + internal void SetWalkingState(bool isWalking) + { + this.isBeingWalked = isWalking; + } + + internal void SetWalkingStateRecursively(bool isWalking) + { + this.SetWalkingState(isWalking); + + lock (this.children) + { + foreach (ITrace child in this.children) + { + if (child is Trace concreteChild) + { + concreteChild.SetWalkingStateRecursively(isWalking); + } + } + } + } } } diff --git a/Microsoft.Azure.Cosmos/src/Tracing/TraceData/SummaryDiagnostics.cs b/Microsoft.Azure.Cosmos/src/Tracing/TraceData/SummaryDiagnostics.cs index e04003412d..fa4b71f5d0 100644 --- a/Microsoft.Azure.Cosmos/src/Tracing/TraceData/SummaryDiagnostics.cs +++ b/Microsoft.Azure.Cosmos/src/Tracing/TraceData/SummaryDiagnostics.cs @@ -38,34 +38,36 @@ public SummaryDiagnostics(ITrace trace) private void CollectSummaryFromTraceTree(ITrace currentTrace) { - // Create snapshots to avoid concurrency issues - KeyValuePair[] dataSnapshot = null; - ITrace[] childrenSnapshot = null; - + // Set walking state to prevent modifications during the walk if (currentTrace is Trace concreteTrace) { - dataSnapshot = concreteTrace.GetDataSnapshot(); - childrenSnapshot = concreteTrace.GetChildrenSnapshot(); - } - else - { - dataSnapshot = currentTrace.Data.ToArray(); - childrenSnapshot = currentTrace.Children.ToArray(); + concreteTrace.SetWalkingStateRecursively(true); } - foreach (KeyValuePair datum in dataSnapshot) + try { - if (datum.Value is ClientSideRequestStatisticsTraceDatum clientSideRequestStatisticsTraceDatum) + foreach (var datum in currentTrace.Data) { - this.AggregateStatsFromStoreResults(clientSideRequestStatisticsTraceDatum.StoreResponseStatisticsList); - this.AggregateGatewayStatistics(clientSideRequestStatisticsTraceDatum.HttpResponseStatisticsList); - this.AggregateRegionsContacted(clientSideRequestStatisticsTraceDatum.RegionsContacted); + if (datum.Value is ClientSideRequestStatisticsTraceDatum clientSideRequestStatisticsTraceDatum) + { + this.AggregateStatsFromStoreResults(clientSideRequestStatisticsTraceDatum.StoreResponseStatisticsList); + this.AggregateGatewayStatistics(clientSideRequestStatisticsTraceDatum.HttpResponseStatisticsList); + this.AggregateRegionsContacted(clientSideRequestStatisticsTraceDatum.RegionsContacted); + } } - } - foreach (ITrace childTrace in childrenSnapshot) + foreach (ITrace childTrace in currentTrace.Children) + { + this.CollectSummaryFromTraceTree(childTrace); + } + } + finally { - this.CollectSummaryFromTraceTree(childTrace); + // Clear walking state when done + if (currentTrace is Trace concreteTraceFinally) + { + concreteTraceFinally.SetWalkingStateRecursively(false); + } } } diff --git a/Microsoft.Azure.Cosmos/src/Tracing/TraceWriter.TraceJsonWriter.cs b/Microsoft.Azure.Cosmos/src/Tracing/TraceWriter.TraceJsonWriter.cs index 1317efa1a5..dbcd024d76 100644 --- a/Microsoft.Azure.Cosmos/src/Tracing/TraceWriter.TraceJsonWriter.cs +++ b/Microsoft.Azure.Cosmos/src/Tracing/TraceWriter.TraceJsonWriter.cs @@ -34,79 +34,74 @@ public static void WriteTrace( throw new ArgumentNullException(nameof(trace)); } - writer.WriteObjectStart(); - - if (isRootTrace) + // Set walking state to prevent modifications during the walk + if (isRootTrace && trace is Trace concreteTrace) { - writer.WriteFieldName("Summary"); - SummaryDiagnostics summaryDiagnostics = new SummaryDiagnostics(trace); - summaryDiagnostics.WriteSummaryDiagnostics(writer); + concreteTrace.SetWalkingStateRecursively(true); } - writer.WriteFieldName("name"); - writer.WriteStringValue(trace.Name); - if (isRootTrace) + try { - writer.WriteFieldName("start datetime"); - writer.WriteStringValue(trace.StartTime.ToString(TraceWriter.DateTimeFormatString)); - } - writer.WriteFieldName("duration in milliseconds"); - writer.WriteNumberValue(trace.Duration.TotalMilliseconds); + writer.WriteObjectStart(); - // Create a snapshot of the data to avoid concurrency issues - KeyValuePair[] dataSnapshot = null; - if (trace is Trace concreteTrace) - { - dataSnapshot = concreteTrace.GetDataSnapshot(); - } - else if (trace.Data.Any()) - { - dataSnapshot = trace.Data.ToArray(); - } + if (isRootTrace) + { + writer.WriteFieldName("Summary"); + SummaryDiagnostics summaryDiagnostics = new SummaryDiagnostics(trace); + summaryDiagnostics.WriteSummaryDiagnostics(writer); + } + writer.WriteFieldName("name"); + writer.WriteStringValue(trace.Name); - if (dataSnapshot != null && dataSnapshot.Any()) - { - writer.WriteFieldName("data"); - writer.WriteObjectStart(); + if (isRootTrace) + { + writer.WriteFieldName("start datetime"); + writer.WriteStringValue(trace.StartTime.ToString(TraceWriter.DateTimeFormatString)); + } + writer.WriteFieldName("duration in milliseconds"); + writer.WriteNumberValue(trace.Duration.TotalMilliseconds); - foreach (KeyValuePair kvp in dataSnapshot) + if (trace.Data.Any()) { - string key = kvp.Key; - object value = kvp.Value; + writer.WriteFieldName("data"); + writer.WriteObjectStart(); + + foreach (KeyValuePair kvp in trace.Data) + { + string key = kvp.Key; + object value = kvp.Value; - writer.WriteFieldName(key); - WriteTraceDatum(writer, value); + writer.WriteFieldName(key); + WriteTraceDatum(writer, value); + } + + writer.WriteObjectEnd(); } - writer.WriteObjectEnd(); - } + if (trace.Children.Any()) + { + writer.WriteFieldName("children"); + writer.WriteArrayStart(); - // Create a snapshot of the children to avoid concurrency issues - ITrace[] childrenSnapshot = null; - if (trace is Trace concreteTraceForChildren) - { - childrenSnapshot = concreteTraceForChildren.GetChildrenSnapshot(); - } - else if (trace.Children.Any()) - { - childrenSnapshot = trace.Children.ToArray(); - } + foreach (ITrace child in trace.Children) + { + WriteTrace(writer, + child, + isRootTrace: false); + } - if (childrenSnapshot != null && childrenSnapshot.Any()) + writer.WriteArrayEnd(); + } + writer.WriteObjectEnd(); + } + finally { - writer.WriteFieldName("children"); - writer.WriteArrayStart(); - - foreach (ITrace child in childrenSnapshot) + // Clear walking state when done + if (isRootTrace && trace is Trace concreteTraceFinally) { - WriteTrace(writer, - child, - isRootTrace: false); + concreteTraceFinally.SetWalkingStateRecursively(false); } - - writer.WriteArrayEnd(); } - writer.WriteObjectEnd(); } } From 4e93c9860a765fd94986af8740fc248ad9f185cd Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 14 Aug 2025 22:19:23 +0000 Subject: [PATCH 05/31] Remove unused snapshot methods from Trace class Since isBeingWalked flag prevents modifications during enumeration, snapshot methods are no longer needed. The trace system now uses direct enumeration with walking state protection instead of creating copies. Co-authored-by: kirankumarkolli <6880899+kirankumarkolli@users.noreply.github.com> --- Microsoft.Azure.Cosmos/src/Tracing/Trace.cs | 21 --------------------- 1 file changed, 21 deletions(-) diff --git a/Microsoft.Azure.Cosmos/src/Tracing/Trace.cs b/Microsoft.Azure.Cosmos/src/Tracing/Trace.cs index 190a7a0f53..6787668b95 100644 --- a/Microsoft.Azure.Cosmos/src/Tracing/Trace.cs +++ b/Microsoft.Azure.Cosmos/src/Tracing/Trace.cs @@ -168,27 +168,6 @@ public void AddOrUpdateDatum(string key, object value) } } - internal ITrace[] GetChildrenSnapshot() - { - lock (this.children) - { - return this.children.ToArray(); - } - } - - internal KeyValuePair[] GetDataSnapshot() - { - if (!this.data.IsValueCreated) - { - return new KeyValuePair[0]; - } - - lock (this.children) - { - return this.data.Value.ToArray(); - } - } - internal void SetWalkingState(bool isWalking) { this.isBeingWalked = isWalking; From 001766a8da4c62a845da8228da783cc7a5f1c02d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 14 Aug 2025 22:44:05 +0000 Subject: [PATCH 06/31] Address reviewer feedback: centralize walking state management in CosmosTraceDiagnostics Co-authored-by: kirankumarkolli <6880899+kirankumarkolli@users.noreply.github.com> --- .../src/Diagnostics/CosmosTraceDiagnostics.cs | 31 ++++++- .../ServerSideMetricsTraceExtractor.cs | 39 ++++---- Microsoft.Azure.Cosmos/src/Tracing/Trace.cs | 19 ++-- .../Tracing/TraceData/SummaryDiagnostics.cs | 31 +++---- .../Tracing/TraceWriter.TraceJsonWriter.cs | 89 ++++++++----------- 5 files changed, 107 insertions(+), 102 deletions(-) diff --git a/Microsoft.Azure.Cosmos/src/Diagnostics/CosmosTraceDiagnostics.cs b/Microsoft.Azure.Cosmos/src/Diagnostics/CosmosTraceDiagnostics.cs index fbaf56a0fb..e3f9510af6 100644 --- a/Microsoft.Azure.Cosmos/src/Diagnostics/CosmosTraceDiagnostics.cs +++ b/Microsoft.Azure.Cosmos/src/Diagnostics/CosmosTraceDiagnostics.cs @@ -40,6 +40,19 @@ public CosmosTraceDiagnostics(ITrace trace) public override string ToString() { + if (this.Value is Tracing.Trace rootConcreteTrace) + { + rootConcreteTrace.SetWalkingStateRecursively(true); + try + { + return this.ToJsonString(); + } + finally + { + rootConcreteTrace.SetWalkingStateRecursively(false); + } + } + return this.ToJsonString(); } @@ -129,7 +142,23 @@ private ReadOnlyMemory WriteTraceToJsonWriter(JsonSerializationFormat json private static ServerSideCumulativeMetrics PopulateServerSideCumulativeMetrics(ITrace trace) { ServerSideMetricsInternalAccumulator accumulator = new ServerSideMetricsInternalAccumulator(); - ServerSideMetricsTraceExtractor.WalkTraceTreeForQueryMetrics(trace, accumulator); + + if (trace is Tracing.Trace rootConcreteTrace) + { + rootConcreteTrace.SetWalkingStateRecursively(true); + try + { + ServerSideMetricsTraceExtractor.WalkTraceTreeForQueryMetrics(trace, accumulator); + } + finally + { + rootConcreteTrace.SetWalkingStateRecursively(false); + } + } + else + { + ServerSideMetricsTraceExtractor.WalkTraceTreeForQueryMetrics(trace, accumulator); + } IReadOnlyList serverSideMetricsList = accumulator.GetPartitionedServerSideMetrics().Select(metrics => new ServerSidePartitionedMetricsInternal(metrics)).ToList(); diff --git a/Microsoft.Azure.Cosmos/src/Query/Core/Metrics/ServerSideMetricsTraceExtractor.cs b/Microsoft.Azure.Cosmos/src/Query/Core/Metrics/ServerSideMetricsTraceExtractor.cs index 602ce4754e..b288d0e864 100644 --- a/Microsoft.Azure.Cosmos/src/Query/Core/Metrics/ServerSideMetricsTraceExtractor.cs +++ b/Microsoft.Azure.Cosmos/src/Query/Core/Metrics/ServerSideMetricsTraceExtractor.cs @@ -18,37 +18,26 @@ public static void WalkTraceTreeForQueryMetrics(ITrace currentTrace, ServerSideM return; } - // Set walking state to prevent modifications during the walk + // Assert that walking state is set if (currentTrace is Tracing.Trace concreteTrace) { - concreteTrace.SetWalkingStateRecursively(true); + System.Diagnostics.Debug.Assert(concreteTrace.IsBeingWalked, "SetWalkingStateRecursively should be set to true"); } - try + foreach (object datum in currentTrace.Data.Values) { - foreach (object datum in currentTrace.Data.Values) + if (datum is QueryMetricsTraceDatum queryMetricsTraceDatum) { - if (datum is QueryMetricsTraceDatum queryMetricsTraceDatum) - { - ServerSideMetricsInternal serverSideMetrics = queryMetricsTraceDatum.QueryMetrics.ServerSideMetrics; - serverSideMetrics.FeedRange = currentTrace.Name; - ServerSideMetricsTraceExtractor.WalkTraceTreeForPartitionInfo(currentTrace, serverSideMetrics); - accumulator.Accumulate(serverSideMetrics); - } - } - - foreach (ITrace childTrace in currentTrace.Children) - { - ServerSideMetricsTraceExtractor.WalkTraceTreeForQueryMetrics(childTrace, accumulator); + ServerSideMetricsInternal serverSideMetrics = queryMetricsTraceDatum.QueryMetrics.ServerSideMetrics; + serverSideMetrics.FeedRange = currentTrace.Name; + ServerSideMetricsTraceExtractor.WalkTraceTreeForPartitionInfo(currentTrace, serverSideMetrics); + accumulator.Accumulate(serverSideMetrics); } } - finally + + foreach (ITrace childTrace in currentTrace.Children) { - // Clear walking state when done - if (currentTrace is Tracing.Trace concreteTraceFinally) - { - concreteTraceFinally.SetWalkingStateRecursively(false); - } + ServerSideMetricsTraceExtractor.WalkTraceTreeForQueryMetrics(childTrace, accumulator); } } @@ -59,6 +48,12 @@ private static void WalkTraceTreeForPartitionInfo(ITrace currentTrace, ServerSid return; } + // Assert that walking state is set + if (currentTrace is Tracing.Trace concreteTrace) + { + System.Diagnostics.Debug.Assert(concreteTrace.IsBeingWalked, "SetWalkingStateRecursively should be set to true"); + } + foreach (Object datum in currentTrace.Data.Values) { if (datum is ClientSideRequestStatisticsTraceDatum clientSideRequestStatisticsTraceDatum) diff --git a/Microsoft.Azure.Cosmos/src/Tracing/Trace.cs b/Microsoft.Azure.Cosmos/src/Tracing/Trace.cs index 6787668b95..9de2c149df 100644 --- a/Microsoft.Azure.Cosmos/src/Tracing/Trace.cs +++ b/Microsoft.Azure.Cosmos/src/Tracing/Trace.cs @@ -59,6 +59,8 @@ private Trace( public IReadOnlyDictionary Data => this.data.IsValueCreated ? this.data.Value : Trace.EmptyDictionary; + internal bool IsBeingWalked => this.isBeingWalked; + public void Dispose() { this.stopwatch.Stop(); @@ -67,6 +69,11 @@ public void Dispose() public ITrace StartChild( string name) { + if (this.isBeingWalked) + { + return this; // Return self when being walked to avoid modifications + } + return this.StartChild( name, level: TraceLevel.Verbose, @@ -78,6 +85,11 @@ public ITrace StartChild( TraceComponent component, TraceLevel level) { + if (this.isBeingWalked) + { + return this; // Return self when being walked to avoid modifications + } + if (this.Parent != null && !this.stopwatch.IsRunning) { return this.Parent.StartChild(name, component, level); @@ -168,14 +180,9 @@ public void AddOrUpdateDatum(string key, object value) } } - internal void SetWalkingState(bool isWalking) - { - this.isBeingWalked = isWalking; - } - internal void SetWalkingStateRecursively(bool isWalking) { - this.SetWalkingState(isWalking); + this.isBeingWalked = isWalking; lock (this.children) { diff --git a/Microsoft.Azure.Cosmos/src/Tracing/TraceData/SummaryDiagnostics.cs b/Microsoft.Azure.Cosmos/src/Tracing/TraceData/SummaryDiagnostics.cs index fa4b71f5d0..4e7a9835c2 100644 --- a/Microsoft.Azure.Cosmos/src/Tracing/TraceData/SummaryDiagnostics.cs +++ b/Microsoft.Azure.Cosmos/src/Tracing/TraceData/SummaryDiagnostics.cs @@ -38,36 +38,25 @@ public SummaryDiagnostics(ITrace trace) private void CollectSummaryFromTraceTree(ITrace currentTrace) { - // Set walking state to prevent modifications during the walk + // Assert that walking state is set if (currentTrace is Trace concreteTrace) { - concreteTrace.SetWalkingStateRecursively(true); + System.Diagnostics.Debug.Assert(concreteTrace.IsBeingWalked, "SetWalkingStateRecursively should be set to true"); } - try + foreach (var datum in currentTrace.Data) { - foreach (var datum in currentTrace.Data) + if (datum.Value is ClientSideRequestStatisticsTraceDatum clientSideRequestStatisticsTraceDatum) { - if (datum.Value is ClientSideRequestStatisticsTraceDatum clientSideRequestStatisticsTraceDatum) - { - this.AggregateStatsFromStoreResults(clientSideRequestStatisticsTraceDatum.StoreResponseStatisticsList); - this.AggregateGatewayStatistics(clientSideRequestStatisticsTraceDatum.HttpResponseStatisticsList); - this.AggregateRegionsContacted(clientSideRequestStatisticsTraceDatum.RegionsContacted); - } - } - - foreach (ITrace childTrace in currentTrace.Children) - { - this.CollectSummaryFromTraceTree(childTrace); + this.AggregateStatsFromStoreResults(clientSideRequestStatisticsTraceDatum.StoreResponseStatisticsList); + this.AggregateGatewayStatistics(clientSideRequestStatisticsTraceDatum.HttpResponseStatisticsList); + this.AggregateRegionsContacted(clientSideRequestStatisticsTraceDatum.RegionsContacted); } } - finally + + foreach (ITrace childTrace in currentTrace.Children) { - // Clear walking state when done - if (currentTrace is Trace concreteTraceFinally) - { - concreteTraceFinally.SetWalkingStateRecursively(false); - } + this.CollectSummaryFromTraceTree(childTrace); } } diff --git a/Microsoft.Azure.Cosmos/src/Tracing/TraceWriter.TraceJsonWriter.cs b/Microsoft.Azure.Cosmos/src/Tracing/TraceWriter.TraceJsonWriter.cs index dbcd024d76..aa6529603d 100644 --- a/Microsoft.Azure.Cosmos/src/Tracing/TraceWriter.TraceJsonWriter.cs +++ b/Microsoft.Azure.Cosmos/src/Tracing/TraceWriter.TraceJsonWriter.cs @@ -34,74 +34,59 @@ public static void WriteTrace( throw new ArgumentNullException(nameof(trace)); } - // Set walking state to prevent modifications during the walk - if (isRootTrace && trace is Trace concreteTrace) + writer.WriteObjectStart(); + + if (isRootTrace) { - concreteTrace.SetWalkingStateRecursively(true); + writer.WriteFieldName("Summary"); + SummaryDiagnostics summaryDiagnostics = new SummaryDiagnostics(trace); + summaryDiagnostics.WriteSummaryDiagnostics(writer); } + writer.WriteFieldName("name"); + writer.WriteStringValue(trace.Name); - try + if (isRootTrace) { + writer.WriteFieldName("start datetime"); + writer.WriteStringValue(trace.StartTime.ToString(TraceWriter.DateTimeFormatString)); + } + writer.WriteFieldName("duration in milliseconds"); + writer.WriteNumberValue(trace.Duration.TotalMilliseconds); + + // Use direct access since walking state prevents modifications + if (trace.Data.Any()) + { + writer.WriteFieldName("data"); writer.WriteObjectStart(); - if (isRootTrace) + foreach (KeyValuePair kvp in trace.Data) { - writer.WriteFieldName("Summary"); - SummaryDiagnostics summaryDiagnostics = new SummaryDiagnostics(trace); - summaryDiagnostics.WriteSummaryDiagnostics(writer); - } - writer.WriteFieldName("name"); - writer.WriteStringValue(trace.Name); + string key = kvp.Key; + object value = kvp.Value; - if (isRootTrace) - { - writer.WriteFieldName("start datetime"); - writer.WriteStringValue(trace.StartTime.ToString(TraceWriter.DateTimeFormatString)); + writer.WriteFieldName(key); + WriteTraceDatum(writer, value); } - writer.WriteFieldName("duration in milliseconds"); - writer.WriteNumberValue(trace.Duration.TotalMilliseconds); - - if (trace.Data.Any()) - { - writer.WriteFieldName("data"); - writer.WriteObjectStart(); - - foreach (KeyValuePair kvp in trace.Data) - { - string key = kvp.Key; - object value = kvp.Value; - - writer.WriteFieldName(key); - WriteTraceDatum(writer, value); - } - writer.WriteObjectEnd(); - } - - if (trace.Children.Any()) - { - writer.WriteFieldName("children"); - writer.WriteArrayStart(); - - foreach (ITrace child in trace.Children) - { - WriteTrace(writer, - child, - isRootTrace: false); - } - - writer.WriteArrayEnd(); - } writer.WriteObjectEnd(); } - finally + + // Use direct access since walking state prevents modifications + if (trace.Children.Any()) { - // Clear walking state when done - if (isRootTrace && trace is Trace concreteTraceFinally) + writer.WriteFieldName("children"); + writer.WriteArrayStart(); + + foreach (ITrace child in trace.Children) { - concreteTraceFinally.SetWalkingStateRecursively(false); + WriteTrace(writer, + child, + isRootTrace: false); } + + writer.WriteArrayEnd(); } + writer.WriteObjectEnd(); } } From 7c15aa8aff5494b8ff54f790716980aabdcfd50e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 14 Aug 2025 23:28:30 +0000 Subject: [PATCH 07/31] Address reviewer feedback: return NoOpTrace and fix locking strategy Co-authored-by: kirankumarkolli <6880899+kirankumarkolli@users.noreply.github.com> --- Microsoft.Azure.Cosmos/src/Tracing/Trace.cs | 44 ++++----- .../Tracing/TraceWriter.TraceJsonWriter.cs | 89 +++++++++++-------- 2 files changed, 74 insertions(+), 59 deletions(-) diff --git a/Microsoft.Azure.Cosmos/src/Tracing/Trace.cs b/Microsoft.Azure.Cosmos/src/Tracing/Trace.cs index 9de2c149df..d4698d5cfa 100644 --- a/Microsoft.Azure.Cosmos/src/Tracing/Trace.cs +++ b/Microsoft.Azure.Cosmos/src/Tracing/Trace.cs @@ -71,7 +71,7 @@ public ITrace StartChild( { if (this.isBeingWalked) { - return this; // Return self when being walked to avoid modifications + return NoOpTrace.Singleton; } return this.StartChild( @@ -87,7 +87,7 @@ public ITrace StartChild( { if (this.isBeingWalked) { - return this; // Return self when being walked to avoid modifications + return NoOpTrace.Singleton; } if (this.Parent != null && !this.stopwatch.IsRunning) @@ -108,13 +108,13 @@ public ITrace StartChild( public void AddChild(ITrace child) { - if (this.isBeingWalked) - { - return; // Ignore modifications while being walked - } - lock (this.children) { + if (this.isBeingWalked) + { + return; // Ignore modifications while being walked + } + this.children.Add(child); } } @@ -142,13 +142,13 @@ public static Trace GetRootTrace( public void AddDatum(string key, TraceDatum traceDatum) { - if (this.isBeingWalked) - { - return; // Ignore modifications while being walked - } - lock (this.children) { + if (this.isBeingWalked) + { + return; // Ignore modifications while being walked + } + this.data.Value.Add(key, traceDatum); } this.Summary.UpdateRegionContacted(traceDatum); @@ -156,26 +156,26 @@ public void AddDatum(string key, TraceDatum traceDatum) public void AddDatum(string key, object value) { - if (this.isBeingWalked) - { - return; // Ignore modifications while being walked - } - lock (this.children) { + if (this.isBeingWalked) + { + return; // Ignore modifications while being walked + } + this.data.Value.Add(key, value); } } public void AddOrUpdateDatum(string key, object value) { - if (this.isBeingWalked) - { - return; // Ignore modifications while being walked - } - lock (this.children) { + if (this.isBeingWalked) + { + return; // Ignore modifications while being walked + } + this.data.Value[key] = value; } } diff --git a/Microsoft.Azure.Cosmos/src/Tracing/TraceWriter.TraceJsonWriter.cs b/Microsoft.Azure.Cosmos/src/Tracing/TraceWriter.TraceJsonWriter.cs index aa6529603d..dbcd024d76 100644 --- a/Microsoft.Azure.Cosmos/src/Tracing/TraceWriter.TraceJsonWriter.cs +++ b/Microsoft.Azure.Cosmos/src/Tracing/TraceWriter.TraceJsonWriter.cs @@ -34,59 +34,74 @@ public static void WriteTrace( throw new ArgumentNullException(nameof(trace)); } - writer.WriteObjectStart(); - - if (isRootTrace) + // Set walking state to prevent modifications during the walk + if (isRootTrace && trace is Trace concreteTrace) { - writer.WriteFieldName("Summary"); - SummaryDiagnostics summaryDiagnostics = new SummaryDiagnostics(trace); - summaryDiagnostics.WriteSummaryDiagnostics(writer); + concreteTrace.SetWalkingStateRecursively(true); } - writer.WriteFieldName("name"); - writer.WriteStringValue(trace.Name); - if (isRootTrace) + try { - writer.WriteFieldName("start datetime"); - writer.WriteStringValue(trace.StartTime.ToString(TraceWriter.DateTimeFormatString)); - } - writer.WriteFieldName("duration in milliseconds"); - writer.WriteNumberValue(trace.Duration.TotalMilliseconds); - - // Use direct access since walking state prevents modifications - if (trace.Data.Any()) - { - writer.WriteFieldName("data"); writer.WriteObjectStart(); - foreach (KeyValuePair kvp in trace.Data) + if (isRootTrace) { - string key = kvp.Key; - object value = kvp.Value; + writer.WriteFieldName("Summary"); + SummaryDiagnostics summaryDiagnostics = new SummaryDiagnostics(trace); + summaryDiagnostics.WriteSummaryDiagnostics(writer); + } + writer.WriteFieldName("name"); + writer.WriteStringValue(trace.Name); - writer.WriteFieldName(key); - WriteTraceDatum(writer, value); + if (isRootTrace) + { + writer.WriteFieldName("start datetime"); + writer.WriteStringValue(trace.StartTime.ToString(TraceWriter.DateTimeFormatString)); } + writer.WriteFieldName("duration in milliseconds"); + writer.WriteNumberValue(trace.Duration.TotalMilliseconds); + + if (trace.Data.Any()) + { + writer.WriteFieldName("data"); + writer.WriteObjectStart(); + + foreach (KeyValuePair kvp in trace.Data) + { + string key = kvp.Key; + object value = kvp.Value; + + writer.WriteFieldName(key); + WriteTraceDatum(writer, value); + } + writer.WriteObjectEnd(); + } + + if (trace.Children.Any()) + { + writer.WriteFieldName("children"); + writer.WriteArrayStart(); + + foreach (ITrace child in trace.Children) + { + WriteTrace(writer, + child, + isRootTrace: false); + } + + writer.WriteArrayEnd(); + } writer.WriteObjectEnd(); } - - // Use direct access since walking state prevents modifications - if (trace.Children.Any()) + finally { - writer.WriteFieldName("children"); - writer.WriteArrayStart(); - - foreach (ITrace child in trace.Children) + // Clear walking state when done + if (isRootTrace && trace is Trace concreteTraceFinally) { - WriteTrace(writer, - child, - isRootTrace: false); + concreteTraceFinally.SetWalkingStateRecursively(false); } - - writer.WriteArrayEnd(); } - writer.WriteObjectEnd(); } } From 824d33d2d78420567efaf522adf9b066d41a2a3f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 15 Aug 2025 00:28:38 +0000 Subject: [PATCH 08/31] Address reviewer feedback: centralize walking state management and remove parameter Co-authored-by: kirankumarkolli <6880899+kirankumarkolli@users.noreply.github.com> --- .../src/Diagnostics/CosmosTraceDiagnostics.cs | 84 ++++++----------- Microsoft.Azure.Cosmos/src/Tracing/Trace.cs | 15 ++-- .../Tracing/TraceWriter.TraceJsonWriter.cs | 89 ++++++++----------- 3 files changed, 76 insertions(+), 112 deletions(-) diff --git a/Microsoft.Azure.Cosmos/src/Diagnostics/CosmosTraceDiagnostics.cs b/Microsoft.Azure.Cosmos/src/Diagnostics/CosmosTraceDiagnostics.cs index e3f9510af6..568a9a9c68 100644 --- a/Microsoft.Azure.Cosmos/src/Diagnostics/CosmosTraceDiagnostics.cs +++ b/Microsoft.Azure.Cosmos/src/Diagnostics/CosmosTraceDiagnostics.cs @@ -42,15 +42,8 @@ public override string ToString() { if (this.Value is Tracing.Trace rootConcreteTrace) { - rootConcreteTrace.SetWalkingStateRecursively(true); - try - { - return this.ToJsonString(); - } - finally - { - rootConcreteTrace.SetWalkingStateRecursively(false); - } + rootConcreteTrace.SetWalkingStateRecursively(); + return this.ToJsonString(); } return this.ToJsonString(); @@ -63,16 +56,31 @@ public override TimeSpan GetClientElapsedTime() public override IReadOnlyList<(string regionName, Uri uri)> GetContactedRegions() { + if (this.Value is Tracing.Trace rootConcreteTrace) + { + rootConcreteTrace.SetWalkingStateRecursively(); + } + return this.Value?.Summary?.RegionsContacted; } public override ServerSideCumulativeMetrics GetQueryMetrics() { + if (this.Value is Tracing.Trace rootConcreteTrace) + { + rootConcreteTrace.SetWalkingStateRecursively(); + } + return this.accumulatedMetrics.Value; } internal bool IsGoneExceptionHit() { + if (this.Value is Tracing.Trace rootConcreteTrace) + { + rootConcreteTrace.SetWalkingStateRecursively(); + } + return this.WalkTraceTreeForGoneException(this.Value); } @@ -83,47 +91,29 @@ private bool WalkTraceTreeForGoneException(ITrace currentTrace) return false; } - // Set walking state to prevent modifications during the walk - bool isRootTrace = currentTrace == this.Value; - if (isRootTrace && currentTrace is Tracing.Trace rootConcreteTrace) - { - rootConcreteTrace.SetWalkingStateRecursively(true); - } - - try + foreach (object datum in currentTrace.Data.Values) { - foreach (object datum in currentTrace.Data.Values) + if (datum is ClientSideRequestStatisticsTraceDatum clientSideRequestStatisticsTraceDatum) { - if (datum is ClientSideRequestStatisticsTraceDatum clientSideRequestStatisticsTraceDatum) + foreach (StoreResponseStatistics responseStatistics in clientSideRequestStatisticsTraceDatum.StoreResponseStatisticsList) { - foreach (StoreResponseStatistics responseStatistics in clientSideRequestStatisticsTraceDatum.StoreResponseStatisticsList) + if (responseStatistics.StoreResult != null && responseStatistics.StoreResult.StatusCode == Documents.StatusCodes.Gone) { - if (responseStatistics.StoreResult != null && responseStatistics.StoreResult.StatusCode == Documents.StatusCodes.Gone) - { - return true; - } + return true; } } } - - foreach (ITrace childTrace in currentTrace.Children) - { - if (this.WalkTraceTreeForGoneException(childTrace)) - { - return true; - } - } - - return false; } - finally + + foreach (ITrace childTrace in currentTrace.Children) { - // Clear walking state when done - if (isRootTrace && currentTrace is Tracing.Trace rootConcreteTraceFinally) + if (this.WalkTraceTreeForGoneException(childTrace)) { - rootConcreteTraceFinally.SetWalkingStateRecursively(false); + return true; } } + + return false; } private string ToJsonString() @@ -142,23 +132,7 @@ private ReadOnlyMemory WriteTraceToJsonWriter(JsonSerializationFormat json private static ServerSideCumulativeMetrics PopulateServerSideCumulativeMetrics(ITrace trace) { ServerSideMetricsInternalAccumulator accumulator = new ServerSideMetricsInternalAccumulator(); - - if (trace is Tracing.Trace rootConcreteTrace) - { - rootConcreteTrace.SetWalkingStateRecursively(true); - try - { - ServerSideMetricsTraceExtractor.WalkTraceTreeForQueryMetrics(trace, accumulator); - } - finally - { - rootConcreteTrace.SetWalkingStateRecursively(false); - } - } - else - { - ServerSideMetricsTraceExtractor.WalkTraceTreeForQueryMetrics(trace, accumulator); - } + ServerSideMetricsTraceExtractor.WalkTraceTreeForQueryMetrics(trace, accumulator); IReadOnlyList serverSideMetricsList = accumulator.GetPartitionedServerSideMetrics().Select(metrics => new ServerSidePartitionedMetricsInternal(metrics)).ToList(); diff --git a/Microsoft.Azure.Cosmos/src/Tracing/Trace.cs b/Microsoft.Azure.Cosmos/src/Tracing/Trace.cs index d4698d5cfa..2a53ef58a4 100644 --- a/Microsoft.Azure.Cosmos/src/Tracing/Trace.cs +++ b/Microsoft.Azure.Cosmos/src/Tracing/Trace.cs @@ -150,8 +150,8 @@ public void AddDatum(string key, TraceDatum traceDatum) } this.data.Value.Add(key, traceDatum); + this.Summary.UpdateRegionContacted(traceDatum); } - this.Summary.UpdateRegionContacted(traceDatum); } public void AddDatum(string key, object value) @@ -180,17 +180,22 @@ public void AddOrUpdateDatum(string key, object value) } } - internal void SetWalkingStateRecursively(bool isWalking) + internal void SetWalkingStateRecursively() { - this.isBeingWalked = isWalking; - lock (this.children) { + if (this.isBeingWalked) + { + return; // Already set, return early + } + + this.isBeingWalked = true; + foreach (ITrace child in this.children) { if (child is Trace concreteChild) { - concreteChild.SetWalkingStateRecursively(isWalking); + concreteChild.SetWalkingStateRecursively(); } } } diff --git a/Microsoft.Azure.Cosmos/src/Tracing/TraceWriter.TraceJsonWriter.cs b/Microsoft.Azure.Cosmos/src/Tracing/TraceWriter.TraceJsonWriter.cs index dbcd024d76..34e7309d71 100644 --- a/Microsoft.Azure.Cosmos/src/Tracing/TraceWriter.TraceJsonWriter.cs +++ b/Microsoft.Azure.Cosmos/src/Tracing/TraceWriter.TraceJsonWriter.cs @@ -34,74 +34,59 @@ public static void WriteTrace( throw new ArgumentNullException(nameof(trace)); } - // Set walking state to prevent modifications during the walk - if (isRootTrace && trace is Trace concreteTrace) + writer.WriteObjectStart(); + + if (isRootTrace) { - concreteTrace.SetWalkingStateRecursively(true); + writer.WriteFieldName("Summary"); + SummaryDiagnostics summaryDiagnostics = new SummaryDiagnostics(trace); + summaryDiagnostics.WriteSummaryDiagnostics(writer); } + writer.WriteFieldName("name"); + writer.WriteStringValue(trace.Name); - try + if (isRootTrace) { + writer.WriteFieldName("start datetime"); + writer.WriteStringValue(trace.StartTime.ToString(TraceWriter.DateTimeFormatString)); + } + writer.WriteFieldName("duration in milliseconds"); + writer.WriteNumberValue(trace.Duration.TotalMilliseconds); + + // Use direct enumeration protected by walking state + if (trace.Data.Any()) + { + writer.WriteFieldName("data"); writer.WriteObjectStart(); - if (isRootTrace) + foreach (KeyValuePair kvp in trace.Data) { - writer.WriteFieldName("Summary"); - SummaryDiagnostics summaryDiagnostics = new SummaryDiagnostics(trace); - summaryDiagnostics.WriteSummaryDiagnostics(writer); - } - writer.WriteFieldName("name"); - writer.WriteStringValue(trace.Name); + string key = kvp.Key; + object value = kvp.Value; - if (isRootTrace) - { - writer.WriteFieldName("start datetime"); - writer.WriteStringValue(trace.StartTime.ToString(TraceWriter.DateTimeFormatString)); + writer.WriteFieldName(key); + WriteTraceDatum(writer, value); } - writer.WriteFieldName("duration in milliseconds"); - writer.WriteNumberValue(trace.Duration.TotalMilliseconds); - - if (trace.Data.Any()) - { - writer.WriteFieldName("data"); - writer.WriteObjectStart(); - - foreach (KeyValuePair kvp in trace.Data) - { - string key = kvp.Key; - object value = kvp.Value; - - writer.WriteFieldName(key); - WriteTraceDatum(writer, value); - } - writer.WriteObjectEnd(); - } - - if (trace.Children.Any()) - { - writer.WriteFieldName("children"); - writer.WriteArrayStart(); - - foreach (ITrace child in trace.Children) - { - WriteTrace(writer, - child, - isRootTrace: false); - } - - writer.WriteArrayEnd(); - } writer.WriteObjectEnd(); } - finally + + // Use direct enumeration protected by walking state + if (trace.Children.Any()) { - // Clear walking state when done - if (isRootTrace && trace is Trace concreteTraceFinally) + writer.WriteFieldName("children"); + writer.WriteArrayStart(); + + foreach (ITrace child in trace.Children) { - concreteTraceFinally.SetWalkingStateRecursively(false); + WriteTrace(writer, + child, + isRootTrace: false); } + + writer.WriteArrayEnd(); } + writer.WriteObjectEnd(); } } From dd1ccd7d9fe36b31b131b6acb69f73c4d75cf43a Mon Sep 17 00:00:00 2001 From: Kiran Kumar Kolli Date: Thu, 14 Aug 2025 20:05:55 -0700 Subject: [PATCH 09/31] Some nits manually pushed --- Microsoft.Azure.Cosmos/src/Tracing/Trace.cs | 5 +++-- .../src/Tracing/TraceData/SummaryDiagnostics.cs | 4 ++-- .../src/Tracing/TraceWriter.TraceJsonWriter.cs | 2 -- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/Microsoft.Azure.Cosmos/src/Tracing/Trace.cs b/Microsoft.Azure.Cosmos/src/Tracing/Trace.cs index 2a53ef58a4..f70b3c4ef6 100644 --- a/Microsoft.Azure.Cosmos/src/Tracing/Trace.cs +++ b/Microsoft.Azure.Cosmos/src/Tracing/Trace.cs @@ -189,8 +189,6 @@ internal void SetWalkingStateRecursively() return; // Already set, return early } - this.isBeingWalked = true; - foreach (ITrace child in this.children) { if (child is Trace concreteChild) @@ -198,6 +196,9 @@ internal void SetWalkingStateRecursively() concreteChild.SetWalkingStateRecursively(); } } + + // Set the walking state for this trace + this.isBeingWalked = true; } } } diff --git a/Microsoft.Azure.Cosmos/src/Tracing/TraceData/SummaryDiagnostics.cs b/Microsoft.Azure.Cosmos/src/Tracing/TraceData/SummaryDiagnostics.cs index 4e7a9835c2..2157596e97 100644 --- a/Microsoft.Azure.Cosmos/src/Tracing/TraceData/SummaryDiagnostics.cs +++ b/Microsoft.Azure.Cosmos/src/Tracing/TraceData/SummaryDiagnostics.cs @@ -44,9 +44,9 @@ private void CollectSummaryFromTraceTree(ITrace currentTrace) System.Diagnostics.Debug.Assert(concreteTrace.IsBeingWalked, "SetWalkingStateRecursively should be set to true"); } - foreach (var datum in currentTrace.Data) + foreach (object datums in currentTrace.Data.Values) { - if (datum.Value is ClientSideRequestStatisticsTraceDatum clientSideRequestStatisticsTraceDatum) + if (datums is ClientSideRequestStatisticsTraceDatum clientSideRequestStatisticsTraceDatum) { this.AggregateStatsFromStoreResults(clientSideRequestStatisticsTraceDatum.StoreResponseStatisticsList); this.AggregateGatewayStatistics(clientSideRequestStatisticsTraceDatum.HttpResponseStatisticsList); diff --git a/Microsoft.Azure.Cosmos/src/Tracing/TraceWriter.TraceJsonWriter.cs b/Microsoft.Azure.Cosmos/src/Tracing/TraceWriter.TraceJsonWriter.cs index 34e7309d71..f3880a517c 100644 --- a/Microsoft.Azure.Cosmos/src/Tracing/TraceWriter.TraceJsonWriter.cs +++ b/Microsoft.Azure.Cosmos/src/Tracing/TraceWriter.TraceJsonWriter.cs @@ -53,7 +53,6 @@ public static void WriteTrace( writer.WriteFieldName("duration in milliseconds"); writer.WriteNumberValue(trace.Duration.TotalMilliseconds); - // Use direct enumeration protected by walking state if (trace.Data.Any()) { writer.WriteFieldName("data"); @@ -71,7 +70,6 @@ public static void WriteTrace( writer.WriteObjectEnd(); } - // Use direct enumeration protected by walking state if (trace.Children.Any()) { writer.WriteFieldName("children"); From 4a302d03d67701225e83dcb6d10e34e85b11aa23 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 15 Aug 2025 03:21:21 +0000 Subject: [PATCH 10/31] Address reviewer feedback: move IsBeingWalked to ITrace interface and adjust assignment order Co-authored-by: kirankumarkolli <6880899+kirankumarkolli@users.noreply.github.com> --- .../src/Pagination/ParallelPrefetch.Testing.cs | 2 ++ Microsoft.Azure.Cosmos/src/Tracing/ITrace.cs | 5 +++++ Microsoft.Azure.Cosmos/src/Tracing/NoOpTrace.cs | 2 ++ Microsoft.Azure.Cosmos/src/Tracing/Trace.cs | 8 ++++---- .../Tracing/EndToEndTraceWriterBaselineTests.cs | 2 ++ .../Tracing/TraceJoiner.cs | 2 ++ .../Pagination/ParallelPrefetchTests.cs | 2 ++ .../Tracing/TraceWriterBaselineTests.cs | 2 ++ 8 files changed, 21 insertions(+), 4 deletions(-) diff --git a/Microsoft.Azure.Cosmos/src/Pagination/ParallelPrefetch.Testing.cs b/Microsoft.Azure.Cosmos/src/Pagination/ParallelPrefetch.Testing.cs index b37fa2f456..2e4cb8b601 100644 --- a/Microsoft.Azure.Cosmos/src/Pagination/ParallelPrefetch.Testing.cs +++ b/Microsoft.Azure.Cosmos/src/Pagination/ParallelPrefetch.Testing.cs @@ -57,6 +57,8 @@ public int AwaitedTasks IReadOnlyDictionary ITrace.Data => this.innerTrace.Data; + bool ITrace.IsBeingWalked => this.innerTrace.IsBeingWalked; + public ParallelPrefetchTestConfig( ArrayPool prefetcherPool, ArrayPool taskPool, diff --git a/Microsoft.Azure.Cosmos/src/Tracing/ITrace.cs b/Microsoft.Azure.Cosmos/src/Tracing/ITrace.cs index bd67890596..22c2c75821 100644 --- a/Microsoft.Azure.Cosmos/src/Tracing/ITrace.cs +++ b/Microsoft.Azure.Cosmos/src/Tracing/ITrace.cs @@ -68,6 +68,11 @@ interface ITrace : IDisposable /// IReadOnlyDictionary Data { get; } + /// + /// Gets a value indicating whether this trace is currently being walked. + /// + bool IsBeingWalked { get; } + /// /// Starts a Trace and adds it as a child to this instance. /// diff --git a/Microsoft.Azure.Cosmos/src/Tracing/NoOpTrace.cs b/Microsoft.Azure.Cosmos/src/Tracing/NoOpTrace.cs index b76e95fc5e..aeb01392ed 100644 --- a/Microsoft.Azure.Cosmos/src/Tracing/NoOpTrace.cs +++ b/Microsoft.Azure.Cosmos/src/Tracing/NoOpTrace.cs @@ -40,6 +40,8 @@ private NoOpTrace() public IReadOnlyDictionary Data => NoOpData; + public bool IsBeingWalked => false; + public void Dispose() { // NoOp diff --git a/Microsoft.Azure.Cosmos/src/Tracing/Trace.cs b/Microsoft.Azure.Cosmos/src/Tracing/Trace.cs index f70b3c4ef6..46cac3e798 100644 --- a/Microsoft.Azure.Cosmos/src/Tracing/Trace.cs +++ b/Microsoft.Azure.Cosmos/src/Tracing/Trace.cs @@ -59,7 +59,7 @@ private Trace( public IReadOnlyDictionary Data => this.data.IsValueCreated ? this.data.Value : Trace.EmptyDictionary; - internal bool IsBeingWalked => this.isBeingWalked; + public bool IsBeingWalked => this.isBeingWalked; public void Dispose() { @@ -196,10 +196,10 @@ internal void SetWalkingStateRecursively() concreteChild.SetWalkingStateRecursively(); } } - - // Set the walking state for this trace - this.isBeingWalked = true; } + + // Set the walking state for this trace after processing children + this.isBeingWalked = true; } } } diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/Tracing/EndToEndTraceWriterBaselineTests.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/Tracing/EndToEndTraceWriterBaselineTests.cs index d056374f38..40bb10ea24 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/Tracing/EndToEndTraceWriterBaselineTests.cs +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/Tracing/EndToEndTraceWriterBaselineTests.cs @@ -1865,6 +1865,8 @@ public TraceForBaselineTesting( public IReadOnlyDictionary Data => this.data; + public bool IsBeingWalked => false; + public IReadOnlyList<(string, Uri)> RegionsContacted => new List<(string, Uri)>(); public void AddDatum(string key, TraceDatum traceDatum) diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/Tracing/TraceJoiner.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/Tracing/TraceJoiner.cs index ab8ef13a8d..6f2f3b7914 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/Tracing/TraceJoiner.cs +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/Tracing/TraceJoiner.cs @@ -65,6 +65,8 @@ public TraceForest(IReadOnlyList children) public IReadOnlyDictionary Data => this.data; + public bool IsBeingWalked => false; + public IReadOnlyList<(string, Uri)> RegionsContacted => new List<(string, Uri)>(); public void AddDatum(string key, TraceDatum traceDatum) diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Pagination/ParallelPrefetchTests.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Pagination/ParallelPrefetchTests.cs index 3a5f5459d9..4e674a76a9 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Pagination/ParallelPrefetchTests.cs +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Pagination/ParallelPrefetchTests.cs @@ -459,6 +459,8 @@ private sealed class SimpleTrace : ITrace public IReadOnlyDictionary Data { get; } = new Dictionary(); + public bool IsBeingWalked => false; + internal SimpleTrace(ITrace parent, string name, TraceComponent component, Cosmos.Tracing.TraceLevel level) { this.Parent = parent; diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Tracing/TraceWriterBaselineTests.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Tracing/TraceWriterBaselineTests.cs index b7b165fc5b..666b00702f 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Tracing/TraceWriterBaselineTests.cs +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Tracing/TraceWriterBaselineTests.cs @@ -905,6 +905,8 @@ public TraceForBaselineTesting( public IReadOnlyDictionary Data => this.data; + public bool IsBeingWalked => false; + public IReadOnlyList<(string, Uri)> RegionsContacted => new List<(string, Uri)>(); public void AddDatum(string key, TraceDatum traceDatum) From 1c0d07277456261593e72effaf21a3c88dbc3289 Mon Sep 17 00:00:00 2001 From: Kiran Kumar Kolli Date: Thu, 14 Aug 2025 20:34:25 -0700 Subject: [PATCH 11/31] Few nits --- .../src/Diagnostics/CosmosTraceDiagnostics.cs | 1 - .../Core/Metrics/ServerSideMetricsTraceExtractor.cs | 11 ++--------- Microsoft.Azure.Cosmos/src/Tracing/NoOpTrace.cs | 2 +- .../src/Tracing/TraceData/SummaryDiagnostics.cs | 5 +---- 4 files changed, 4 insertions(+), 15 deletions(-) diff --git a/Microsoft.Azure.Cosmos/src/Diagnostics/CosmosTraceDiagnostics.cs b/Microsoft.Azure.Cosmos/src/Diagnostics/CosmosTraceDiagnostics.cs index 568a9a9c68..5eae8492ea 100644 --- a/Microsoft.Azure.Cosmos/src/Diagnostics/CosmosTraceDiagnostics.cs +++ b/Microsoft.Azure.Cosmos/src/Diagnostics/CosmosTraceDiagnostics.cs @@ -43,7 +43,6 @@ public override string ToString() if (this.Value is Tracing.Trace rootConcreteTrace) { rootConcreteTrace.SetWalkingStateRecursively(); - return this.ToJsonString(); } return this.ToJsonString(); diff --git a/Microsoft.Azure.Cosmos/src/Query/Core/Metrics/ServerSideMetricsTraceExtractor.cs b/Microsoft.Azure.Cosmos/src/Query/Core/Metrics/ServerSideMetricsTraceExtractor.cs index b288d0e864..398f886097 100644 --- a/Microsoft.Azure.Cosmos/src/Query/Core/Metrics/ServerSideMetricsTraceExtractor.cs +++ b/Microsoft.Azure.Cosmos/src/Query/Core/Metrics/ServerSideMetricsTraceExtractor.cs @@ -5,7 +5,6 @@ namespace Microsoft.Azure.Cosmos.Query.Core.Metrics { using System; - using System.Linq; using Microsoft.Azure.Cosmos.Tracing; using Microsoft.Azure.Cosmos.Tracing.TraceData; @@ -19,10 +18,7 @@ public static void WalkTraceTreeForQueryMetrics(ITrace currentTrace, ServerSideM } // Assert that walking state is set - if (currentTrace is Tracing.Trace concreteTrace) - { - System.Diagnostics.Debug.Assert(concreteTrace.IsBeingWalked, "SetWalkingStateRecursively should be set to true"); - } + System.Diagnostics.Debug.Assert(currentTrace.IsBeingWalked, "SetWalkingStateRecursively should be set to true"); foreach (object datum in currentTrace.Data.Values) { @@ -49,10 +45,7 @@ private static void WalkTraceTreeForPartitionInfo(ITrace currentTrace, ServerSid } // Assert that walking state is set - if (currentTrace is Tracing.Trace concreteTrace) - { - System.Diagnostics.Debug.Assert(concreteTrace.IsBeingWalked, "SetWalkingStateRecursively should be set to true"); - } + System.Diagnostics.Debug.Assert(currentTrace.IsBeingWalked, "SetWalkingStateRecursively should be set to true"); foreach (Object datum in currentTrace.Data.Values) { diff --git a/Microsoft.Azure.Cosmos/src/Tracing/NoOpTrace.cs b/Microsoft.Azure.Cosmos/src/Tracing/NoOpTrace.cs index aeb01392ed..e11ad574f2 100644 --- a/Microsoft.Azure.Cosmos/src/Tracing/NoOpTrace.cs +++ b/Microsoft.Azure.Cosmos/src/Tracing/NoOpTrace.cs @@ -40,7 +40,7 @@ private NoOpTrace() public IReadOnlyDictionary Data => NoOpData; - public bool IsBeingWalked => false; + public bool IsBeingWalked => true; public void Dispose() { diff --git a/Microsoft.Azure.Cosmos/src/Tracing/TraceData/SummaryDiagnostics.cs b/Microsoft.Azure.Cosmos/src/Tracing/TraceData/SummaryDiagnostics.cs index 2157596e97..6ba485eeda 100644 --- a/Microsoft.Azure.Cosmos/src/Tracing/TraceData/SummaryDiagnostics.cs +++ b/Microsoft.Azure.Cosmos/src/Tracing/TraceData/SummaryDiagnostics.cs @@ -39,10 +39,7 @@ public SummaryDiagnostics(ITrace trace) private void CollectSummaryFromTraceTree(ITrace currentTrace) { // Assert that walking state is set - if (currentTrace is Trace concreteTrace) - { - System.Diagnostics.Debug.Assert(concreteTrace.IsBeingWalked, "SetWalkingStateRecursively should be set to true"); - } + System.Diagnostics.Debug.Assert(currentTrace.IsBeingWalked, "SetWalkingStateRecursively should be set to true"); foreach (object datums in currentTrace.Data.Values) { From e00ed260fa0b50badd3542a83fa2e2027127c92e Mon Sep 17 00:00:00 2001 From: Fabian Meiswinkel Date: Mon, 18 Aug 2025 18:53:33 +0000 Subject: [PATCH 12/31] Changing the logic to not ignore modifications when materialization has started - but instead just operate on cloned snapshots --- Microsoft.Azure.Cosmos/src/Tracing/Trace.cs | 75 +++++++++++-------- .../Tracing/TraceTests.cs | 48 +++++++++++- 2 files changed, 92 insertions(+), 31 deletions(-) diff --git a/Microsoft.Azure.Cosmos/src/Tracing/Trace.cs b/Microsoft.Azure.Cosmos/src/Tracing/Trace.cs index 46cac3e798..a1a2d26329 100644 --- a/Microsoft.Azure.Cosmos/src/Tracing/Trace.cs +++ b/Microsoft.Azure.Cosmos/src/Tracing/Trace.cs @@ -6,17 +6,13 @@ namespace Microsoft.Azure.Cosmos.Tracing { using System; using System.Collections.Generic; - using System.Diagnostics; - using System.Linq; - using System.Runtime.CompilerServices; - using Microsoft.Azure.Cosmos.Tracing.TraceData; using Microsoft.Azure.Documents; internal sealed class Trace : ITrace { private static readonly IReadOnlyDictionary EmptyDictionary = new Dictionary(); - private readonly List children; - private readonly Lazy> data; + private volatile List children; + private volatile Dictionary data; private ValueStopwatch stopwatch; private volatile bool isBeingWalked; @@ -35,7 +31,7 @@ private Trace( this.Component = component; this.Parent = parent; this.children = new List(); - this.data = new Lazy>(); + this.data = null; this.Summary = summary ?? throw new ArgumentNullException(nameof(summary)); } @@ -57,7 +53,7 @@ private Trace( public IReadOnlyList Children => this.children; - public IReadOnlyDictionary Data => this.data.IsValueCreated ? this.data.Value : Trace.EmptyDictionary; + public IReadOnlyDictionary Data => this.data ?? Trace.EmptyDictionary; public bool IsBeingWalked => this.isBeingWalked; @@ -85,11 +81,6 @@ public ITrace StartChild( TraceComponent component, TraceLevel level) { - if (this.isBeingWalked) - { - return NoOpTrace.Singleton; - } - if (this.Parent != null && !this.stopwatch.IsRunning) { return this.Parent.StartChild(name, component, level); @@ -103,19 +94,25 @@ public ITrace StartChild( summary: this.Summary); this.AddChild(child); + return child; } public void AddChild(ITrace child) { - lock (this.children) + lock (this.Name) { - if (this.isBeingWalked) + if (!this.isBeingWalked) { - return; // Ignore modifications while being walked + this.children.Add(child); + + return; } - this.children.Add(child); + List writableSnapshot = new List(this.children.Count + 1); + writableSnapshot.AddRange(this.children); + writableSnapshot.Add(child); + this.children = writableSnapshot; } } @@ -142,47 +139,65 @@ public static Trace GetRootTrace( public void AddDatum(string key, TraceDatum traceDatum) { - lock (this.children) + lock (this.Name) { - if (this.isBeingWalked) + this.data ??= new Dictionary(); + + if (!this.isBeingWalked) { - return; // Ignore modifications while being walked + // If materialization has not started yet no cloning is needed + this.data.Add(key, traceDatum); + return; } - this.data.Value.Add(key, traceDatum); - this.Summary.UpdateRegionContacted(traceDatum); + this.data = new Dictionary(this.data) + { + { key, traceDatum } + }; } } public void AddDatum(string key, object value) { - lock (this.children) + lock (this.Name) { - if (this.isBeingWalked) + this.data ??= new Dictionary(); + + if (!this.isBeingWalked) { - return; // Ignore modifications while being walked + // If materialization has not started yet no cloning is needed + this.data.Add(key, value); + return; } - this.data.Value.Add(key, value); + this.data = new Dictionary(this.data) + { + { key, value } + }; } } public void AddOrUpdateDatum(string key, object value) { - lock (this.children) + lock (this.Name) { - if (this.isBeingWalked) + if (!this.isBeingWalked) { + // If materialization has not started yet no cloning is needed + this.data[key] = value; return; // Ignore modifications while being walked } - this.data.Value[key] = value; + this.data = new Dictionary(this.data) + { + [key] = value + }; } } internal void SetWalkingStateRecursively() { - lock (this.children) + lock (this.Name) { if (this.isBeingWalked) { diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Tracing/TraceTests.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Tracing/TraceTests.cs index 07472a3a44..1ba6e67253 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Tracing/TraceTests.cs +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Tracing/TraceTests.cs @@ -4,8 +4,8 @@ using System.Collections.Generic; using System.Linq; using System.Reflection; + using System.Threading.Tasks; using Microsoft.Azure.Cosmos.Diagnostics; - using Microsoft.Azure.Cosmos.Json; using Microsoft.Azure.Cosmos.Tracing; using Microsoft.Azure.Cosmos.Tracing.TraceData; using Microsoft.Azure.Documents; @@ -134,5 +134,51 @@ public void ValidateStoreResultSerialization() Assert.AreEqual(0, storeResultProperties.Count, $"Json is missing properties: {string.Join(';', storeResultProperties)}"); } + + [TestMethod] + public void TestAddOrUpdateDatumThreadSafety() + { + Trace trace = Trace.GetRootTrace("ThreadSafetyTest"); + + // Create multiple threads to access the dictionary concurrently + const int numThreads = 10; + const int operationsPerThread = 100; + + // Use a list to keep track of the tasks + List tasks = new List(); + + for (int i = 0; i < numThreads; i++) + { + int threadIndex = i; + tasks.Add(Task.Run(() => + { + for (int j = 0; j < operationsPerThread; j++) + { + string key = $"key_{threadIndex}_{j}"; + object value = j; + + // Perform operations that would previously cause thread safety issues + trace.AddOrUpdateDatum(key, value); + + // Also test AddDatum + try + { + trace.AddDatum($"add_{threadIndex}_{j}", value); + } + catch (ArgumentException) + { + // Ignore key already exists exceptions which may occur + // when threads try to add the same key + } + } + })); + } + + // Wait for all tasks to complete + Task.WaitAll(tasks.ToArray()); + + // Verify the data dictionary has entries + Assert.IsTrue(trace.Data.Count > 0); + } } } \ No newline at end of file From eba2640b8043db2d0d80671b0558a0d6f973dbc7 Mon Sep 17 00:00:00 2001 From: Fabian Meiswinkel Date: Mon, 18 Aug 2025 19:26:17 +0000 Subject: [PATCH 13/31] Update Trace.cs --- Microsoft.Azure.Cosmos/src/Tracing/Trace.cs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/Microsoft.Azure.Cosmos/src/Tracing/Trace.cs b/Microsoft.Azure.Cosmos/src/Tracing/Trace.cs index a1a2d26329..693e083f69 100644 --- a/Microsoft.Azure.Cosmos/src/Tracing/Trace.cs +++ b/Microsoft.Azure.Cosmos/src/Tracing/Trace.cs @@ -51,8 +51,12 @@ private Trace( public ITrace Parent { get; } + // NOTE: no lock necessary here only because every reference to any this.children instance is immutable when isBeingWalked == true + // and isBeingWalked is guaranteed to be set to true before this Property is called public IReadOnlyList Children => this.children; + // NOTE: no lock necessary here only because every reference to any this.data instance is immutable when isBeingWalked == true + // and isBeingWalked is guaranteed to be set to true before this Property is called public IReadOnlyDictionary Data => this.data ?? Trace.EmptyDictionary; public bool IsBeingWalked => this.isBeingWalked; @@ -65,11 +69,6 @@ public void Dispose() public ITrace StartChild( string name) { - if (this.isBeingWalked) - { - return NoOpTrace.Singleton; - } - return this.StartChild( name, level: TraceLevel.Verbose, From c5b66cca81c35b0787208aa89c371df7fd4b607e Mon Sep 17 00:00:00 2001 From: Fabian Meiswinkel Date: Mon, 18 Aug 2025 19:39:01 +0000 Subject: [PATCH 14/31] Update SummaryDiagnostics.cs --- .../src/Tracing/TraceData/SummaryDiagnostics.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Microsoft.Azure.Cosmos/src/Tracing/TraceData/SummaryDiagnostics.cs b/Microsoft.Azure.Cosmos/src/Tracing/TraceData/SummaryDiagnostics.cs index 6ba485eeda..8de9f00992 100644 --- a/Microsoft.Azure.Cosmos/src/Tracing/TraceData/SummaryDiagnostics.cs +++ b/Microsoft.Azure.Cosmos/src/Tracing/TraceData/SummaryDiagnostics.cs @@ -22,7 +22,11 @@ public SummaryDiagnostics(ITrace trace) = new Lazy>(() => new Dictionary<(int, int), int>()); this.AllRegionsContacted = new Lazy>(() => new HashSet()); - + + if (trace is Tracing.Trace rootConcreteTrace) + { + rootConcreteTrace.SetWalkingStateRecursively(); + } this.CollectSummaryFromTraceTree(trace); } From ab1bff220ebac4c4b3f2d0ec926d1c7844c2d495 Mon Sep 17 00:00:00 2001 From: Fabian Meiswinkel Date: Mon, 18 Aug 2025 19:50:46 +0000 Subject: [PATCH 15/31] Update Trace.cs --- Microsoft.Azure.Cosmos/src/Tracing/Trace.cs | 18 ++---------------- 1 file changed, 2 insertions(+), 16 deletions(-) diff --git a/Microsoft.Azure.Cosmos/src/Tracing/Trace.cs b/Microsoft.Azure.Cosmos/src/Tracing/Trace.cs index 693e083f69..2d3c10afeb 100644 --- a/Microsoft.Azure.Cosmos/src/Tracing/Trace.cs +++ b/Microsoft.Azure.Cosmos/src/Tracing/Trace.cs @@ -138,22 +138,8 @@ public static Trace GetRootTrace( public void AddDatum(string key, TraceDatum traceDatum) { - lock (this.Name) - { - this.data ??= new Dictionary(); - - if (!this.isBeingWalked) - { - // If materialization has not started yet no cloning is needed - this.data.Add(key, traceDatum); - return; - } - - this.data = new Dictionary(this.data) - { - { key, traceDatum } - }; - } + this.Summary.UpdateRegionContacted(traceDatum); + this.AddDatum(key, traceDatum as Object); } public void AddDatum(string key, object value) From 563dd44eaf1cd439f5d2f82f6d645577849fdf2d Mon Sep 17 00:00:00 2001 From: Fabian Meiswinkel Date: Mon, 18 Aug 2025 19:54:42 +0000 Subject: [PATCH 16/31] Update Trace.cs --- Microsoft.Azure.Cosmos/src/Tracing/Trace.cs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/Microsoft.Azure.Cosmos/src/Tracing/Trace.cs b/Microsoft.Azure.Cosmos/src/Tracing/Trace.cs index 2d3c10afeb..e985aee6d1 100644 --- a/Microsoft.Azure.Cosmos/src/Tracing/Trace.cs +++ b/Microsoft.Azure.Cosmos/src/Tracing/Trace.cs @@ -182,6 +182,11 @@ public void AddOrUpdateDatum(string key, object value) internal void SetWalkingStateRecursively() { + if (this.isBeingWalked) + { + return; // Already set, return early + } + lock (this.Name) { if (this.isBeingWalked) @@ -196,10 +201,10 @@ internal void SetWalkingStateRecursively() concreteChild.SetWalkingStateRecursively(); } } - } - // Set the walking state for this trace after processing children - this.isBeingWalked = true; + // Set the walking state for this trace after processing children + this.isBeingWalked = true; + } } } } From b2219110e4996f8fac01d8eb1fef34d6ba3f1ff8 Mon Sep 17 00:00:00 2001 From: Fabian Meiswinkel Date: Mon, 18 Aug 2025 20:01:21 +0000 Subject: [PATCH 17/31] Adding Assert on IsBeingWalked --- Microsoft.Azure.Cosmos/src/Tracing/Trace.cs | 35 +++++++++++++++---- .../Tracing/TraceData/SummaryDiagnostics.cs | 3 +- 2 files changed, 31 insertions(+), 7 deletions(-) diff --git a/Microsoft.Azure.Cosmos/src/Tracing/Trace.cs b/Microsoft.Azure.Cosmos/src/Tracing/Trace.cs index e985aee6d1..82d7357cd0 100644 --- a/Microsoft.Azure.Cosmos/src/Tracing/Trace.cs +++ b/Microsoft.Azure.Cosmos/src/Tracing/Trace.cs @@ -6,6 +6,7 @@ namespace Microsoft.Azure.Cosmos.Tracing { using System; using System.Collections.Generic; + using System.Diagnostics; using Microsoft.Azure.Documents; internal sealed class Trace : ITrace @@ -51,13 +52,35 @@ private Trace( public ITrace Parent { get; } - // NOTE: no lock necessary here only because every reference to any this.children instance is immutable when isBeingWalked == true - // and isBeingWalked is guaranteed to be set to true before this Property is called - public IReadOnlyList Children => this.children; + // NOTE: no lock necessary here only because this.children is volatile + // and every reference to it is immutable when isBeingWalked == true + // and isBeingWalked is guaranteed to be set to true before this + // Property is called + public IReadOnlyList Children + { + get + { + // Assert that walking state is set + Debug.Assert(this.isBeingWalked, "SetWalkingStateRecursively should be set to true"); + + return this.children; + } + } - // NOTE: no lock necessary here only because every reference to any this.data instance is immutable when isBeingWalked == true - // and isBeingWalked is guaranteed to be set to true before this Property is called - public IReadOnlyDictionary Data => this.data ?? Trace.EmptyDictionary; + // NOTE: no lock necessary here only because this.data is volatile + // and every reference to it is immutable when isBeingWalked == true + // and isBeingWalked is guaranteed to be set to true before this + // Property is called + public IReadOnlyDictionary Data + { + get + { + // Assert that walking state is set + Debug.Assert(this.isBeingWalked, "SetWalkingStateRecursively should be set to true"); + + return this.data ?? Trace.EmptyDictionary; + } + } public bool IsBeingWalked => this.isBeingWalked; diff --git a/Microsoft.Azure.Cosmos/src/Tracing/TraceData/SummaryDiagnostics.cs b/Microsoft.Azure.Cosmos/src/Tracing/TraceData/SummaryDiagnostics.cs index 8de9f00992..f1f708c1b7 100644 --- a/Microsoft.Azure.Cosmos/src/Tracing/TraceData/SummaryDiagnostics.cs +++ b/Microsoft.Azure.Cosmos/src/Tracing/TraceData/SummaryDiagnostics.cs @@ -6,6 +6,7 @@ namespace Microsoft.Azure.Cosmos.Tracing.TraceData { using System; using System.Collections.Generic; + using System.Diagnostics; using System.Globalization; using System.Linq; using Microsoft.Azure.Cosmos.Json; @@ -43,7 +44,7 @@ public SummaryDiagnostics(ITrace trace) private void CollectSummaryFromTraceTree(ITrace currentTrace) { // Assert that walking state is set - System.Diagnostics.Debug.Assert(currentTrace.IsBeingWalked, "SetWalkingStateRecursively should be set to true"); + Debug.Assert(currentTrace.IsBeingWalked, "SetWalkingStateRecursively should be set to true"); foreach (object datums in currentTrace.Data.Values) { From 6820d2ebb4d81de78b9ac553be82761c38747ba6 Mon Sep 17 00:00:00 2001 From: Fabian Meiswinkel Date: Mon, 18 Aug 2025 20:51:01 +0000 Subject: [PATCH 18/31] Fixing CorrelatedActivityId Tracing --- .../src/Pagination/ParallelPrefetch.Testing.cs | 5 +++++ .../src/Query/v3Query/QueryIterator.cs | 2 +- Microsoft.Azure.Cosmos/src/Tracing/ITrace.cs | 7 +++++++ Microsoft.Azure.Cosmos/src/Tracing/NoOpTrace.cs | 6 ++++++ Microsoft.Azure.Cosmos/src/Tracing/Trace.cs | 14 ++++++++++++++ .../Tracing/EndToEndTraceWriterBaselineTests.cs | 5 +++++ .../Tracing/TraceJoiner.cs | 5 +++++ .../Pagination/ParallelPrefetchTests.cs | 6 ++++++ .../Tracing/TraceWriterBaselineTests.cs | 5 +++++ 9 files changed, 54 insertions(+), 1 deletion(-) diff --git a/Microsoft.Azure.Cosmos/src/Pagination/ParallelPrefetch.Testing.cs b/Microsoft.Azure.Cosmos/src/Pagination/ParallelPrefetch.Testing.cs index 2e4cb8b601..c60455a711 100644 --- a/Microsoft.Azure.Cosmos/src/Pagination/ParallelPrefetch.Testing.cs +++ b/Microsoft.Azure.Cosmos/src/Pagination/ParallelPrefetch.Testing.cs @@ -118,6 +118,11 @@ void IDisposable.Dispose() { this.innerTrace.Dispose(); } + + bool ITrace.TryGetDatum(string key, out object datum) + { + return this.innerTrace.TryGetDatum(key, out datum); + } } } } diff --git a/Microsoft.Azure.Cosmos/src/Query/v3Query/QueryIterator.cs b/Microsoft.Azure.Cosmos/src/Query/v3Query/QueryIterator.cs index 5891066b42..cc09cd3215 100644 --- a/Microsoft.Azure.Cosmos/src/Query/v3Query/QueryIterator.cs +++ b/Microsoft.Azure.Cosmos/src/Query/v3Query/QueryIterator.cs @@ -180,7 +180,7 @@ public override async Task ReadNextAsync(ITrace trace, Cancella // If Correlated Id already exists and is different, add a new one in comma separated list // Scenario: A new iterator is created with same ContinuationToken and Trace - if (trace.Data.TryGetValue(QueryIterator.CorrelatedActivityIdKeyName, out object correlatedActivityIds)) + if (trace.TryGetDatum(QueryIterator.CorrelatedActivityIdKeyName, out object correlatedActivityIds)) { List correlatedIdList = correlatedActivityIds.ToString().Split(',').ToList(); if (!correlatedIdList.Contains(this.correlatedActivityId.ToString())) diff --git a/Microsoft.Azure.Cosmos/src/Tracing/ITrace.cs b/Microsoft.Azure.Cosmos/src/Tracing/ITrace.cs index 22c2c75821..4966aa716b 100644 --- a/Microsoft.Azure.Cosmos/src/Tracing/ITrace.cs +++ b/Microsoft.Azure.Cosmos/src/Tracing/ITrace.cs @@ -120,5 +120,12 @@ ITrace StartChild( /// Existing trace. void AddChild(ITrace trace); + /// + /// Tries to get a specific datum - it is safe to call this even before IsBeingWalked is set. + /// + /// The key to identify the datum. + /// The datum itself. + /// A flag indicating whether the datum with this key exists. + bool TryGetDatum(string key, out object datum); } } diff --git a/Microsoft.Azure.Cosmos/src/Tracing/NoOpTrace.cs b/Microsoft.Azure.Cosmos/src/Tracing/NoOpTrace.cs index e11ad574f2..172f8f16e6 100644 --- a/Microsoft.Azure.Cosmos/src/Tracing/NoOpTrace.cs +++ b/Microsoft.Azure.Cosmos/src/Tracing/NoOpTrace.cs @@ -88,5 +88,11 @@ public void UpdateRegionContacted(TraceDatum traceDatum) { // NoOp } + + bool ITrace.TryGetDatum(string key, out object datum) + { + datum = null; + return false; + } } } diff --git a/Microsoft.Azure.Cosmos/src/Tracing/Trace.cs b/Microsoft.Azure.Cosmos/src/Tracing/Trace.cs index 82d7357cd0..0efc12550d 100644 --- a/Microsoft.Azure.Cosmos/src/Tracing/Trace.cs +++ b/Microsoft.Azure.Cosmos/src/Tracing/Trace.cs @@ -229,5 +229,19 @@ internal void SetWalkingStateRecursively() this.isBeingWalked = true; } } + + bool ITrace.TryGetDatum(string key, out object datum) + { + lock (this.Name) + { + if (this.data == null) + { + datum = null; + return false; + } + + return this.data.TryGetValue(key, out datum); + } + } } } diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/Tracing/EndToEndTraceWriterBaselineTests.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/Tracing/EndToEndTraceWriterBaselineTests.cs index 40bb10ea24..dd8b867c2c 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/Tracing/EndToEndTraceWriterBaselineTests.cs +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/Tracing/EndToEndTraceWriterBaselineTests.cs @@ -1926,6 +1926,11 @@ public void AddOrUpdateDatum(string key, object value) this.data[key] = "Redacted To Not Change The Baselines From Run To Run"; } + + bool ITrace.TryGetDatum(string key, out object datum) + { + return this.data.TryGetValue(key, out datum); + } } private sealed class RequestHandlerSleepHelper : RequestHandler diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/Tracing/TraceJoiner.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/Tracing/TraceJoiner.cs index 6f2f3b7914..0fdd38b071 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/Tracing/TraceJoiner.cs +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/Tracing/TraceJoiner.cs @@ -109,6 +109,11 @@ public void AddOrUpdateDatum(string key, object value) { this.data[key] = value; } + + bool ITrace.TryGetDatum(string key, out object datum) + { + return this.data.TryGetValue(key, out datum); + } } } } diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Pagination/ParallelPrefetchTests.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Pagination/ParallelPrefetchTests.cs index 4e674a76a9..df4a071e8d 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Pagination/ParallelPrefetchTests.cs +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Pagination/ParallelPrefetchTests.cs @@ -511,6 +511,12 @@ public ITrace StartChild(string name, TraceComponent component, Cosmos.Tracing.T return child; } + + bool ITrace.TryGetDatum(string key, out object datum) + { + datum = null; + return false; + } } /// diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Tracing/TraceWriterBaselineTests.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Tracing/TraceWriterBaselineTests.cs index 666b00702f..2f5469e644 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Tracing/TraceWriterBaselineTests.cs +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Tracing/TraceWriterBaselineTests.cs @@ -953,6 +953,11 @@ public void UpdateRegionContacted(TraceDatum _) public void AddOrUpdateDatum(string key, object value) { this.data[key] = value; + } + + bool ITrace.TryGetDatum(string key, out object datum) + { + return this.data.TryGetValue(key, out datum); } } } From 130a8c3bfbe09f8423eec90da4529daed7cc786b Mon Sep 17 00:00:00 2001 From: Fabian Meiswinkel Date: Mon, 18 Aug 2025 22:18:40 +0000 Subject: [PATCH 19/31] Update Trace.cs --- Microsoft.Azure.Cosmos/src/Tracing/Trace.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Microsoft.Azure.Cosmos/src/Tracing/Trace.cs b/Microsoft.Azure.Cosmos/src/Tracing/Trace.cs index 0efc12550d..f5727c28f2 100644 --- a/Microsoft.Azure.Cosmos/src/Tracing/Trace.cs +++ b/Microsoft.Azure.Cosmos/src/Tracing/Trace.cs @@ -189,6 +189,8 @@ public void AddOrUpdateDatum(string key, object value) { lock (this.Name) { + this.data ??= new Dictionary(); + if (!this.isBeingWalked) { // If materialization has not started yet no cloning is needed From 7b73bbec234aa084ee41b3049586699bca44d018 Mon Sep 17 00:00:00 2001 From: Fabian Meiswinkel Date: Tue, 19 Aug 2025 12:18:01 +0000 Subject: [PATCH 20/31] Update BatchAsyncOperationContextTests.cs --- .../Batch/BatchAsyncOperationContextTests.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Batch/BatchAsyncOperationContextTests.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Batch/BatchAsyncOperationContextTests.cs index b686449c42..121b5c0a4a 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Batch/BatchAsyncOperationContextTests.cs +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Batch/BatchAsyncOperationContextTests.cs @@ -52,6 +52,11 @@ public async Task TraceIsJoinedOnCompletionWithRetry() batchAsyncOperationContext.Complete(null, result); + if (result.Trace is Trace rootLevelTrace) + { + rootLevelTrace.SetWalkingStateRecursively(); + } + Assert.AreEqual(result, await batchAsyncOperationContext.OperationTask); Assert.AreEqual(2, result.Trace.Children.Count, "The final trace should have the initial trace, plus the retries, plus the final trace"); Assert.AreEqual(rootTrace, result.Trace, "The first trace child should be the initial root"); From 26a05bb98f45a8500085ba79d5459e03c43bd30e Mon Sep 17 00:00:00 2001 From: Fabian Meiswinkel Date: Tue, 19 Aug 2025 12:59:48 +0000 Subject: [PATCH 21/31] Update BatchAsyncOperationContextTests.cs --- .../Batch/BatchAsyncOperationContextTests.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Batch/BatchAsyncOperationContextTests.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Batch/BatchAsyncOperationContextTests.cs index 121b5c0a4a..a5bd3b374e 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Batch/BatchAsyncOperationContextTests.cs +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Batch/BatchAsyncOperationContextTests.cs @@ -98,6 +98,11 @@ public async Task TraceIsJoinedOnCompletionWithoutRetry() batchAsyncOperationContext.Complete(null, result); + if (result.Trace is Trace rootLevelTrace) + { + rootLevelTrace.SetWalkingStateRecursively(); + } + Assert.AreEqual(result, await batchAsyncOperationContext.OperationTask); Assert.AreEqual(1, result.Trace.Children.Count, "The final trace should have the initial trace, plus the final trace, since the result is not retried, it should not capture it"); Assert.AreEqual(rootTrace, result.Trace, "The first trace child should be the initial root"); From 4a7eeb4f4a0acad2d6ef923ae38de4b901c98fae Mon Sep 17 00:00:00 2001 From: Fabian Meiswinkel Date: Tue, 19 Aug 2025 14:31:19 +0000 Subject: [PATCH 22/31] Update PartitionKeyBatchResponseTests.cs --- .../Batch/PartitionKeyBatchResponseTests.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Batch/PartitionKeyBatchResponseTests.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Batch/PartitionKeyBatchResponseTests.cs index a396112dac..b7d832a8f9 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Batch/PartitionKeyBatchResponseTests.cs +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Batch/PartitionKeyBatchResponseTests.cs @@ -125,6 +125,11 @@ public async Task DiagnosticsAreSetThroughResponseAsync() throw new Exception(); } + if (cosmosTraceDiagnostics.Value is Trace rootLevelTrace) + { + rootLevelTrace.SetWalkingStateRecursively(); + } + Assert.AreEqual(diagnostics, cosmosTraceDiagnostics.Value.Data.Values.First()); } } From b9cd3d6ff65e906939e2f0dd9633de3148610aa1 Mon Sep 17 00:00:00 2001 From: Fabian Meiswinkel Date: Tue, 19 Aug 2025 15:06:47 +0000 Subject: [PATCH 23/31] Update CosmosDiagnosticsUnitTests.cs --- .../CosmosDiagnosticsUnitTests.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/CosmosDiagnosticsUnitTests.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/CosmosDiagnosticsUnitTests.cs index a59d6057ae..5dcee22a47 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/CosmosDiagnosticsUnitTests.cs +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/CosmosDiagnosticsUnitTests.cs @@ -69,6 +69,11 @@ public void ValidateTransportHandlerLogging() Assert.AreEqual(HttpStatusCode.Gone, response.StatusCode); Assert.AreEqual(SubStatusCodes.PartitionKeyRangeGone, response.Headers.SubStatusCode); + if (trace is Cosmos.Tracing.Trace rootLevelTrace) + { + rootLevelTrace.SetWalkingStateRecursively(); + } + IEnumerable pointOperationStatistics = trace.Data.Values .Where(traceDatum => traceDatum is PointOperationStatisticsTraceDatum operationStatistics) .Select(x => (PointOperationStatisticsTraceDatum)x); From c8125d3b4342639c2d044976f6a05a3ca3708e26 Mon Sep 17 00:00:00 2001 From: Fabian Meiswinkel Date: Tue, 19 Aug 2025 16:28:38 +0000 Subject: [PATCH 24/31] Update Trace.cs --- Microsoft.Azure.Cosmos/src/Tracing/Trace.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Microsoft.Azure.Cosmos/src/Tracing/Trace.cs b/Microsoft.Azure.Cosmos/src/Tracing/Trace.cs index f5727c28f2..53fc03e23f 100644 --- a/Microsoft.Azure.Cosmos/src/Tracing/Trace.cs +++ b/Microsoft.Azure.Cosmos/src/Tracing/Trace.cs @@ -131,6 +131,11 @@ public void AddChild(ITrace child) return; } + if (child is Trace traceChild) + { + traceChild.SetWalkingStateRecursively(); + } + List writableSnapshot = new List(this.children.Count + 1); writableSnapshot.AddRange(this.children); writableSnapshot.Add(child); From 269aeb4fc77b4a35134d137fcf0ca92e45727627 Mon Sep 17 00:00:00 2001 From: Fabian Meiswinkel Date: Tue, 19 Aug 2025 17:27:18 +0000 Subject: [PATCH 25/31] Update RegionFailoverTests.cs --- .../RegionFailoverTests.cs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/PartitionKeyRangeFailoverTests/RegionFailoverTests.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/PartitionKeyRangeFailoverTests/RegionFailoverTests.cs index 05f82bec36..080f308392 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/PartitionKeyRangeFailoverTests/RegionFailoverTests.cs +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/PartitionKeyRangeFailoverTests/RegionFailoverTests.cs @@ -16,6 +16,7 @@ namespace Microsoft.Azure.Cosmos.Tests using System.Threading.Tasks; using Microsoft.Azure.Cosmos.Diagnostics; using Microsoft.Azure.Cosmos.Routing; + using Microsoft.Azure.Cosmos.Tracing; using Microsoft.Azure.Documents; using Microsoft.VisualStudio.TestTools.UnitTesting; using Moq; @@ -344,6 +345,11 @@ public async Task ReadItemAsync_WithThinClientEnabledAndServiceUnavailableReceiv CosmosTraceDiagnostics traceDiagnostic = readResponse.Diagnostics as CosmosTraceDiagnostics; Assert.IsNotNull(traceDiagnostic); + if (traceDiagnostic.Value is Trace rootLevelTrace) + { + rootLevelTrace.SetWalkingStateRecursively(); + } + traceDiagnostic.Value.Data.TryGetValue("Hedge Context", out object hedgeContext); if (enablePartitionLevelFailover) @@ -638,6 +644,11 @@ public async Task CreateItemAsync_WithThinClientEnabledAndServiceUnavailableRece CosmosTraceDiagnostics traceDiagnostic = createItemResponse.Diagnostics as CosmosTraceDiagnostics; Assert.IsNotNull(traceDiagnostic); + if (traceDiagnostic.Value is Trace rootLevelTrace) + { + rootLevelTrace.SetWalkingStateRecursively(); + } + traceDiagnostic.Value.Data.TryGetValue("Hedge Context", out object hedgeContext); Assert.IsNull(hedgeContext); } From 07cce9c8d6d1e2a8cf5ba44642c2d176ffa2ad08 Mon Sep 17 00:00:00 2001 From: Fabian Meiswinkel Date: Tue, 19 Aug 2025 18:02:37 +0000 Subject: [PATCH 26/31] Update TraceTests.cs --- .../tests/Microsoft.Azure.Cosmos.Tests/Tracing/TraceTests.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Tracing/TraceTests.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Tracing/TraceTests.cs index 1ba6e67253..6333750da1 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Tracing/TraceTests.cs +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Tracing/TraceTests.cs @@ -23,6 +23,7 @@ public void TestRootTrace() using (rootTrace = Trace.GetRootTrace(name: "RootTrace")) { Assert.IsNotNull(rootTrace); + rootTrace.SetWalkingStateRecursively(); Assert.IsNotNull(rootTrace.Children); Assert.AreEqual(0, rootTrace.Children.Count); Assert.AreEqual(rootTrace.Component, TraceComponent.Unknown); @@ -49,6 +50,7 @@ public void TestAddChild() rootTrace.AddChild(twoChild); } + rootTrace.SetWalkingStateRecursively(); Assert.AreEqual(2, rootTrace.Children.Count); Assert.AreEqual(oneChild, rootTrace.Children[0]); Assert.AreEqual(twoChild, rootTrace.Children[1]); @@ -177,6 +179,8 @@ public void TestAddOrUpdateDatumThreadSafety() // Wait for all tasks to complete Task.WaitAll(tasks.ToArray()); + trace.SetWalkingStateRecursively(); + // Verify the data dictionary has entries Assert.IsTrue(trace.Data.Count > 0); } From 6d64bffa35d7f2e57f03cf5e481ebdfe2749c253 Mon Sep 17 00:00:00 2001 From: Fabian Meiswinkel Date: Tue, 19 Aug 2025 19:12:47 +0000 Subject: [PATCH 27/31] Update TraceTests.cs --- .../tests/Microsoft.Azure.Cosmos.Tests/Tracing/TraceTests.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Tracing/TraceTests.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Tracing/TraceTests.cs index 6333750da1..203561d638 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Tracing/TraceTests.cs +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Tracing/TraceTests.cs @@ -69,6 +69,7 @@ public void TestTraceChildren() { } + rootTrace.SetWalkingStateRecursively(); Assert.AreEqual(rootTrace.Children.Count, 2); Assert.AreEqual(rootTrace.Children[0].Component, TraceComponent.Query); Assert.AreEqual(rootTrace.Children[1].Component, TraceComponent.Transport); From 9109651b4ffdd2ffee99e5a1c9adffb4c01a4bf0 Mon Sep 17 00:00:00 2001 From: Fabian Meiswinkel Date: Tue, 19 Aug 2025 19:16:56 +0000 Subject: [PATCH 28/31] Update NoOpTrace.cs --- Microsoft.Azure.Cosmos/src/Tracing/NoOpTrace.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Microsoft.Azure.Cosmos/src/Tracing/NoOpTrace.cs b/Microsoft.Azure.Cosmos/src/Tracing/NoOpTrace.cs index 172f8f16e6..f14b45bd12 100644 --- a/Microsoft.Azure.Cosmos/src/Tracing/NoOpTrace.cs +++ b/Microsoft.Azure.Cosmos/src/Tracing/NoOpTrace.cs @@ -40,7 +40,7 @@ private NoOpTrace() public IReadOnlyDictionary Data => NoOpData; - public bool IsBeingWalked => true; + public bool IsBeingWalked => false; public void Dispose() { From 2c06779cd85b4e0e9f81da87bb40c9a222800307 Mon Sep 17 00:00:00 2001 From: Fabian Meiswinkel Date: Tue, 19 Aug 2025 19:38:11 +0000 Subject: [PATCH 29/31] Update NoOpTrace.cs --- Microsoft.Azure.Cosmos/src/Tracing/NoOpTrace.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Microsoft.Azure.Cosmos/src/Tracing/NoOpTrace.cs b/Microsoft.Azure.Cosmos/src/Tracing/NoOpTrace.cs index f14b45bd12..6077546f4a 100644 --- a/Microsoft.Azure.Cosmos/src/Tracing/NoOpTrace.cs +++ b/Microsoft.Azure.Cosmos/src/Tracing/NoOpTrace.cs @@ -40,7 +40,7 @@ private NoOpTrace() public IReadOnlyDictionary Data => NoOpData; - public bool IsBeingWalked => false; + public bool IsBeingWalked => true; // this needs to return true to allow materialization of NoOpTrace public void Dispose() { From 059a5ce26485c4059dff3d86c5bd6782d920c12f Mon Sep 17 00:00:00 2001 From: Fabian Meiswinkel Date: Tue, 19 Aug 2025 20:20:39 +0000 Subject: [PATCH 30/31] Fixing test issues --- .../Tracing/EndToEndTraceWriterBaselineTests.cs | 2 +- .../Microsoft.Azure.Cosmos.EmulatorTests/Tracing/TraceJoiner.cs | 2 +- .../Pagination/ParallelPrefetchTests.cs | 2 +- .../Tracing/TraceWriterBaselineTests.cs | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/Tracing/EndToEndTraceWriterBaselineTests.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/Tracing/EndToEndTraceWriterBaselineTests.cs index dd8b867c2c..db053e0570 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/Tracing/EndToEndTraceWriterBaselineTests.cs +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/Tracing/EndToEndTraceWriterBaselineTests.cs @@ -1865,7 +1865,7 @@ public TraceForBaselineTesting( public IReadOnlyDictionary Data => this.data; - public bool IsBeingWalked => false; + public bool IsBeingWalked => true; // needs to return true to allow materialization public IReadOnlyList<(string, Uri)> RegionsContacted => new List<(string, Uri)>(); diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/Tracing/TraceJoiner.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/Tracing/TraceJoiner.cs index 0fdd38b071..369c146f61 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/Tracing/TraceJoiner.cs +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/Tracing/TraceJoiner.cs @@ -65,7 +65,7 @@ public TraceForest(IReadOnlyList children) public IReadOnlyDictionary Data => this.data; - public bool IsBeingWalked => false; + public bool IsBeingWalked => true; // needs to return true to allow materialization public IReadOnlyList<(string, Uri)> RegionsContacted => new List<(string, Uri)>(); diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Pagination/ParallelPrefetchTests.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Pagination/ParallelPrefetchTests.cs index df4a071e8d..72147aea1c 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Pagination/ParallelPrefetchTests.cs +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Pagination/ParallelPrefetchTests.cs @@ -459,7 +459,7 @@ private sealed class SimpleTrace : ITrace public IReadOnlyDictionary Data { get; } = new Dictionary(); - public bool IsBeingWalked => false; + public bool IsBeingWalked => true; // needs to return true to allow materialization internal SimpleTrace(ITrace parent, string name, TraceComponent component, Cosmos.Tracing.TraceLevel level) { diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Tracing/TraceWriterBaselineTests.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Tracing/TraceWriterBaselineTests.cs index 2f5469e644..9b9ba1b29a 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Tracing/TraceWriterBaselineTests.cs +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Tracing/TraceWriterBaselineTests.cs @@ -905,7 +905,7 @@ public TraceForBaselineTesting( public IReadOnlyDictionary Data => this.data; - public bool IsBeingWalked => false; + public bool IsBeingWalked => true; // needs to return true to allow materialization public IReadOnlyList<(string, Uri)> RegionsContacted => new List<(string, Uri)>(); From d3050b0e5ffd35f75367766f4d8f199aac7eba71 Mon Sep 17 00:00:00 2001 From: Fabian Meiswinkel Date: Tue, 19 Aug 2025 20:50:36 +0000 Subject: [PATCH 31/31] Update FullPipelineTests.cs --- .../Query/Pipeline/FullPipelineTests.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Query/Pipeline/FullPipelineTests.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Query/Pipeline/FullPipelineTests.cs index 4ef83ab057..1a5400f941 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Query/Pipeline/FullPipelineTests.cs +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Query/Pipeline/FullPipelineTests.cs @@ -253,7 +253,8 @@ public async Task Tracing() numTraces++; } } - + + rootTrace.SetWalkingStateRecursively(); Assert.AreEqual(numTraces, rootTrace.Children.Count); }