Correct & extend Cosmos diagnostics contract doc by NaluTripician · Pull Request #4782 · Azure/azure-sdk-for-rust

NaluTripician · 2026-07-15T18:45:54Z

Documentation only. This supersedes the diagnostics contract doc from #4684, lifting its skeleton onto a fresh main-based branch and correcting its substantive errors. It is the WS1 deliverable of the Cosmos diagnostics plan.

Why this PR

The #4684 draft was a strong skeleton but had real bugs that would mislead the implementation workstreams. This PR fixes them:

Wrong OpenTelemetry semconv names. Add Cosmos diagnostics contract design doc #4684 used non-existent db.cosmosdb.* names (db.cosmosdb.operation.duration, db.cosmosdb.status_code, …). Corrected to the real names verified against open-telemetry/semantic-conventions (docs/db/cosmosdb.md): db.client.operation.duration, db.response.status_code, db.operation.name, azure.cosmosdb.* (operation.request_charge, consistency.level, response.sub_status_code, client.active_instance.count), and db.system.name = "azure.cosmosdb" with span kind CLIENT.
No emission model. Added the SDK-side DiagnosticsHandler chain (driver produces & materializes; SDK emits).
Sampling was under-specified. Added tail-based (late-bound) sampling as a concept explicitly distinct from DiagnosticsLevel (static depth).
No storm protection for emission count. Added cross-operation rate limiting, distinct from the per-artifact size bound.
azure_core gaps not called out. Noted the missing Meter abstraction and span-backdating hook, plus the develop-Cosmos-local-then-upstream plan.
Mis-tagged [main]. Add Cosmos diagnostics contract design doc #4684 tagged DiagnosticsContext & friends as on main; they are not — only DiagnosticsThresholds is. Re-tagged those to land in the WS0 foundation.
Added an R1-R9 requirement-to-section map so every requirement traces to a section.

Scope / reviewer notes

No code changes; additive design doc only. CosmosResponse::diagnostics() stays non-optional.
No CHANGELOG entry (doc-only, per repo rules).
.cspell.json gains a few technical words (semconv, workstream, materializer, …).
Verified: cSpell clean and markdownlint clean on the doc.

Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com

Salvage the DIAGNOSTICS-CONTRACT.md skeleton from PR Azure#4684 onto a fresh main-based branch and fix its substantive errors: - Correct the OpenTelemetry semconv names to the real ones: db.client.operation.duration, db.response.status_code, db.operation.name, azure.cosmosdb.* (request_charge, consistency.level, response.sub_status_code, active_instance.count) and db.system.name=azure.cosmosdb, replacing the non-existent db.cosmosdb.* names. - Add the DiagnosticsHandler chain as the SDK emission model (D1). - Add tail-based sampling as a concept distinct from DiagnosticsLevel (D4). - Add cross-operation rate limiting (D5), distinct from the per-artifact size bound. - Note the azure_core metrics/backdating gaps and the develop-local-then-upstream plan (D2/D3). - Re-tag DiagnosticsContext and friends as WS0 (not on main yet); only DiagnosticsThresholds is actually on main today. - Add an R1-R9 requirement-to-section map. Documentation only; no code changes and no CHANGELOG entry. Spelling and markdownlint pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Adds a Cosmos diagnostics contract and OpenTelemetry mapping, but also includes unrelated retry-policy changes.

Changes:

Defines diagnostics workstreams, emission, sampling, rate limiting, and OTel mappings.
Adds bounded 410 retry handling and terminal 503 conversion.
Adds tests, changelog documentation, and spelling terms.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 13 comments.

Show a summary per file

File	Description
`cosmos_fault_injection.rs`	Tests terminal 410-to-503 conversion.
`retry_policies/mod.rs`	Implements terminal response conversion.
`client_retry_policy.rs`	Adds Gone-family retries and refresh signaling.
`retry_handler.rs`	Applies terminal conversion after retry exhaustion.
`CHANGELOG.md`	Documents retry behavior.
`DIAGNOSTICS-CONTRACT.md`	Defines the diagnostics contract and OTel mapping.
`.cspell.json`	Adds diagnostics terminology.

+### Bugs Fixed
+
+- Fixed `410 Gone` responses carrying a partition-key-range routing sub-status (`1000` NameCacheIsStale, `1002` PartitionKeyRangeGone, `1007` CompletingSplit) escaping the retry pipeline as a raw, unclassifiable `410` (returned directly to the caller on writes, or after non-curative cross-region failover on reads). These are now retried within a small bound — flagging a routing-cache refresh so SDK-routing paths re-resolve stale routing information — and, on exhaustion, surfaced as `503 Service Unavailable` with the original sub-status preserved. This aligns the surfaced status with the rest of the Gone family so application-layer retry/circuit-breaker logic (wired for `503`, not `410`) can classify it.


+        if self.refresh_routing_on_next_attempt {
+            request.force_name_cache_refresh = true;
+            request.force_partition_key_range_refresh = true;
+            request.force_collection_routing_map_refresh = true;


+        // Not a Gone result: leave it untouched.
+        _ => return result,
+    };
+    headers.insert(SUB_STATUS, sub_status.to_string());


