feat(inbound): include srv_port label in server metrics

olix0r · olix0r · commit 154ef433a062 · 2025-03-09T18:07:42.000Z
We include a group/version/kind for inbound server resources, but we do not
indicate which specific port the server is applied to. This is important context
to understand the inbound proxy's behavior.

This change adds a `srv_port` label to inbound server metrics.
diff --git a/linkerd/app/core/src/metrics.rs b/linkerd/app/core/src/metrics.rs
@@ -73,7 +73,7 @@ pub struct InboundEndpointLabels {
 
 /// A label referencing an inbound `Server` (i.e. for policy).
 #[derive(Clone, Debug, Eq, PartialEq, Hash)]
-pub struct ServerLabel(pub Arc<policy::Meta>);
+pub struct ServerLabel(pub Arc<policy::Meta>, pub u16);
 
 /// Labels referencing an inbound server and authorization.
 #[derive(Clone, Debug, Eq, PartialEq, Hash)]
@@ -331,10 +331,11 @@ impl FmtLabels for ServerLabel {
     fn fmt_labels(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         write!(
             f,
-            "srv_group=\"{}\",srv_kind=\"{}\",srv_name=\"{}\"",
+            "srv_group=\"{}\",srv_kind=\"{}\",srv_name=\"{}\",srv_port=\"{}\"",
             self.0.group(),
             self.0.kind(),
-            self.0.name()
+            self.0.name(),
+            self.1
         )
     }
 }
@@ -351,6 +352,7 @@ impl EncodeLabelSetMut for ServerLabel {
         ("srv_group", self.0.group()).encode(enc.encode_label())?;
         ("srv_kind", self.0.kind()).encode(enc.encode_label())?;
         ("srv_name", self.0.name()).encode(enc.encode_label())?;
+        ("srv_port", self.1).encode(enc.encode_label())?;
         Ok(())
     }
 }
diff --git a/linkerd/app/core/src/transport/labels.rs b/linkerd/app/core/src/transport/labels.rs
@@ -199,18 +199,21 @@ mod tests {
                 negotiated_protocol: None,
             }),
             ([192, 0, 2, 4], 40000).into(),
-            PolicyServerLabel(Arc::new(Meta::Resource {
-                group: "policy.linkerd.io".into(),
-                kind: "server".into(),
-                name: "testserver".into(),
-            })),
+            PolicyServerLabel(
+                Arc::new(Meta::Resource {
+                    group: "policy.linkerd.io".into(),
+                    kind: "server".into(),
+                    name: "testserver".into(),
+                }),
+                40000,
+            ),
         );
         assert_eq!(
             labels.to_string(),
             "direction=\"inbound\",peer=\"src\",\
             target_addr=\"192.0.2.4:40000\",target_ip=\"192.0.2.4\",target_port=\"40000\",\
             tls=\"true\",client_id=\"foo.id.example.com\",\
-            srv_group=\"policy.linkerd.io\",srv_kind=\"server\",srv_name=\"testserver\""
+            srv_group=\"policy.linkerd.io\",srv_kind=\"server\",srv_name=\"testserver\",srv_port=\"40000\""
         );
     }
 }
