feat(tonic-xds): make outlier detection transport-agnostic by LYZJU2019 · Pull Request #2849 · grpc/grpc-rust

LYZJU2019 · 2026-09-02T18:34:01Z

Motivation

Outlier detection records a per-call success/failure via svc.record_outcome(result.is_ok()) — a transport-level signal that counts an Ok(503) or a gRPC trailers-only error (200 + grpc-status != 0) as a success. That is wrong for a non-gRPC transport, so a plain-HTTP consumer (e.g. an xDS HTTP data plane) cannot use OD correctly.

Solution

Introduce a pluggable OutcomeClassifier seam so the transport decides what a call outcome means for OD.

Scope: this is the classification seam only. Threading the classifier through XdsChannelBuilder and the OD Discover is deferred to a follow-up, since the OD load balancer is not yet on the production channel path.

Testing Done

6 GrpcOutcomeClassifier unit tests (transport error; 2xx without status; trailers-only 0 / non-zero / unparseable; non-2xx).
2 load-balancer wiring tests: a Failure verdict ejects an endpoint whose transport calls all succeed; an Ignore verdict never ejects endpoints whose transport calls all fail.
Full tonic-xds lib suite green (423 tests); clippy clean (default and --all-features); rustdoc intra-doc links resolve.

…ort-agnostic Outlier detection recorded a per-call success/failure via `svc.record_outcome(result.is_ok())`, a transport-level signal that counts an `Ok(503)` or a gRPC trailers-only error as success. That is wrong for a non-gRPC transport, so a plain-HTTP caller cannot use OD correctly. Introduce a pluggable `OutcomeClassifier` seam, mirroring the retry layer's `RetryClassifier`: - `CallOutcome<'a>` borrows the endpoint result (status + headers, or an error marker) with no error payload, so it works with the load balancer's generic endpoint error type. - `HealthOutcome` is a three-way verdict (Success / Failure / Ignore); `Ignore` records nothing, letting a transport drop outcomes that reflect neither upstream health nor fault (e.g. HTTP 4xx). - `GrpcOutcomeClassifier` is the built-in default: a transport error or non-2xx is a failure, and on a 2xx response the `grpc-status` header decides. The load balancer holds an `Arc<dyn OutcomeClassifier>` and consults it at the call site instead of `result.is_ok()`. The response is read through a small internal `OutcomeSource` trait (blanket-implemented for `http::Response<B>`), keeping the balancer generic over the response body. This is the classification seam only; wiring the classifier through the channel builder and the OD Discover is left to a follow-up, since the OD load balancer is not yet on the production channel path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

ankurmittal · 2026-09-02T20:55:13Z

@@ -0,0 +1,174 @@
+/*
+ *
+ * Copyright 2025 gRPC authors.


addressed in 7363d8c

ankurmittal · 2026-09-02T21:13:22Z

+    }
+
+    #[tokio::test]
+    async fn outcome_classifier_ignore_records_nothing() {


Every transport call fails and the test asserts no endpoint was ejected. If
Ignore recorded a success instead of nothing, there would still be no
ejection.

addressed in ce8469b

ankurmittal · 2026-09-02T21:14:01Z

 pub use client::endpoint::{
    ClusterConfig, Connector, EndpointAddress, EndpointChannel, MakeConnector,
 };
+pub use client::loadbalance::outcome::{


LoadBalancer::new is pub(crate), so nothing outside the crate can inject a
classifier, but all four types are re-exported from the crate root.

Two of them are frozen as shaped: #[non_exhaustive] sits on the CallOutcome
enum rather than on the Response variant, so the deferred follow-up can't add
a trailers field, and HealthOutcome has no #[non_exhaustive] at all.

addressed in f0c2750

ankurmittal · 2026-09-02T21:14:36Z

+                }
+                let ok = headers
+                    .get("grpc-status")
+                    .is_none_or(|v| v.to_str().ok().and_then(|s| s.parse::<u32>().ok()) == Some(0));


parse::<u32>() accepts leading zeros. The protocol says the value is decimal
encoded "without any leading zeros", and tonic's Code::from_bytes has no
(b'0', b'0') arm, so it maps 00 to Unknown. The RPC fails and outlier
detection counts a success.

addressed in f6f59a7

ankurmittal · 2026-09-02T21:14:51Z

+    fn call_outcome(&self) -> CallOutcome<'_> {
+        CallOutcome::Response {
+            status: self.status(),
+            headers: self.headers(),


call_outcome() reads self.headers(), and it runs on the value returned by
svc.call(req).await — before the body is consumed, so trailers don't exist
yet. The protocol puts the status there for everything that isn't
Trailers-Only:

Response → (Response-Headers *Length-Prefixed-Message Trailers) / Trailers-Only

is_none_or at line 97 maps an absent grpc-status to success, so a streaming
call that fails after the first message, or a unary call where the server sent
headers before erroring, is recorded as healthy.

Not addressed here: reading trailers requires draining the response body after call returns, which is out of scope for this classification seam and is the same limitation as the result.is_ok() path it replaces (a trailer-delivered failure was already counted healthy). It lands with the trailer-aware follow-up — CallOutcome::Response is kept crate-internal (see f0c2750) so a trailers field can be added then without a breaking change.

ankurmittal

did you forget to push the commit

by mistake

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

The classifier parsed `grpc-status` with `parse::<u32>()`, which accepts a leading-zero encoding like `00`. The gRPC protocol requires the value be decimal without leading zeros, and tonic's `Code::from_bytes` maps `00` to `Unknown` — so the RPC fails while OD recorded a success. Defer decoding to `tonic::Status::from_header_map` (as the retry classifier already does) so the health verdict matches how the RPC resolves. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

The Ignore test drove failing calls and only checked that nothing was ejected, which a classifier that recorded a success on each call would also satisfy. Assert the per-channel counters stay at (0, 0) instead, which fails if Ignore recorded either a success or a failure. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

`LoadBalancer::new` is `pub(crate)`, so nothing outside the crate can inject a classifier yet, but the four types were re-exported from the crate root — a public API that can't be used, with `CallOutcome`/`HealthOutcome` already frozen in shapes the deferred trailer-aware follow-up would need to change. Drop the crate-root re-export and make the types `pub(crate)`. The public builder hook, and the final `#[non_exhaustive]` shaping, land together with the outlier-detection Discover wiring. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

