metrics: Add endpoint labels to outbound TCP metrics (#654)

TCP metrics are pretty terse. Now that we're doing discovery for TCP forwards,
it seems appropriate to expose endpoint labels on TCP metrics (at least to
determine if/when mTLS is being applied).

Author: olix0r

linkerd/app/core/src/transport/labels.rs

@@ -1,4 +1,5 @@
 use super::tls;
+pub use crate::metric_labels::{Direction, EndpointLabels};
 use linkerd2_conditional::Conditional;
 use linkerd2_metrics::FmtLabels;
 use std::fmt;
@@ -8,68 +9,28 @@ use std::fmt;
 /// A `Metrics` type exists for each unique `Key`.
 ///
 /// Implements `FmtLabels`.
-#[derive(Copy, Clone, Debug, Eq, PartialEq, Hash)]
-pub struct Key {
-    direction: Direction,
-    peer: Peer,
-    tls_status: TlsStatus,
-}
-
-#[derive(Copy, Clone, Debug, Eq, PartialEq, Hash)]
-struct Direction(&'static str);
-
-#[derive(Copy, Clone, Debug, Eq, PartialEq, Hash)]
-enum Peer {
-    /// Represents the side of the proxy that accepts connections.
-    Src,
-    /// Represents the side of the proxy that opens connections.
-    Dst,
+#[derive(Clone, Debug, Eq, PartialEq, Hash)]
+pub enum Key {
+    Accept(Direction, TlsStatus),
+    Connect(EndpointLabels),
 }
 
 #[derive(Copy, Clone, Debug, Eq, PartialEq, Hash)]
 pub struct TlsStatus(tls::Conditional<()>);
 
-// ===== impl Key =====
-
-impl Key {
-    pub fn accept<T>(direction: &'static str, tls: tls::Conditional<T>) -> Self {
-        Self {
-            direction: Direction(direction),
-            tls_status: TlsStatus(tls.map(|_| ())),
-            peer: Peer::Src,
-        }
-    }
-
-    pub fn connect<T>(direction: &'static str, tls: tls::Conditional<T>) -> Self {
-        Self {
-            direction: Direction(direction),
-            tls_status: TlsStatus(tls.map(|_| ())),
-            peer: Peer::Dst,
-        }
-    }
-}
+// === impl Key ===
 
 impl FmtLabels for Key {
-    fn fmt_labels(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        ((self.direction, self.peer), self.tls_status).fmt_labels(f)
-    }
-}
-
-// ===== impl Peer =====
-
-impl FmtLabels for Direction {
-    fn fmt_labels(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        write!(f, "direction=\"{}\"", self.0)
-    }
-}
-
-// ===== impl Peer =====
-
-impl FmtLabels for Peer {
     fn fmt_labels(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match self {
-            Peer::Src => f.pad("peer=\"src\""),
-            Peer::Dst => f.pad("peer=\"dst\""),
+            Self::Accept(direction, identity) => {
+                write!(f, "peer=\"src\",")?;
+                (direction, identity).fmt_labels(f)
+            }
+            Self::Connect(labels) => {
+                write!(f, "peer=\"dst\",")?;
+                labels.fmt_labels(f)
+            }
         }
     }
 }

linkerd/app/inbound/src/lib.rs

@@ -409,31 +409,35 @@ impl transport::metrics::TransportLabels<HttpEndpoint> for TransportLabels {
     type Labels = transport::labels::Key;
 
     fn transport_labels(&self, _: &HttpEndpoint) -> Self::Labels {
-        transport::labels::Key::connect::<()>(
-            "inbound",
-            tls::Conditional::None(tls::ReasonForNoPeerName::Loopback.into()),
-        )
+        transport::labels::Key::Connect(transport::labels::EndpointLabels {
+            direction: transport::labels::Direction::In,
+            authority: None,
+            labels: None,
+            tls_id: tls::Conditional::None(tls::ReasonForNoPeerName::Loopback.into()).into(),
+        })
     }
 }
 
 impl transport::metrics::TransportLabels<TcpEndpoint> for TransportLabels {
     type Labels = transport::labels::Key;
 
     fn transport_labels(&self, _: &TcpEndpoint) -> Self::Labels {
-        transport::labels::Key::connect::<()>(
-            "inbound",
-            tls::Conditional::None(tls::ReasonForNoPeerName::Loopback.into()),
-        )
+        transport::labels::Key::Connect(transport::labels::EndpointLabels {
+            direction: transport::labels::Direction::In,
+            authority: None,
+            labels: None,
+            tls_id: tls::Conditional::None(tls::ReasonForNoPeerName::Loopback.into()).into(),
+        })
     }
 }
 
 impl transport::metrics::TransportLabels<listen::Addrs> for TransportLabels {
     type Labels = transport::labels::Key;
 
     fn transport_labels(&self, _: &listen::Addrs) -> Self::Labels {
-        transport::labels::Key::accept::<()>(
-            "inbound",
-            tls::Conditional::None(tls::ReasonForNoPeerName::PortSkipped),
+        transport::labels::Key::Accept(
+            transport::labels::Direction::In,
+            tls::Conditional::<()>::None(tls::ReasonForNoPeerName::PortSkipped).into(),
         )
     }
 }
@@ -442,7 +446,10 @@ impl transport::metrics::TransportLabels<tls::accept::Meta> for TransportLabels
     type Labels = transport::labels::Key;
 
     fn transport_labels(&self, target: &tls::accept::Meta) -> Self::Labels {
-        transport::labels::Key::accept("inbound", target.peer_identity.as_ref())
+        transport::labels::Key::Accept(
+            transport::labels::Direction::In,
+            target.peer_identity.as_ref().into(),
+        )
     }
 }
 

linkerd/app/integration/tests/discovery.rs

@@ -591,7 +591,7 @@ mod http2 {
         // Wait until the proxy has seen the `srv1` disconnect...
         assert_eventually_contains!(
             metrics.get("/metrics").await,
-            "tcp_close_total{direction=\"outbound\",peer=\"dst\",tls=\"no_identity\",no_tls_reason=\"not_provided_by_service_discovery\",errno=\"\"} 1"
+            "tcp_close_total{peer=\"dst\",authority=\"disco.test.svc.cluster.local\",direction=\"outbound\",tls=\"no_identity\",no_tls_reason=\"not_provided_by_service_discovery\",errno=\"\"} 1"
         );
 
         // Start a new request to the destination, now that the server is dead.