diff --git a/linkerd/app/gateway/src/http/tests.rs b/linkerd/app/gateway/src/http/tests.rs
@@ -62,7 +62,7 @@ async fn upgraded_request_remains_relative_form() {
 
     impl svc::Param<ServerLabel> for Target {
         fn param(&self) -> ServerLabel {
-            ServerLabel(policy::Meta::new_default("test"))
+            ServerLabel(policy::Meta::new_default("test"), 4143)
         }
     }
 
diff --git a/linkerd/app/inbound/src/http.rs b/linkerd/app/inbound/src/http.rs
@@ -238,11 +238,14 @@ pub mod fuzz {
 
     impl svc::Param<policy::ServerLabel> for Target {
         fn param(&self) -> policy::ServerLabel {
-            policy::ServerLabel(Arc::new(policy::Meta::Resource {
-                group: "policy.linkerd.io".into(),
-                kind: "server".into(),
-                name: "testsrv".into(),
-            }))
+            policy::ServerLabel(
+                Arc::new(policy::Meta::Resource {
+                    group: "policy.linkerd.io".into(),
+                    kind: "server".into(),
+                    name: "testsrv".into(),
+                }),
+                1000,
+            )
         }
     }
 
diff --git a/linkerd/app/inbound/src/http/tests.rs b/linkerd/app/inbound/src/http/tests.rs
@@ -655,11 +655,14 @@ async fn grpc_response_class() {
                 target_addr: "127.0.0.1:80".parse().unwrap(),
                 policy: metrics::RouteAuthzLabels {
                     route: metrics::RouteLabels {
-                        server: metrics::ServerLabel(Arc::new(policy::Meta::Resource {
-                            group: "policy.linkerd.io".into(),
-                            kind: "server".into(),
-                            name: "testsrv".into(),
-                        })),
+                        server: metrics::ServerLabel(
+                            Arc::new(policy::Meta::Resource {
+                                group: "policy.linkerd.io".into(),
+                                kind: "server".into(),
+                                name: "testsrv".into(),
+                            }),
+                            80,
+                        ),
                         route: policy::Meta::new_default("default"),
                     },
                     authz: Arc::new(policy::Meta::Resource {
@@ -889,11 +892,14 @@ impl svc::Param<policy::AllowPolicy> for Target {
 
 impl svc::Param<policy::ServerLabel> for Target {
     fn param(&self) -> policy::ServerLabel {
-        policy::ServerLabel(Arc::new(policy::Meta::Resource {
-            group: "policy.linkerd.io".into(),
-            kind: "server".into(),
-            name: "testsrv".into(),
-        }))
+        policy::ServerLabel(
+            Arc::new(policy::Meta::Resource {
+                group: "policy.linkerd.io".into(),
+                kind: "server".into(),
+                name: "testsrv".into(),
+            }),
+            80,
+        )
     }
 }
 
diff --git a/linkerd/app/inbound/src/policy.rs b/linkerd/app/inbound/src/policy.rs
@@ -133,7 +133,7 @@ impl AllowPolicy {
 
     #[inline]
     pub fn server_label(&self) -> ServerLabel {
-        ServerLabel(self.server.borrow().meta.clone())
+        ServerLabel(self.server.borrow().meta.clone(), self.dst.port())
     }
 
     pub fn ratelimit_label(&self, error: &RateLimitError) -> HTTPLocalRateLimitLabels {
@@ -220,7 +220,7 @@ impl ServerPermit {
             protocol: server.protocol.clone(),
             labels: ServerAuthzLabels {
                 authz: authz.meta.clone(),
-                server: ServerLabel(server.meta.clone()),
+                server: ServerLabel(server.meta.clone(), dst.port()),
             },
         }
     }
diff --git a/linkerd/app/inbound/src/policy/tcp/tests.rs b/linkerd/app/inbound/src/policy/tcp/tests.rs
@@ -43,11 +43,14 @@ async fn unauthenticated_allowed() {
                     kind: "serverauthorization".into(),
                     name: "unauth".into()
                 }),
-                server: ServerLabel(Arc::new(Meta::Resource {
-                    group: "policy.linkerd.io".into(),
-                    kind: "server".into(),
-                    name: "test".into()
-                }))
+                server: ServerLabel(
+                    Arc::new(Meta::Resource {
+                        group: "policy.linkerd.io".into(),
+                        kind: "server".into(),
+                        name: "test".into()
+                    }),
+                    1000
+                )
             },
         }
     );
@@ -96,11 +99,14 @@ async fn authenticated_identity() {
                     kind: "serverauthorization".into(),
                     name: "tls-auth".into()
                 }),
-                server: ServerLabel(Arc::new(Meta::Resource {
-                    group: "policy.linkerd.io".into(),
-                    kind: "server".into(),
-                    name: "test".into()
-                }))
+                server: ServerLabel(
+                    Arc::new(Meta::Resource {
+                        group: "policy.linkerd.io".into(),
+                        kind: "server".into(),
+                        name: "test".into()
+                    }),
+                    1000
+                )
             }
         }
     );
@@ -159,11 +165,14 @@ async fn authenticated_suffix() {
                     kind: "serverauthorization".into(),
                     name: "tls-auth".into()
                 }),
-                server: ServerLabel(Arc::new(Meta::Resource {
-                    group: "policy.linkerd.io".into(),
-                    kind: "server".into(),
-                    name: "test".into()
-                })),
+                server: ServerLabel(
+                    Arc::new(Meta::Resource {
+                        group: "policy.linkerd.io".into(),
+                        kind: "server".into(),
+                        name: "test".into()
+                    }),
+                    1000
+                ),
             }
         }
     );