+) -> azure_core::Result<RawResponse> {
+    let mut headers = match &result {
+        Ok(response) if response.status() == StatusCode::Gone => response.headers().clone(),
+        Err(err) if err.http_status() == Some(StatusCode::Gone) => Headers::new(),


+        Ok(_) => Ok(RawResponse::from_bytes(
+            StatusCode::ServiceUnavailable,
+            headers,
+            Vec::new(),
+        )),


+> **Client instance id (D10).** `azure.client.id` and the active-instance metric use a stable
+> per-client id. Prefer `vmId`; when VM metadata is unreachable, fall back to a **static GUID**
+> so two requests can be attributed to the same `CosmosClient`/driver instance. **Check whether
+> [`DiagnosticsContext`][ctx] already carries this before adding it.**


+| **D7** | Metric cardinality | Operation-scope tags always-on; high-cardinality tags (consistency level, per-request region, partition-key-range, endpoint, replica) off by default, opt-in. (§10.2/§10.3) |
+| **D8** | SDK vs driver emission | Default **SDK-side** emission → exactly one public span per op; driver may expose an opt-in exporter. (§11) |
+| **D9** | Bounded size vs capture engine | Land the **bounded-size guarantee** [WS6] as a customer-facing property; keep the append-only capture engine [WS7] as an OFF-by-default internal optimization. (§8) |
+| **D10** | Client instance id | Stable per-client id feeds the active-instance metric + `azure.client.id`. Prefer `vmId`, fall back to a static GUID; check whether `DiagnosticsContext` already carries it. (§10.3) |


+
+[thresholds]: https://github.com/Azure/azure-sdk-for-rust/blob/main/sdk/cosmos/azure_data_cosmos_driver/src/options/diagnostics_thresholds.rs
+[opoptions]: https://github.com/Azure/azure-sdk-for-rust/blob/main/sdk/cosmos/azure_data_cosmos_driver/src/options/operation_options.rs
+[ctx]: https://github.com/Azure/azure-sdk-for-rust/blob/main/sdk/cosmos/azure_data_cosmos_driver/src/models/cosmos_response.rs


+  `is_failure()` / `is_threshold_violated()` predicates, [`RequestDiagnostics`][ctx], the
+  `DiagnosticsOptions` / `DiagnosticsVerbosity` config, and
+  `CosmosResponse::diagnostics() -> Arc<DiagnosticsContext>` (non-optional). Salvaged from the
+  closed PR #4619 **minus** its capture engine.


+  trait + chain (the emission extension point).
+- **[WS3]/[WS4]/[WS5]** — the three built-in handlers: metrics, tracing (tail-sampled,
+  backdated spans), and the sampling / rate-limiting log handler.
+- **[WS6]** — the retry-storm **bounded-size** compaction (salvaged from the closed PR #4683),


Bring DIAGNOSTICS-CONTRACT.md onto the observability branch (from the former standalone doc PR Azure#4782) and correct its now-stale framing: - The earlier draft assumed the DiagnosticsContext foundation was NOT yet on main and tagged it [WS0] as a blocking-root workstream. In reality the full foundation (DiagnosticsContext, CosmosResponse::diagnostics(), RequestDiagnostics, DiagnosticsVerbosity, DiagnosticsOptions) is already on upstream/main. Re-tag all foundation references [WS0] -> [main] and drop the "WS0 builds the foundation" narrative (WS0 dropped; PR Azure#4785 closed as redundant). - DiagnosticsThresholds was mis-tagged [main]; it is the one diagnostics type NOT on upstream, added by this PR -> re-tag [WS4/WS5]. Correct the regions_contacted() accessor name. - Add WS8 (op-scope wiring) to the legend; note WS2-WS8 ship together in the combined PR Azure#4789; mark DiagnosticsLevel as a [design]-only surface. - Fix the "documentation only" scope note now that the doc rides with the implementation. Keep the OTel semconv names accurate. Doc-only change: cSpell + markdownlint pass; added the doc's technical terms (workstream(s), materializer, underspecified, flatbuffer) to the Cosmos cSpell dictionary. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

NaluTripician · 2026-07-16T18:12:38Z

Folded into #4789 (combined observability PR).

Copilot AI review requested due to automatic review settings July 15, 2026 18:45

NaluTripician requested a review from a team as a code owner July 15, 2026 18:45

github-actions Bot added the Cosmos The azure_cosmos crate label Jul 15, 2026

github-project-automation Bot added this to CosmosDB Rust SDK and Driver Jul 15, 2026

github-project-automation Bot moved this to Todo in CosmosDB Rust SDK and Driver Jul 15, 2026

Copilot started reviewing on behalf of NaluTripician July 15, 2026 18:46 View session

Copilot AI reviewed Jul 15, 2026

View reviewed changes

NaluTripician force-pushed the nalutripician/cosmos-diag-ws1-doc branch from 0c2172d to 58443e3 Compare July 15, 2026 18:56

NaluTripician closed this Jul 16, 2026

github-project-automation Bot moved this from Todo to Done in CosmosDB Rust SDK and Driver Jul 16, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Correct & extend Cosmos diagnostics contract doc#4782

Correct & extend Cosmos diagnostics contract doc#4782
NaluTripician wants to merge 1 commit into
Azure:mainfrom
NaluTripician:nalutripician/cosmos-diag-ws1-doc

NaluTripician commented Jul 15, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

NaluTripician commented Jul 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		### Bugs Fixed

		- Fixed `410 Gone` responses carrying a partition-key-range routing sub-status (`1000` NameCacheIsStale, `1002` PartitionKeyRangeGone, `1007` CompletingSplit) escaping the retry pipeline as a raw, unclassifiable `410` (returned directly to the caller on writes, or after non-curative cross-region failover on reads). These are now retried within a small bound — flagging a routing-cache refresh so SDK-routing paths re-resolve stale routing information — and, on exhaustion, surfaced as `503 Service Unavailable` with the original sub-status preserved. This aligns the surfaced status with the rest of the Gone family so application-layer retry/circuit-breaker logic (wired for `503`, not `410`) can classify it.

Uh oh!

Conversation

NaluTripician commented Jul 15, 2026

Why this PR

Scope / reviewer notes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

NaluTripician commented Jul 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants