Skip to content

Commit 8ba09c7

Browse files
[Internal] Spec: Remove CosmosDiagnostics.Verbosity property per review feedback
Address Fabian's review comment: drop the mutable Verbosity property from CosmosDiagnostics to avoid thread-safety issues. Callers use the explicit ToString(DiagnosticsVerbosity) / ToJsonString(DiagnosticsVerbosity) overloads instead. Parameterless ToString() always returns Detailed (backward compat). Changes: - Remove Verbosity get/set property from CosmosDiagnostics API surface - Update CosmosClientOptions doc to clarify it is a config value, not auto-flowed - Simplify precedence order from 4 levels to 3 - Remove ResponseMessage.cs from modified files list (no verbosity propagation needed) - Update WI-1 and WI-3 scope/acceptance criteria - Replace ToString_UsesSummary_WhenVerbosityPropertySet test with ToString_Parameterless_AlwaysDetailed test Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 48e6848 commit 8ba09c7

File tree

1 file changed

+23
-26
lines changed

1 file changed

+23
-26
lines changed

specs/diagnostics-compaction.md

Lines changed: 23 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -64,8 +64,8 @@ public class CosmosClientOptions
6464
/// <summary>
6565
/// Gets or sets the default verbosity for CosmosDiagnostics serialization.
6666
/// Default: <see cref="DiagnosticsVerbosity.Detailed"/>.
67-
/// This value flows to <see cref="CosmosDiagnostics.Verbosity"/> on each response.
68-
/// Can be overridden per-call via <see cref="CosmosDiagnostics.ToString(DiagnosticsVerbosity)"/>.
67+
/// This is a client-level configuration value. Pass it to
68+
/// <see cref="CosmosDiagnostics.ToString(DiagnosticsVerbosity)"/> when serializing diagnostics.
6969
/// Can also be set via the AZURE_COSMOS_DIAGNOSTICS_VERBOSITY environment variable.
7070
/// </summary>
7171
public DiagnosticsVerbosity DiagnosticsVerbosity { get; set; } = DiagnosticsVerbosity.Detailed;
@@ -80,24 +80,21 @@ public class CosmosClientOptions
8080
}
8181
```
8282

83-
### 3.3 CosmosDiagnostics — Verbosity Property & Serialization Overloads
83+
### 3.3 CosmosDiagnostics — Serialization Overloads
84+
85+
The parameterless `ToString()` continues to return full detailed output (backward compatible). New overloads accept an explicit `DiagnosticsVerbosity` parameter — no mutable state is stored on the diagnostics object, avoiding thread-safety concerns.
8486

8587
```csharp
8688
// File: Microsoft.Azure.Cosmos/src/Diagnostics/CosmosDiagnostics.cs
8789
public abstract class CosmosDiagnostics
8890
{
89-
// ... existing members ...
90-
91-
/// <summary>
92-
/// Gets or sets the verbosity level used when serializing these diagnostics via ToString().
93-
/// This is set automatically from CosmosClientOptions but can be
94-
/// overridden by the caller before calling ToString(), or by using the
95-
/// ToString(DiagnosticsVerbosity) overload.
96-
/// </summary>
97-
public DiagnosticsVerbosity Verbosity { get; set; } = DiagnosticsVerbosity.Detailed;
91+
// ... existing members (ToString(), GetContactedRegions(), etc. unchanged) ...
9892
9993
/// <summary>
10094
/// Returns the string representation of diagnostics using the specified verbosity.
95+
/// When <paramref name="verbosity"/> is <see cref="DiagnosticsVerbosity.Summary"/>,
96+
/// produces a compacted region-grouped summary. When <see cref="DiagnosticsVerbosity.Detailed"/>,
97+
/// produces the full trace output (same as parameterless <see cref="ToString()"/>).
10198
/// </summary>
10299
/// <param name="verbosity">The verbosity level to use for serialization.</param>
103100
/// <returns>A JSON string with diagnostics at the requested verbosity level.</returns>
@@ -121,9 +118,10 @@ public abstract class CosmosDiagnostics
121118

122119
**Precedence order** (highest to lowest):
123120
1. Explicit `ToString(DiagnosticsVerbosity)` / `ToJsonString(DiagnosticsVerbosity)` parameter
124-
2. `CosmosDiagnostics.Verbosity` property (set from `CosmosClientOptions.DiagnosticsVerbosity` during response creation, or manually by caller)
125-
3. Environment variable `AZURE_COSMOS_DIAGNOSTICS_VERBOSITY`
126-
4. Default: `DiagnosticsVerbosity.Detailed`
121+
2. `CosmosClientOptions.DiagnosticsVerbosity` (set in code, or populated from `AZURE_COSMOS_DIAGNOSTICS_VERBOSITY` environment variable)
122+
3. Default: `DiagnosticsVerbosity.Detailed`
123+
124+
> **Note:** The parameterless `ToString()` always returns `Detailed` output for backward compatibility. To use `Summary` mode, callers must explicitly pass `DiagnosticsVerbosity.Summary` to the overload.
127125
128126
## 4. Summary Mode JSON Format
129127

@@ -273,12 +271,11 @@ Both `StoreResponseStatistics` (direct mode) and `HttpResponseStatistics` (gatew
273271
| File | Change |
274272
|------|--------|
275273
| `CosmosClientOptions.cs` | Add `DiagnosticsVerbosity` and `MaxDiagnosticsSummarySizeBytes` properties with validation |
276-
| `CosmosDiagnostics.cs` | Add `Verbosity` property, `ToString(DiagnosticsVerbosity)` and `ToJsonString(DiagnosticsVerbosity)` overloads |
277-
| `CosmosTraceDiagnostics.cs` | Wire `Verbosity` into `ToString()` / `ToJsonString()` to branch between detailed and summary serialization; implement new overloads |
274+
| `CosmosDiagnostics.cs` | Add `ToString(DiagnosticsVerbosity)` and `ToJsonString(DiagnosticsVerbosity)` abstract overloads |
275+
| `CosmosTraceDiagnostics.cs` | Implement `ToString(DiagnosticsVerbosity)` and `ToJsonString(DiagnosticsVerbosity)` overloads; delegate to `DiagnosticsSummaryWriter` when verbosity is `Summary` |
278276
| `TraceWriter.TraceJsonWriter.cs` | Add summary serialization path that delegates to `DiagnosticsSummaryWriter` when verbosity is `Summary` |
279277
| `SummaryDiagnostics.cs` | Extend `CollectSummaryFromTraceTree()` to support region-grouped collection with ordering |
280278
| `ClientSideRequestStatisticsTraceDatum.cs` | Ensure `StoreResponseStatistics` and `HttpResponseStatistics` lists are accessible for summary computation |
281-
| `ResponseMessage.cs` (or wherever diagnostics are attached to response) | Propagate effective verbosity from `CosmosClientOptions` to `CosmosDiagnostics.Verbosity` |
282279

283280
### 6.3 Contract/Baseline Updates
284281

@@ -289,16 +286,16 @@ Both `StoreResponseStatistics` (direct mode) and `HttpResponseStatistics` (gatew
289286
## 7. Work Items
290287

291288
### WI-1: DiagnosticsVerbosity Enum & Options Plumbing
292-
**Scope:** Create the enum, add properties to `CosmosClientOptions`, add `ToString(DiagnosticsVerbosity)` / `ToJsonString(DiagnosticsVerbosity)` overloads to `CosmosDiagnostics`, add environment variable support, wire verbosity from `CosmosClientOptions` through to `CosmosDiagnostics.Verbosity`.
293-
**Acceptance:** Verbosity flows from client options → diagnostics object. `ToString(verbosity)` overloads compile and delegate correctly. No behavioral change yet.
289+
**Scope:** Create the enum, add `DiagnosticsVerbosity` and `MaxDiagnosticsSummarySizeBytes` properties to `CosmosClientOptions`, add `ToString(DiagnosticsVerbosity)` / `ToJsonString(DiagnosticsVerbosity)` abstract overloads to `CosmosDiagnostics`, add environment variable support.
290+
**Acceptance:** `ToString(verbosity)` overloads compile and delegate correctly. Parameterless `ToString()` is unchanged (always `Detailed`). No behavioral change yet.
294291

295292
### WI-2: Summary Computation Engine
296293
**Scope:** Implement `DiagnosticsSummaryWriter` — the core logic that walks the trace tree, collects stats, groups by region, computes first/last/aggregated groups, and produces the summary JSON structure.
297294
**Acceptance:** Given an `ITrace` tree, produces the correct summary JSON. Unit-testable in isolation.
298295

299296
### WI-3: Summary Serialization Integration
300-
**Scope:** Modify `CosmosTraceDiagnostics.ToString()` and `TraceWriter.TraceJsonWriter` to branch on verbosity. When `Summary`, delegate to `DiagnosticsSummaryWriter`. Implement size enforcement and truncated output fallback.
301-
**Acceptance:** `ToString()` returns compact summary JSON when verbosity is `Summary`, full trace when `Detailed`.
297+
**Scope:** Implement `CosmosTraceDiagnostics.ToString(DiagnosticsVerbosity)` and `ToJsonString(DiagnosticsVerbosity)`. When `Summary`, delegate to `DiagnosticsSummaryWriter`. Implement size enforcement and truncated output fallback. Parameterless `ToString()` remains unchanged.
298+
**Acceptance:** `ToString(DiagnosticsVerbosity.Summary)` returns compact summary JSON. `ToString()` (parameterless) continues to return full `Detailed` trace.
302299

303300
### WI-4: Contract Updates & Public API Validation
304301
**Scope:** Update `ContractEnforcementTests` baselines for new public API surface. Ensure the new enum and properties appear in contracts.
@@ -335,9 +332,9 @@ Both `StoreResponseStatistics` (direct mode) and `HttpResponseStatistics` (gatew
335332
| `DiagnosticsVerbosity_DefaultIsDetailed` | Verify enum default |
336333
| `CosmosClientOptions_DiagnosticsVerbosity_DefaultValue` | Verify options default |
337334
| `CosmosClientOptions_MaxSummarySizeBytes_Validation` | Min 4096 enforced |
338-
| `Verbosity_Precedence_ClientOptionsOverridesEnvVar` | Client options overrides env var default |
339-
| `Verbosity_Precedence_EnvVarOverridesCodeDefault` | Environment variable fallback |
340-
| `ToString_Overload_UsesSummary_WhenExplicit` | `ToString(Summary)` produces summary output regardless of `Verbosity` property |
335+
| `CosmosClientOptions_DiagnosticsVerbosity_EnvVarFallback` | Env var populates `CosmosClientOptions.DiagnosticsVerbosity` when not set in code |
336+
| `CosmosClientOptions_DiagnosticsVerbosity_CodeOverridesEnvVar` | Code-set value takes precedence over env var |
337+
| `ToString_Overload_UsesSummary_WhenExplicit` | `ToString(Summary)` produces summary output |
341338
| `ToJsonString_Overload_UsesSummary_WhenExplicit` | `ToJsonString(Summary)` produces summary JSON |
342339
| `Summary_SingleRegion_SingleRequest` | No deduplication, first only |
343340
| `Summary_SingleRegion_TwoRequests` | First + last, no middle |
@@ -353,7 +350,7 @@ Both `StoreResponseStatistics` (direct mode) and `HttpResponseStatistics` (gatew
353350
| `Summary_EmptyTrace` | No requests produces minimal output |
354351
| `Summary_RegionOrdering_Deterministic` | Regions sorted alphabetically |
355352
| `Detailed_Mode_Unchanged` | Existing detailed output is byte-for-byte identical |
356-
| `ToString_UsesSummary_WhenVerbosityPropertySet` | Parameterless `ToString()` branches on `Verbosity` property |
353+
| `ToString_Parameterless_AlwaysDetailed` | Parameterless `ToString()` always returns `Detailed` output regardless of `CosmosClientOptions` setting |
357354

358355
### 8.2 Integration Tests (Emulator)
359356

0 commit comments

Comments
 (0)