@@ -219,11 +228,14 @@ async fn tls_unauthenticated() {
                     kind: "serverauthorization".into(),
                     name: "tls-unauth".into()
                 }),
-                server: ServerLabel(Arc::new(Meta::Resource {
-                    group: "policy.linkerd.io".into(),
-                    kind: "server".into(),
-                    name: "test".into()
-                })),
+                server: ServerLabel(
+                    Arc::new(Meta::Resource {
+                        group: "policy.linkerd.io".into(),
+                        kind: "server".into(),
+                        name: "test".into()
+                    }),
+                    1000
+                ),
             }
         }
     );

Original file line number	Diff line number	Diff line change
`@@ -73,7 +73,7 @@ pub struct InboundEndpointLabels {`
`73`	`73`
`74`	`74`	/// A label referencing an inbound `Server` (i.e. for policy).
`75`	`75`	`#[derive(Clone, Debug, Eq, PartialEq, Hash)]`
`76`		`-pub struct ServerLabel(pub Arc<policy::Meta>);`
	`76`	`+pub struct ServerLabel(pub Arc<policy::Meta>, pub u16);`
`77`	`77`
`78`	`78`	`/// Labels referencing an inbound server and authorization.`
`79`	`79`	`#[derive(Clone, Debug, Eq, PartialEq, Hash)]`
`@@ -331,10 +331,11 @@ impl FmtLabels for ServerLabel {`
`331`	`331`	`fn fmt_labels(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {`
`332`	`332`	`write!(`
`333`	`333`	`f,`
`334`		`- "srv_group=\"{}\",srv_kind=\"{}\",srv_name=\"{}\"",`
	`334`	`+ "srv_group=\"{}\",srv_kind=\"{}\",srv_name=\"{}\",srv_port=\"{}\"",`
`335`	`335`	`self.0.group(),`
`336`	`336`	`self.0.kind(),`
`337`		`- self.0.name()`
	`337`	`+ self.0.name(),`
	`338`	`+ self.1`
`338`	`339`	`)`
`339`	`340`	`}`
`340`	`341`	`}`
`@@ -351,6 +352,7 @@ impl EncodeLabelSetMut for ServerLabel {`
`351`	`352`	`("srv_group", self.0.group()).encode(enc.encode_label())?;`
`352`	`353`	`("srv_kind", self.0.kind()).encode(enc.encode_label())?;`
`353`	`354`	`("srv_name", self.0.name()).encode(enc.encode_label())?;`
	`355`	`+ ("srv_port", self.1).encode(enc.encode_label())?;`
`354`	`356`	`Ok(())`
`355`	`357`	`}`
`356`	`358`	`}`
Original file line number	Diff line number	Diff line change
`@@ -62,7 +62,7 @@ async fn upgraded_request_remains_relative_form() {`
`62`	`62`
`63`	`63`	`impl svc::Param<ServerLabel> for Target {`
`64`	`64`	`fn param(&self) -> ServerLabel {`
`65`		`- ServerLabel(policy::Meta::new_default("test"))`
	`65`	`+ ServerLabel(policy::Meta::new_default("test"), 4143)`
`66`	`66`	`}`
`67`	`67`	`}`
`68`	`68`
Original file line number	Diff line number	Diff line change
`@@ -133,7 +133,7 @@ impl AllowPolicy {`
`133`	`133`
`134`	`134`	`#[inline]`
`135`	`135`	`pub fn server_label(&self) -> ServerLabel {`
`136`		`- ServerLabel(self.server.borrow().meta.clone())`
	`136`	`+ ServerLabel(self.server.borrow().meta.clone(), self.dst.port())`
`137`	`137`	`}`
`138`	`138`
`139`	`139`	`pub fn ratelimit_label(&self, error: &RateLimitError) -> HTTPLocalRateLimitLabels {`
`@@ -220,7 +220,7 @@ impl ServerPermit {`
`220`	`220`	`protocol: server.protocol.clone(),`
`221`	`221`	`labels: ServerAuthzLabels {`
`222`	`222`	`authz: authz.meta.clone(),`
`223`		`- server: ServerLabel(server.meta.clone()),`
	`223`	`+ server: ServerLabel(server.meta.clone(), dst.port()),`
`224`	`224`	`},`
`225`	`225`	`}`
`226`	`226`	`}`