From fd5a6aabb10d5cfd20220c7e8647ee383c1f958b Mon Sep 17 00:00:00 2001 From: Oliver Gould Date: Wed, 13 May 2020 11:48:22 -0700 Subject: [PATCH] Add an `i/o` error label to http metrics (#512) This change modifies HTTP error-labeling to detect I/O errors and label them explicitly. Previously all I/O errors were reported as `unexpected`. Additionally, an `errno` label is included when possible. Fixes linkerd/linkerd2#4364 --- Cargo.lock | 6 ++++ Cargo.toml | 1 + linkerd/app/core/Cargo.toml | 1 + linkerd/app/core/src/errors.rs | 33 ++++++++++++++----- linkerd/errno/Cargo.toml | 6 ++++ .../src/metrics/errno.rs => errno/src/lib.rs} | 2 ++ linkerd/proxy/transport/Cargo.toml | 1 + linkerd/proxy/transport/src/metrics/mod.rs | 4 +-- 8 files changed, 44 insertions(+), 10 deletions(-) create mode 100644 linkerd/errno/Cargo.toml rename linkerd/{proxy/transport/src/metrics/errno.rs => errno/src/lib.rs} (99%) diff --git a/Cargo.lock b/Cargo.lock index 00e49b37ea..c6129834bd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -641,6 +641,7 @@ dependencies = [ "linkerd2-dns", "linkerd2-drain", "linkerd2-duplex", + "linkerd2-errno", "linkerd2-error", "linkerd2-error-metrics", "linkerd2-error-respond", @@ -856,6 +857,10 @@ dependencies = [ "tracing", ] +[[package]] +name = "linkerd2-errno" +version = "0.1.0" + [[package]] name = "linkerd2-error" version = "0.1.0" @@ -1197,6 +1202,7 @@ dependencies = [ "libc", "linkerd2-conditional", "linkerd2-dns-name", + "linkerd2-errno", "linkerd2-error", "linkerd2-identity", "linkerd2-io", diff --git a/Cargo.toml b/Cargo.toml index a031e9d9bf..0f0af28514 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -19,6 +19,7 @@ members = [ "linkerd/drain", "linkerd/duplex", "linkerd/error", + "linkerd/errno", "linkerd/error-metrics", "linkerd/error-respond", "linkerd/exp-backoff", diff --git a/linkerd/app/core/Cargo.toml b/linkerd/app/core/Cargo.toml index 24e2a4b05f..9cec71be69 100644 --- a/linkerd/app/core/Cargo.toml +++ b/linkerd/app/core/Cargo.toml @@ -29,6 +29,7 @@ linkerd2-conditional = { path = "../../conditional" } linkerd2-dns = { path = "../../dns" } linkerd2-drain = { path = "../../drain" } linkerd2-duplex = { path = "../../duplex" } +linkerd2-errno = { path = "../../errno" } linkerd2-error = { path = "../../error" } linkerd2-error-metrics = { path = "../../error-metrics" } linkerd2-error-respond = { path = "../../error-respond" } diff --git a/linkerd/app/core/src/errors.rs b/linkerd/app/core/src/errors.rs index 3ff66e8314..68f44971b3 100644 --- a/linkerd/app/core/src/errors.rs +++ b/linkerd/app/core/src/errors.rs @@ -1,6 +1,7 @@ use crate::proxy::identity; use futures::{Async, Poll}; use http::{header::HeaderValue, StatusCode}; +use linkerd2_errno::Errno; use linkerd2_error::Error; use linkerd2_error_metrics as metrics; use linkerd2_error_respond as respond; @@ -30,6 +31,7 @@ pub enum Reason { DispatchTimeout, ResponseTimeout, IdentityRequired, + Io(Option), FailFast, Unexpected, } @@ -295,11 +297,9 @@ impl std::fmt::Display for IdentityRequired { impl std::error::Error for IdentityRequired {} -impl metrics::LabelError for LabelError { - type Labels = Label; - - fn label_error(&self, err: &Error) -> Self::Labels { - let reason = if err.is::() { +impl LabelError { + fn reason(err: &(dyn std::error::Error + 'static)) -> Reason { + if err.is::() { Reason::ResponseTimeout } else if err.is::() { Reason::FailFast @@ -307,11 +307,21 @@ impl metrics::LabelError for LabelError { Reason::DispatchTimeout } else if err.is::() { Reason::IdentityRequired + } else if let Some(e) = err.downcast_ref::() { + Reason::Io(e.raw_os_error().map(Errno::from)) + } else if let Some(e) = err.source() { + Self::reason(e) } else { Reason::Unexpected - }; + } + } +} - (self.0, reason) +impl metrics::LabelError for LabelError { + type Labels = Label; + + fn label_error(&self, err: &Error) -> Self::Labels { + (self.0, Self::reason(err.as_ref())) } } @@ -325,9 +335,16 @@ impl metrics::FmtLabels for Reason { Reason::DispatchTimeout => "dispatch timeout", Reason::ResponseTimeout => "response timeout", Reason::IdentityRequired => "identity required", + Reason::Io(_) => "i/o", Reason::Unexpected => "unexpected", } - ) + )?; + + if let Reason::Io(Some(errno)) = self { + write!(f, ",errno=\"{}\"", errno)?; + } + + Ok(()) } } diff --git a/linkerd/errno/Cargo.toml b/linkerd/errno/Cargo.toml new file mode 100644 index 0000000000..28399240f8 --- /dev/null +++ b/linkerd/errno/Cargo.toml @@ -0,0 +1,6 @@ +[package] +name = "linkerd2-errno" +version = "0.1.0" +authors = ["Linkerd Developers "] +edition = "2018" +publish = false diff --git a/linkerd/proxy/transport/src/metrics/errno.rs b/linkerd/errno/src/lib.rs similarity index 99% rename from linkerd/proxy/transport/src/metrics/errno.rs rename to linkerd/errno/src/lib.rs index 02f873bc74..46e4c2633e 100644 --- a/linkerd/proxy/transport/src/metrics/errno.rs +++ b/linkerd/errno/src/lib.rs @@ -1,3 +1,5 @@ +#![deny(warnings, rust_2018_idioms)] + use std::fmt; /// Represents a platform-agnostic system error for metrics labels. diff --git a/linkerd/proxy/transport/Cargo.toml b/linkerd/proxy/transport/Cargo.toml index 7184daca01..de34cf8307 100644 --- a/linkerd/proxy/transport/Cargo.toml +++ b/linkerd/proxy/transport/Cargo.toml @@ -19,6 +19,7 @@ futures = "0.1" indexmap = "1.0.0" linkerd2-conditional = { path = "../../conditional" } linkerd2-dns-name = { path = "../../dns/name" } +linkerd2-errno = { path = "../../errno" } linkerd2-error = { path = "../../error" } linkerd2-identity = { path = "../../identity" } linkerd2-io = { path = "../../io" } diff --git a/linkerd/proxy/transport/src/metrics/mod.rs b/linkerd/proxy/transport/src/metrics/mod.rs index b4032a23a2..c90f0d2d3b 100644 --- a/linkerd/proxy/transport/src/metrics/mod.rs +++ b/linkerd/proxy/transport/src/metrics/mod.rs @@ -1,5 +1,6 @@ use futures::{try_ready, Future, Poll}; use indexmap::IndexMap; +use linkerd2_errno::Errno; use linkerd2_metrics::{ latency, metrics, Counter, FmtLabels, FmtMetric, FmtMetrics, Gauge, Histogram, Metric, }; @@ -10,10 +11,9 @@ use std::time::Instant; use tokio::io::{AsyncRead, AsyncWrite}; use tracing::debug; -mod errno; mod io; -pub use self::{errno::Errno, io::Io}; +pub use self::io::Io; metrics! { tcp_open_total: Counter { "Total count of opened connections" },