CosmosDiagnosticsL Fixes InvalidOperationException in CosmosDiagnostics with permanent walking state management

[Copilot]

This PR fixes a race condition that causes InvalidOperationException: Collection was modified; enumeration operation may not execute. when calling CosmosDiagnostics.ToString() under concurrent access scenarios.

Problem

The issue occurs when multiple threads are simultaneously:

1. Adding children or data to trace objects (via background operations like CrossRegionalHedging)
2. Calling CosmosDiagnostics.ToString() which enumerates trace collections

This led to "Collection was modified" exceptions during enumeration, particularly when operations are cancelled and diagnostics are being read from CosmosOperationCanceledException.

Solution

The fix implements a permanent walking state management approach for thread-safe enumeration:

1. Centralized control: Walking state is managed from public API entry points (ToString(), GetContactedRegions(), GetQueryMetrics(), and IsGoneExceptionHit())

2. Permanent state: Once a trace tree starts being walked, the walking state is set permanently - no reset to false, simplifying the logic

3. Enhanced modification prevention: All modification methods (AddChild(), AddDatum(), AddOrUpdateDatum(), and both StartChild() overloads) check the walking state inside locks and ignore changes when enumeration is in progress

4. NoOpTrace returns: When trace is being walked, StartChild() methods return NoOpTrace.Singleton instead of creating new traces

5. Atomic operations: All modification checks are performed inside locks to prevent race conditions between checking walking state and performing modifications

6. Direct enumeration: Uses direct enumeration protected by the permanent walking state instead of snapshot-based approaches

Key Benefits

• ✅ Eliminates race conditions: Prevents concurrent modifications during enumeration operations
• ✅ Simple architecture: No complex try/finally blocks or state reset logic
• ✅ Comprehensive coverage: All modification and enumeration points protected
• ✅ Performance optimized: Direct enumeration instead of creating snapshots
• ✅ No breaking changes: Public APIs remain unchanged
• ✅ Clean separation: Walking state management separated from enumeration logic

Example

The following scenario would previously throw InvalidOperationException but now works correctly:

// Multiple threads adding data to trace
trace.AddDatum("key", "value");
trace.StartChild("operation");

// Concurrent thread calling ToString() - now safe
string diagnostics = cosmosDiagnostics.ToString(); // No longer throws

The permanent walking state approach ensures that once enumeration begins on a trace tree, all further modifications are safely ignored, providing comprehensive protection against concurrent modifications.

Fixes #5112.

---

💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

[FabianMeiswinkel]

This PR has better approach #5209

[github-actions]

Please follow the required format: "[Internal] Category: (Adds|Fixes|Refactors|Removes) Description"

Internal should be used for PRs that have no customer impact. This flag is used to help generate the changelog to know which PRs should be included. Examples:
Diagnostics: Adds GetElapsedClientLatency to CosmosDiagnostics
PartitionKey: Fixes null reference when using default(PartitionKey)
[v4] Client Encryption: Refactors code to external project
[Internal] Query: Adds code generator for CosmosNumbers for easy additions in the future.

[kirankumarkolli]

Update logic to not allow any changes to the Trace once WalkTrace started.

[kirankumarkolli]

@copilot look into the comments

[Copilot]

@copilot look into the comments

Addressed all reviewer feedback:

1. ✅ Moved Summary.UpdateRegionContacted() inside lock
2. ✅ Moved isBeingWalked assignment inside lock
3. ✅ Removed try/finally and false reset from ToString()
4. ✅ Updated SetWalkingStateRecursively() to take no arguments
5. ✅ Added early return if already set
6. ✅ Added SetWalkingStateRecursively() to GetContactedRegions, GetQueryMetrics, IsGoneExceptionHit
7. ✅ Reverted WalkTraceTreeForGoneException to original implementation
8. ✅ Reverted PopulateServerSideCumulativeMetrics to original implementation

The implementation now uses centralized walking state management with permanent state setting and direct enumeration protected by locks. Commit: 824d33d

[FabianMeiswinkel]

Decided to go with ConcurrentDictionary for now. - See #5209

[FabianMeiswinkel]

Closing this - opened #5361 instead