From 6c1f866c537881f9af61a336b98dc02f81de89c2 Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Thu, 1 Oct 2020 11:04:27 -0700 Subject: [PATCH 1/3] add client & server identity to all metric labels --- linkerd/app/core/src/metric_labels.rs | 14 ++---- linkerd/app/core/src/transport/labels.rs | 59 ++++++++++++++++-------- linkerd/app/inbound/src/endpoint.rs | 5 +- linkerd/app/inbound/src/lib.rs | 8 ++-- linkerd/app/outbound/src/endpoint.rs | 10 ++-- linkerd/app/outbound/src/lib.rs | 5 +- 6 files changed, 61 insertions(+), 40 deletions(-) diff --git a/linkerd/app/core/src/metric_labels.rs b/linkerd/app/core/src/metric_labels.rs index 204921a719..e22fd7b569 100644 --- a/linkerd/app/core/src/metric_labels.rs +++ b/linkerd/app/core/src/metric_labels.rs @@ -1,7 +1,6 @@ use crate::proxy::identity; -use crate::transport::{labels::TlsStatus, tls}; +pub use crate::transport::labels::TlsStatus; use linkerd2_addr::Addr; -use linkerd2_conditional::Conditional; use linkerd2_metrics::FmtLabels; use std::fmt::{self, Write}; @@ -16,7 +15,7 @@ pub struct ControlLabels { #[derive(Clone, Debug, PartialEq, Eq, Hash)] pub struct EndpointLabels { pub direction: Direction, - pub tls_id: Conditional, + pub tls_id: TlsStatus, pub authority: Option, pub labels: Option, } @@ -55,7 +54,7 @@ impl From for ControlLabels { fn from(c: control::ControlAddr) -> Self { ControlLabels { addr: c.addr.clone(), - tls_status: c.identity.as_ref().into(), + tls_status: c.identity.map(|id| TlsId::ServerId(id)).into(), } } } @@ -106,12 +105,7 @@ impl FmtLabels for EndpointLabels { } write!(f, ",")?; - TlsStatus::from(self.tls_id.as_ref()).fmt_labels(f)?; - - if let Conditional::Some(ref id) = self.tls_id { - write!(f, ",")?; - id.fmt_labels(f)?; - } + self.tls_id.fmt_labels(f)?; Ok(()) } diff --git a/linkerd/app/core/src/transport/labels.rs b/linkerd/app/core/src/transport/labels.rs index 1e0a92147f..425b46a439 100644 --- a/linkerd/app/core/src/transport/labels.rs +++ b/linkerd/app/core/src/transport/labels.rs @@ -1,5 +1,5 @@ use super::tls; -pub use crate::metric_labels::{Direction, EndpointLabels}; +pub use crate::metric_labels::{Direction, EndpointLabels, TlsId}; use linkerd2_conditional::Conditional; use linkerd2_metrics::FmtLabels; use std::fmt; @@ -15,11 +15,17 @@ pub enum Key { Connect(EndpointLabels), } -#[derive(Copy, Clone, Debug, Eq, PartialEq, Hash)] -pub struct TlsStatus(tls::Conditional<()>); +#[derive(Clone, Debug, Eq, PartialEq, Hash)] +pub struct TlsStatus(tls::Conditional); // === impl Key === +impl Key { + pub fn accept(direction: Direction, id: tls::PeerIdentity) -> Self { + Self::Accept(direction, TlsStatus::client(id)) + } +} + impl FmtLabels for Key { fn fmt_labels(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { @@ -37,36 +43,53 @@ impl FmtLabels for Key { // === impl TlsStatus === -impl From> for TlsStatus { - fn from(inner: tls::Conditional) -> Self { - TlsStatus(inner.map(|_| ())) +impl TlsStatus { + pub fn client(id: tls::PeerIdentity) -> Self { + Self(id.map(TlsId::ClientId)) + } + + pub fn server(id: tls::PeerIdentity) -> Self { + Self(id.map(TlsId::ClientId)) } } -impl Into> for TlsStatus { - fn into(self) -> tls::Conditional<()> { - self.0 +impl From> for TlsStatus { + fn from(inner: tls::Conditional) -> Self { + TlsStatus(inner) + } +} + +impl Into for TlsStatus { + fn into(self) -> tls::PeerIdentity { + self.0.map(|id| match id { + TlsId::ClientId(id) => id, + TlsId::ServerId(id) => id, + }) } } impl fmt::Display for TlsStatus { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self.0 { - Conditional::Some(()) => write!(f, "true"), - Conditional::None(r) => fmt::Display::fmt(&r, f), + Conditional::Some(_) => write!(f, "true"), + Conditional::None(ref r) => fmt::Display::fmt(&r, f), } } } impl FmtLabels for TlsStatus { fn fmt_labels(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - if let Self(Conditional::None(tls::ReasonForNoPeerName::LocalIdentityDisabled)) = self { - return write!(f, "tls=\"disabled\""); - } - if let Self(Conditional::None(why)) = self { - return write!(f, "tls=\"no_identity\",no_tls_reason=\"{}\"", why); + match self.0 { + Conditional::None(tls::ReasonForNoPeerName::LocalIdentityDisabled) => { + write!(f, "tls=\"disabled\"") + } + Conditional::None(ref why) => { + write!(f, "tls=\"no_identity\",no_tls_reason=\"{}\"", why) + } + Conditional::Some(ref id) => { + write!(f, "tls=\"true\"")?; + id.fmt_labels(f) + } } - - write!(f, "tls=\"{}\"", self) } } diff --git a/linkerd/app/inbound/src/endpoint.rs b/linkerd/app/inbound/src/endpoint.rs index 57e2520c19..8b42717f34 100644 --- a/linkerd/app/inbound/src/endpoint.rs +++ b/linkerd/app/inbound/src/endpoint.rs @@ -137,7 +137,10 @@ impl Into for Target { metric_labels::EndpointLabels { authority: self.dst.name_addr().map(|d| d.as_http_authority()), direction: metric_labels::Direction::In, - tls_id: self.tls_client_id.map(metric_labels::TlsId::ClientId), + tls_id: self + .tls_client_id + .map(metric_labels::TlsId::ClientId) + .into(), labels: None, } } diff --git a/linkerd/app/inbound/src/lib.rs b/linkerd/app/inbound/src/lib.rs index 8333364d06..499c7bf7c5 100644 --- a/linkerd/app/inbound/src/lib.rs +++ b/linkerd/app/inbound/src/lib.rs @@ -471,9 +471,9 @@ impl transport::metrics::TransportLabels for TransportLabels { type Labels = transport::labels::Key; fn transport_labels(&self, _: &listen::Addrs) -> Self::Labels { - transport::labels::Key::Accept( + transport::labels::Key::accept( transport::labels::Direction::In, - tls::Conditional::<()>::None(tls::ReasonForNoPeerName::PortSkipped).into(), + tls::Conditional::None(tls::ReasonForNoPeerName::PortSkipped), ) } } @@ -482,9 +482,9 @@ impl transport::metrics::TransportLabels for TransportLabels type Labels = transport::labels::Key; fn transport_labels(&self, target: &tls::accept::Meta) -> Self::Labels { - transport::labels::Key::Accept( + transport::labels::Key::accept( transport::labels::Direction::In, - target.peer_identity.as_ref().into(), + target.peer_identity.clone(), ) } } diff --git a/linkerd/app/outbound/src/endpoint.rs b/linkerd/app/outbound/src/endpoint.rs index b23c18a551..87beffc6a4 100644 --- a/linkerd/app/outbound/src/endpoint.rs +++ b/linkerd/app/outbound/src/endpoint.rs @@ -3,7 +3,7 @@ use indexmap::IndexMap; use linkerd2_app_core::{ dst, metric_labels, - metric_labels::{prefix_labels, EndpointLabels}, + metric_labels::{prefix_labels, EndpointLabels, TlsStatus}, profiles, proxy::{ api_resolve::{Metadata, ProtocolHint}, @@ -314,11 +314,11 @@ impl CanOverrideAuthority for HttpEndpoint { impl Into for HttpEndpoint { fn into(self) -> EndpointLabels { - use linkerd2_app_core::metric_labels::{Direction, TlsId}; + use linkerd2_app_core::metric_labels::Direction; EndpointLabels { authority: Some(self.concrete.logical.dst.to_http_authority()), direction: Direction::Out, - tls_id: self.identity.as_ref().map(|id| TlsId::ServerId(id.clone())), + tls_id: TlsStatus::server(self.identity.clone()), labels: prefix_labels("dst", self.metadata.labels().into_iter()), } } @@ -357,12 +357,12 @@ impl tls::HasPeerIdentity for TcpEndpoint { impl Into for TcpEndpoint { fn into(self) -> EndpointLabels { - use linkerd2_app_core::metric_labels::{Direction, TlsId}; + use linkerd2_app_core::metric_labels::Direction; EndpointLabels { authority: Some(self.dst.to_http_authority()), direction: Direction::Out, labels: self.labels, - tls_id: self.identity.as_ref().map(|id| TlsId::ServerId(id.clone())), + tls_id: TlsStatus::server(self.identity), } } } diff --git a/linkerd/app/outbound/src/lib.rs b/linkerd/app/outbound/src/lib.rs index c9f0b3a73a..08df3613b2 100644 --- a/linkerd/app/outbound/src/lib.rs +++ b/linkerd/app/outbound/src/lib.rs @@ -577,8 +577,9 @@ impl transport::metrics::TransportLabels for TransportLabels { type Labels = transport::labels::Key; fn transport_labels(&self, _: &listen::Addrs) -> Self::Labels { - const NO_TLS: tls::Conditional<()> = Conditional::None(tls::ReasonForNoPeerName::Loopback); - transport::labels::Key::Accept(transport::labels::Direction::Out, NO_TLS.into()) + const NO_TLS: tls::Conditional = + Conditional::None(tls::ReasonForNoPeerName::Loopback); + transport::labels::Key::accept(transport::labels::Direction::Out, NO_TLS.into()) } } From e5bda48860b34c27c9cde39024098c8eec9b809f Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Thu, 1 Oct 2020 12:30:49 -0700 Subject: [PATCH 2/3] missing comma Signed-off-by: Eliza Weisman --- linkerd/app/core/src/transport/labels.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/linkerd/app/core/src/transport/labels.rs b/linkerd/app/core/src/transport/labels.rs index 425b46a439..86ec3be5e7 100644 --- a/linkerd/app/core/src/transport/labels.rs +++ b/linkerd/app/core/src/transport/labels.rs @@ -87,7 +87,7 @@ impl FmtLabels for TlsStatus { write!(f, "tls=\"no_identity\",no_tls_reason=\"{}\"", why) } Conditional::Some(ref id) => { - write!(f, "tls=\"true\"")?; + write!(f, "tls=\"true\",")?; id.fmt_labels(f) } } From b1bc961f4ed4343accc8eeea72d5f3b426f25c6e Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Thu, 1 Oct 2020 12:43:37 -0700 Subject: [PATCH 3/3] whoops, fix both being client ids Signed-off-by: Eliza Weisman --- linkerd/app/core/src/transport/labels.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/linkerd/app/core/src/transport/labels.rs b/linkerd/app/core/src/transport/labels.rs index 86ec3be5e7..d8e69dedcd 100644 --- a/linkerd/app/core/src/transport/labels.rs +++ b/linkerd/app/core/src/transport/labels.rs @@ -49,7 +49,7 @@ impl TlsStatus { } pub fn server(id: tls::PeerIdentity) -> Self { - Self(id.map(TlsId::ClientId)) + Self(id.map(TlsId::ServerId)) } }