From 82b55ce1f22ca75ca96cf19da19cc5590acd2e42 Mon Sep 17 00:00:00 2001
From: Bruce Chen <wenliang.chen@personio.de>
Date: Thu, 14 Sep 2023 16:37:37 +0200
Subject: [PATCH] feat: format prefix labels

---
 linkerd/app/core/src/metrics.rs               | 17 ++++++++++
 .../app/integration/src/tests/telemetry.rs    | 32 ++++++++++++++++---
 linkerd/app/outbound/src/http/concrete.rs     | 22 ++-----------
 3 files changed, 48 insertions(+), 23 deletions(-)

diff --git a/linkerd/app/core/src/metrics.rs b/linkerd/app/core/src/metrics.rs
index 5f937ea836..21b47402cc 100644
--- a/linkerd/app/core/src/metrics.rs
+++ b/linkerd/app/core/src/metrics.rs
@@ -135,6 +135,23 @@ where
     Some(out)
 }
 
+pub fn prefix_outbound_endpoint_labels<'i, I>(prefix: &str, mut labels_iter: I) -> Option<String>
+where
+    I: Iterator<Item = (&'i String, &'i String)>,
+{
+    let (k0, v0) = labels_iter.next()?;
+    let mut out = format!("{}_{}=\"{}\"", prefix, k0, v0);
+
+    for (k, v) in labels_iter {
+        if k == "pod" || k == "pod_template_hash" {
+            continue;
+        }
+
+        write!(out, ",{}_{}=\"{}\"", prefix, k, v).expect("label concat must succeed");
+    }
+    Some(out)
+}
+
 // === impl Metrics ===
 
 impl Metrics {
diff --git a/linkerd/app/integration/src/tests/telemetry.rs b/linkerd/app/integration/src/tests/telemetry.rs
index ca8a46cd58..b018ce75f4 100644
--- a/linkerd/app/integration/src/tests/telemetry.rs
+++ b/linkerd/app/integration/src/tests/telemetry.rs
@@ -563,12 +563,12 @@ mod outbound_dst_labels {
         let (
             Fixture {
                 client,
-                metrics: _metrics,
+                metrics,
                 proxy: _proxy,
                 _profile,
                 dst_tx,
                 pol_out_tx: _pol_out_tx,
-                labels: _labels,
+                labels,
                 ..
             },
             addr,
@@ -582,6 +582,18 @@ mod outbound_dst_labels {
 
         info!("client.get(/)");
         assert_eq!(client.get("/").await, "hello");
+
+        let labels = labels
+            .label("dst_addr_label1", "foo")
+            .label("dst_addr_label2", "bar");
+
+        for &metric in &[
+            "request_total",
+            "response_total",
+            "response_latency_ms_count",
+        ] {
+            labels.metric(metric).assert_in(&metrics).await;
+        }
     }
 
     #[tokio::test]
@@ -590,11 +602,11 @@ mod outbound_dst_labels {
         let (
             Fixture {
                 client,
-                metrics: _metrics,
+                metrics,
                 proxy: _proxy,
                 _profile,
                 dst_tx,
-                labels: _labels,
+                labels,
                 ..
             },
             addr,
@@ -614,6 +626,18 @@ mod outbound_dst_labels {
 
         info!("client.get(/)");
         assert_eq!(client.get("/").await, "hello");
+
+        let labels = labels
+            .label("dst_set_label1", "foo")
+            .label("dst_set_label2", "bar");
+
+        for &metric in &[
+            "request_total",
+            "response_total",
+            "response_latency_ms_count",
+        ] {
+            labels.metric(metric).assert_in(&metrics).await;
+        }
     }
 
     #[tokio::test]
diff --git a/linkerd/app/outbound/src/http/concrete.rs b/linkerd/app/outbound/src/http/concrete.rs
index 3e0bc535e7..49e39c7052 100644
--- a/linkerd/app/outbound/src/http/concrete.rs
+++ b/linkerd/app/outbound/src/http/concrete.rs
@@ -5,7 +5,7 @@ use super::{balance, breaker, client, handle_proxy_error_headers};
 use crate::{http, stack_labels, BackendRef, Outbound, ParentRef};
 use linkerd_app_core::{
     classify,
-    metrics::{prefix_labels, EndpointLabels, OutboundEndpointLabels},
+    metrics::{prefix_outbound_endpoint_labels, EndpointLabels, OutboundEndpointLabels},
     profiles,
     proxy::{
         api_resolve::{ConcreteAddr, Metadata, ProtocolHint},
@@ -18,7 +18,7 @@ use linkerd_app_core::{
     Error, Infallible, NameAddr,
 };
 use linkerd_proxy_client_policy::FailureAccrual;
-use std::{collections::BTreeMap, fmt::Debug, net::SocketAddr, sync::Arc};
+use std::{fmt::Debug, net::SocketAddr, sync::Arc};
 use tracing::info_span;
 
 mod metrics;
@@ -342,24 +342,8 @@ where
     T: svc::Param<Option<http::uri::Authority>>,
 {
     fn param(&self) -> OutboundEndpointLabels {
-        let original_labels = self.metadata.labels().clone();
-        // self.metadata.labels() could return Err in some cases
-        // if that case the dst_labels won't carry any value
-        let dst_labels = match Arc::try_unwrap(self.metadata.labels()) {
-            Ok(result) => result,
-            Err(_e) => BTreeMap::new(),
-        };
-
-        // dst_labels.remove("pod");
-        // dst_labels.remove("pod_template_hash");
-
-        let label_iterator = match dst_labels.is_empty() {
-            true => dst_labels.iter(),
-            false => original_labels.iter()
-        };
-
         OutboundEndpointLabels {
-            labels: prefix_labels("dst", label_iterator),
+            labels: prefix_outbound_endpoint_labels("dst", self.metadata.labels().iter()),
             server_id: self.param(),
         }
     }