Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

inbound: Improve policy metrics #1237

Merged
merged 48 commits into from
Sep 3, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
48 commits
Select commit Hold shift + click to select a range
4695f77
Split error metrics tracking from stack layer
olix0r Aug 30, 2021
8b800ab
Move deny errors into errors mod
olix0r Aug 30, 2021
fcf47d6
Ensure connect timeouts are annotated properly; remove dispatch timeout
olix0r Aug 30, 2021
6fc5ea1
wip: start moving inbound metrics into inbound
olix0r Aug 30, 2021
67bdba9
wip: Move outbound metrics into outbound
olix0r Aug 31, 2021
56315f8
Restore HttpError type to decouple error responder from proxy-specifi…
olix0r Aug 31, 2021
4a6b3bd
Move errors into their respective crates
olix0r Aug 31, 2021
10c8a8b
comments
olix0r Aug 31, 2021
ded6fe4
Merge branch 'main' into ver/monitor
olix0r Aug 31, 2021
13981e5
fmt
olix0r Aug 31, 2021
adb07b6
Include target_addr in inbound HTTP errors
olix0r Aug 31, 2021
784b9b1
fix gateway error tests
olix0r Aug 31, 2021
a4719e9
Merge branch 'main' into ver/monitor
olix0r Aug 31, 2021
8943d00
wip
olix0r Aug 31, 2021
30f06c7
wip: separate http authorization layer
olix0r Sep 1, 2021
d8b20f6
Separate TCP authorization layer
olix0r Sep 1, 2021
53f41ed
wip
olix0r Sep 1, 2021
e38cb78
Always permit connections from localhost (for the admin server, in pa…
olix0r Sep 1, 2021
32dddfe
back-out admin policy
olix0r Sep 1, 2021
3e4591a
lint
olix0r Sep 2, 2021
185ab97
wire up authz metrics with minimal labeling
olix0r Sep 2, 2021
976f4b6
cleanup policy labels
olix0r Sep 2, 2021
b7ab6e7
fix telemetry tests
olix0r Sep 2, 2021
1a08b08
Stop handling policy labels as free-form maps.
olix0r Sep 2, 2021
8fea62d
fixup error metric labels
olix0r Sep 2, 2021
0b48476
backout loopback authorization changes
olix0r Sep 2, 2021
a883aca
metrics initialization touchup
olix0r Sep 2, 2021
a881656
Merge branch 'main' into ver/monitor
olix0r Sep 2, 2021
238d77b
report authz metrics
olix0r Sep 2, 2021
e745ca6
Set default labels on the admin server
olix0r Sep 2, 2021
e014101
admin stack doesn't need inbound metrics handle
olix0r Sep 2, 2021
5c58be6
back-out unneeded runtime changes
olix0r Sep 2, 2021
44a419c
smaller gateway change
olix0r Sep 2, 2021
8ba1a08
+comments
olix0r Sep 2, 2021
cbcec8b
inbound: Improve policy metrics
olix0r Sep 2, 2021
662b6e7
back out env changes
olix0r Sep 2, 2021
9c025b5
fixup fuzzer
olix0r Sep 2, 2021
93907af
Restore http error metrics on the admin server
olix0r Sep 2, 2021
230b707
fix fuzzer
olix0r Sep 2, 2021
8773eba
Merge branch 'main' into ver/monitor
olix0r Sep 2, 2021
4c8633c
indent
olix0r Sep 2, 2021
9b4020b
-comment
olix0r Sep 2, 2021
bf87bc5
dedupe forward stack
olix0r Sep 3, 2021
e24fefa
fixup
olix0r Sep 3, 2021
dd50466
We don't really gain anything from using a RwLock
olix0r Sep 3, 2021
632c1b8
fix tcp metrics comments
olix0r Sep 3, 2021
f390a29
Add a comment for AllowPolicy::changed
olix0r Sep 3, 2021
e25bf93
fixup fuzzer
olix0r Sep 3, 2021
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 2 additions & 12 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -668,7 +668,6 @@ dependencies = [
"linkerd-duplex",
"linkerd-errno",
"linkerd-error",
"linkerd-error-metrics",
"linkerd-error-respond",
"linkerd-exp-backoff",
"linkerd-http-classify",
Expand Down Expand Up @@ -750,6 +749,7 @@ dependencies = [
"linkerd-tonic-watch",
"linkerd-tracing",
"linkerd2-proxy-api",
"parking_lot",
"thiserror",
"tokio",
"tokio-test",
Expand Down Expand Up @@ -801,6 +801,7 @@ dependencies = [
"linkerd-identity",
"linkerd-io",
"linkerd-tracing",
"parking_lot",
"pin-project",
"thiserror",
"tokio",
Expand Down Expand Up @@ -910,17 +911,6 @@ dependencies = [
"futures",
]

[[package]]
name = "linkerd-error-metrics"
version = "0.1.0"
dependencies = [
"futures",
"linkerd-metrics",
"parking_lot",
"pin-project",
"tower",
]

[[package]]
name = "linkerd-error-respond"
version = "0.1.0"
Expand Down
1 change: 0 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ members = [
"linkerd/duplex",
"linkerd/error",
"linkerd/errno",
"linkerd/error-metrics",
"linkerd/error-respond",
"linkerd/exp-backoff",
"linkerd/http-box",
Expand Down
39 changes: 28 additions & 11 deletions linkerd/app/admin/src/stack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,10 @@ use linkerd_app_core::{
serve,
svc::{self, ExtractParam, InsertParam, Param},
tls, trace,
transport::{self, listen::Bind, ClientAddr, Local, Remote, ServerAddr},
Error,
transport::{self, listen::Bind, ClientAddr, Local, OrigDstAddr, Remote, ServerAddr},
Error, Result,
};
use linkerd_app_inbound as inbound;
use std::{pin::Pin, time::Duration};
use thiserror::Error;
use tokio::sync::mpsc;
Expand Down Expand Up @@ -64,7 +65,7 @@ impl Config {
bind: B,
identity: Option<LocalCrtKey>,
report: R,
metrics: metrics::Proxy,
metrics: inbound::Metrics,
trace: trace::Handle,
drain: drain::Watch,
shutdown: mpsc::UnboundedSender<()>,
Expand All @@ -79,11 +80,11 @@ impl Config {
let (ready, latch) = crate::server::Readiness::new();
let admin = crate::server::Admin::new(report, ready, shutdown, trace);
let admin = svc::stack(move |_| admin.clone())
.push(metrics.http_endpoint.to_layer::<classify::Response, _, Http>())
.push(metrics.proxy.http_endpoint.to_layer::<classify::Response, _, Http>())
.push(metrics.http_errors.to_layer())
.push_on_service(
svc::layers()
.push(metrics.http_errors.clone())
.push(errors::layer())
.push(errors::respond::layer())
.push(http::BoxResponse::layer()),
)
.push(http::NewServeHttp::layer(Default::default(), drain.clone()))
Expand Down Expand Up @@ -130,8 +131,10 @@ impl Config {
)
.push(svc::BoxNewService::layer())
.push(detect::NewDetectService::layer(detect::Config::<http::DetectHttp>::from_timeout(DETECT_TIMEOUT)))
.push(transport::metrics::NewServer::layer(metrics.transport))
.push_map_target(|(tls, addrs): (tls::ConditionalServerTls, B::Addrs)| {
.push(transport::metrics::NewServer::layer(metrics.proxy.transport))
.push_map_target(move |(tls, addrs): (tls::ConditionalServerTls, B::Addrs)| {
// TODO(ver): We should enforce policy here; but we need to permit liveness probes
// for destination pods to startup...
Tcp {
tls,
client: addrs.param(),
Expand Down Expand Up @@ -161,8 +164,7 @@ impl Param<transport::labels::Key> for Tcp {
self.tls.clone(),
self.addr.into(),
// TODO(ver) enforce policies on the proxy's admin port.
Default::default(),
Default::default(),
metrics::ServerLabel("default:admin".to_string()),
)
}
}
Expand All @@ -175,13 +177,28 @@ impl Param<http::Version> for Http {
}
}

impl Param<OrigDstAddr> for Http {
fn param(&self) -> OrigDstAddr {
OrigDstAddr(self.tcp.addr.into())
}
}

impl Param<metrics::ServerLabel> for Http {
fn param(&self) -> metrics::ServerLabel {
metrics::ServerLabel("default:admin".to_string())
}
}

impl Param<metrics::EndpointLabels> for Http {
fn param(&self) -> metrics::EndpointLabels {
metrics::InboundEndpointLabels {
tls: self.tcp.tls.clone(),
authority: None,
target_addr: self.tcp.addr.into(),
policy: Default::default(),
policy: metrics::AuthzLabels {
server: self.param(),
authz: "default:all-unauthenticated".to_string(),
},
}
.into()
}
Expand Down
1 change: 0 additions & 1 deletion linkerd/app/core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ linkerd-detect = { path = "../../detect" }
linkerd-duplex = { path = "../../duplex" }
linkerd-errno = { path = "../../errno" }
linkerd-error = { path = "../../error" }
linkerd-error-metrics = { path = "../../error-metrics" }
linkerd-error-respond = { path = "../../error-respond" }
linkerd-exp-backoff = { path = "../../exp-backoff" }
linkerd-http-classify = { path = "../../http-classify" }
Expand Down
2 changes: 1 addition & 1 deletion linkerd/app/core/src/control.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ impl Config {

svc::stack(ConnectTcp::new(self.connect.keepalive))
.push(tls::Client::layer(identity))
.push_timeout(self.connect.timeout)
.push_connect_timeout(self.connect.timeout)
.push(self::client::layer())
.push_on_service(svc::MapErrLayer::new(Into::into))
.into_new_service()
Expand Down
44 changes: 44 additions & 0 deletions linkerd/app/core/src/errors/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
pub mod respond;

use linkerd_error::Error;
pub use linkerd_timeout::{FailFastError, ResponseTimeout};
use thiserror::Error;

#[derive(Debug, Error)]
#[error("connect timed out after {0:?}")]
pub(crate) struct ConnectTimeout(pub std::time::Duration);

#[derive(Debug, Error)]
#[error("{source}")]
pub struct HttpError {
#[source]
source: Error,
http_status: http::StatusCode,
grpc_status: tonic::Code,
}

impl HttpError {
pub fn bad_request(source: impl Into<Error>) -> Self {
Self {
source: source.into(),
http_status: http::StatusCode::BAD_REQUEST,
grpc_status: tonic::Code::InvalidArgument,
}
}

pub fn forbidden(source: impl Into<Error>) -> Self {
Self {
source: source.into(),
http_status: http::StatusCode::FORBIDDEN,
grpc_status: tonic::Code::PermissionDenied,
}
}

pub fn loop_detected(source: impl Into<Error>) -> Self {
Self {
source: source.into(),
http_status: http::StatusCode::LOOP_DETECTED,
grpc_status: tonic::Code::Aborted,
}
}
}
Loading