Skip to content

Commit

Permalink
Add an i/o error label to http metrics (#512)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
olix0r committed May 13, 2020
1 parent 0a8c546 commit fd5a6aa
Show file tree
Hide file tree
Showing 8 changed files with 44 additions and 10 deletions.
6 changes: 6 additions & 0 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -641,6 +641,7 @@ dependencies = [
"linkerd2-dns",
"linkerd2-drain",
"linkerd2-duplex",
"linkerd2-errno",
"linkerd2-error",
"linkerd2-error-metrics",
"linkerd2-error-respond",
Expand Down Expand Up @@ -856,6 +857,10 @@ dependencies = [
"tracing",
]

[[package]]
name = "linkerd2-errno"
version = "0.1.0"

[[package]]
name = "linkerd2-error"
version = "0.1.0"
Expand Down Expand Up @@ -1197,6 +1202,7 @@ dependencies = [
"libc",
"linkerd2-conditional",
"linkerd2-dns-name",
"linkerd2-errno",
"linkerd2-error",
"linkerd2-identity",
"linkerd2-io",
Expand Down
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ members = [
"linkerd/drain",
"linkerd/duplex",
"linkerd/error",
"linkerd/errno",
"linkerd/error-metrics",
"linkerd/error-respond",
"linkerd/exp-backoff",
Expand Down
1 change: 1 addition & 0 deletions linkerd/app/core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
Expand Down
33 changes: 25 additions & 8 deletions linkerd/app/core/src/errors.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -30,6 +31,7 @@ pub enum Reason {
DispatchTimeout,
ResponseTimeout,
IdentityRequired,
Io(Option<Errno>),
FailFast,
Unexpected,
}
Expand Down Expand Up @@ -295,23 +297,31 @@ impl std::fmt::Display for IdentityRequired {

impl std::error::Error for IdentityRequired {}

impl metrics::LabelError<Error> for LabelError {
type Labels = Label;

fn label_error(&self, err: &Error) -> Self::Labels {
let reason = if err.is::<ResponseTimeout>() {
impl LabelError {
fn reason(err: &(dyn std::error::Error + 'static)) -> Reason {
if err.is::<ResponseTimeout>() {
Reason::ResponseTimeout
} else if err.is::<FailFastError>() {
Reason::FailFast
} else if err.is::<tower::timeout::error::Elapsed>() {
Reason::DispatchTimeout
} else if err.is::<IdentityRequired>() {
Reason::IdentityRequired
} else if let Some(e) = err.downcast_ref::<std::io::Error>() {
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<Error> for LabelError {
type Labels = Label;

fn label_error(&self, err: &Error) -> Self::Labels {
(self.0, Self::reason(err.as_ref()))
}
}

Expand All @@ -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(())
}
}

Expand Down
6 changes: 6 additions & 0 deletions linkerd/errno/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[package]
name = "linkerd2-errno"
version = "0.1.0"
authors = ["Linkerd Developers <[email protected]>"]
edition = "2018"
publish = false
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
#![deny(warnings, rust_2018_idioms)]

use std::fmt;

/// Represents a platform-agnostic system error for metrics labels.
Expand Down
1 change: 1 addition & 0 deletions linkerd/proxy/transport/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
Expand Down
4 changes: 2 additions & 2 deletions linkerd/proxy/transport/src/metrics/mod.rs
Original file line number Diff line number Diff line change
@@ -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,
};
Expand All @@ -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" },
Expand Down

0 comments on commit fd5a6aa

Please sign in to comment.