LYZJU2019 changed the title ~~feat(tonic-xds): make outlier-detection outcome classification transport-agnostic~~ feat(tonic-xds): make outlier detection transport-agnostic Sep 2, 2026

ankurmittal reviewed Sep 2, 2026

View reviewed changes

ankurmittal approved these changes Sep 3, 2026

View reviewed changes

ankurmittal previously requested changes Sep 3, 2026

View reviewed changes

ankurmittal self-requested a review September 3, 2026 22:05

LYZJU2019 and others added 4 commits September 3, 2026 15:08

chore(tonic-xds): use the file's creation year in the outcome.rs header

7363d8c

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

Navigation Menu

Uh oh!

feat(tonic-xds): make outlier detection transport-agnostic - #2849

feat(tonic-xds): make outlier detection transport-agnostic#2849
LYZJU2019 wants to merge 5 commits into
grpc:masterfrom
LYZJU2019:lyzju2019/od-transport-agnostic-outcome-classifier

LYZJU2019 commented Sep 2, 2026 •

edited

Loading

Uh oh!

ankurmittal Sep 2, 2026

Uh oh!

LYZJU2019 Sep 3, 2026

Uh oh!

ankurmittal Sep 2, 2026

Uh oh!

LYZJU2019 Sep 3, 2026

Uh oh!

ankurmittal Sep 2, 2026

Uh oh!

LYZJU2019 Sep 3, 2026

Uh oh!

ankurmittal Sep 2, 2026

Uh oh!

LYZJU2019 Sep 3, 2026

Uh oh!

ankurmittal Sep 2, 2026

Uh oh!

LYZJU2019 Sep 3, 2026

Uh oh!

ankurmittal left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

LYZJU2019 commented Sep 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Solution

Testing Done

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ankurmittal left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

LYZJU2019 commented Sep 2, 2026 •

edited

Loading