From 6de2884969643c422661d75712a162a87f38036a Mon Sep 17 00:00:00 2001
From: Yueh-Hsuan Chiang <93241502+yhchiang-sol@users.noreply.github.com>
Date: Thu, 26 May 2022 17:00:58 -0700
Subject: [PATCH] Extend the datapoint macro to support group-by operations.
 (#25385)

#### Problem
Our existing datapoint macro syntax does directly not support group-by tags.
The existing workaround is to embed the group-by tags into the metric name which does not scale well.

#### Summary of Changes
This PR extends the existing syntax to support group-by tags as follows.  The new syntax is also compatible with the existing syntax:
```
datapoint_debug!(
   "metric_name",
   "tag" => "tag-value",
   "tag2" => "tag-value2",
   ....
   ("field1", 100, i64),  // field syntax is the same as the current syntax.
   ("field2", "hello", String),
   ...
);
```
---
 metrics/src/datapoint.rs | 99 ++++++++++++++++++++++++++++++++++++++++
 metrics/src/metrics.rs   |  6 +++
 2 files changed, 105 insertions(+)

diff --git a/metrics/src/datapoint.rs b/metrics/src/datapoint.rs
index 807e52963a..eaa0a732b7 100644
--- a/metrics/src/datapoint.rs
+++ b/metrics/src/datapoint.rs
@@ -12,6 +12,12 @@
 //! The matric macro consists of the following three main parts:
 //!  - name: the name of the metric.
 //!
+//!  - tags (optional): when a metric sample is reported with tags, you can use
+//!    group-by when querying the reported samples.  Each metric sample can be
+//!    attached with zero to many tags.  Each tag is of the format:
+//!
+//!    - "tag-name" => "tag-value"
+//!
 //!  - fields (optional): fields are the main content of a metric sample. The
 //!    macro supports four different types of fields: bool, i64, f64, and String.
 //!    Here're their syntax:
@@ -25,6 +31,8 @@
 //!
 //! datapoint_debug!(
 //!     "name-of-the-metric",
+//!     "tag" => "tag-value",
+//!     "tag2" => "tag-value2",
 //!     ("some-bool", false, bool),
 //!     ("some-int", 100, i64),
 //!     ("some-float", 1.05, f64),
@@ -37,6 +45,8 @@ use std::{fmt, time::SystemTime};
 pub struct DataPoint {
     pub name: &'static str,
     pub timestamp: SystemTime,
+    /// tags are eligible for group-by operations.
+    pub tags: Vec<(&'static str, &'static str)>,
     pub fields: Vec<(&'static str, String)>,
 }
 
@@ -45,10 +55,16 @@ impl DataPoint {
         DataPoint {
             name,
             timestamp: SystemTime::now(),
+            tags: vec![],
             fields: vec![],
         }
     }
 
+    pub fn add_tag(&mut self, name: &'static str, value: &'static str) -> &mut Self {
+        self.tags.push((name, value));
+        self
+    }
+
     pub fn add_field_str(&mut self, name: &'static str, value: &str) -> &mut Self {
         self.fields
             .push((name, format!("\"{}\"", value.replace('\"', "\\\""))));
@@ -74,6 +90,9 @@ impl DataPoint {
 impl fmt::Display for DataPoint {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         write!(f, "datapoint: {}", self.name)?;
+        for tag in &self.tags {
+            write!(f, ",{}={}", tag.0, tag.1)?;
+        }
         for field in &self.fields {
             write!(f, " {}={}", field.0, field.1)?;
         }
@@ -95,8 +114,22 @@ macro_rules! create_datapoint {
     (@field $point:ident $name:expr, $value:expr, bool) => {
         $point.add_field_bool($name, $value as bool);
     };
+    (@tag $point:ident $tag_name:expr, $tag_value:expr) => {
+        $point.add_tag($tag_name, &$tag_value);
+    };
 
     (@fields $point:ident) => {};
+
+    // process tags
+    (@fields $point:ident $tag_name:expr => $tag_value:expr, $($rest:tt)*) => {
+        $crate::create_datapoint!(@tag $point $tag_name, $tag_value);
+        $crate::create_datapoint!(@fields $point $($rest)*);
+    };
+    (@fields $point:ident $tag_name:expr => $tag_value:expr) => {
+        $crate::create_datapoint!(@tag $point $tag_name, $tag_value);
+    };
+
+    // process fields
     (@fields $point:ident ($name:expr, $value:expr, $type:ident) , $($rest:tt)*) => {
         $crate::create_datapoint!(@field $point $name, $value, $type);
         $crate::create_datapoint!(@fields $point $($rest)*);
@@ -218,6 +251,7 @@ mod test {
             ("bool", true, bool)
         );
         assert_eq!(point.name, "name");
+        assert_eq!(point.tags.len(), 0);
         assert_eq!(point.fields[0], ("i64", "1i".to_string()));
         assert_eq!(
             point.fields[1],
@@ -226,4 +260,69 @@ mod test {
         assert_eq!(point.fields[2], ("f64", "12.34".to_string()));
         assert_eq!(point.fields[3], ("bool", "true".to_string()));
     }
+
+    #[test]
+    fn test_datapoint_with_tags() {
+        datapoint_debug!("name", "tag" => "tag-value", ("field name", "test", String));
+        datapoint_info!(
+            "name",
+            "tag" => "tag-value",
+            "tag2" => "tag-value-2",
+            ("field name", 12.34_f64, f64)
+        );
+        datapoint_trace!(
+            "name",
+            "tag" => "tag-value",
+            "tag2" => "tag-value-2",
+            "tag3" => "tag-value-3",
+            ("field name", true, bool)
+        );
+        datapoint_warn!("name", "tag" => "tag-value");
+        datapoint_error!("name", "tag" => "tag-value", ("field name", 1, i64),);
+        datapoint!(
+            log::Level::Warn,
+            "name",
+            "tag" => "tag-value",
+            ("field1 name", 2, i64),
+            ("field2 name", 2, i64)
+        );
+        datapoint_info!("name", ("field1 name", 2, i64), ("field2 name", 2, i64),);
+        datapoint_trace!(
+            "name",
+            "tag" => "tag-value",
+            ("field1 name", 2, i64),
+            ("field2 name", 2, i64),
+            ("field3 name", 3, i64)
+        );
+        datapoint!(
+            log::Level::Error,
+            "name",
+            "tag" => "tag-value",
+            ("field1 name", 2, i64),
+            ("field2 name", 2, i64),
+            ("field3 name", 3, i64),
+        );
+
+        let point = create_datapoint!(
+            @point "name",
+            "tag1" => "tag-value-1",
+            "tag2" => "tag-value-2",
+            "tag3" => "tag-value-3",
+            ("i64", 1, i64),
+            ("String", "string space string", String),
+            ("f64", 12.34_f64, f64),
+            ("bool", true, bool)
+        );
+        assert_eq!(point.name, "name");
+        assert_eq!(point.fields[0], ("i64", "1i".to_string()));
+        assert_eq!(
+            point.fields[1],
+            ("String", "\"string space string\"".to_string())
+        );
+        assert_eq!(point.fields[2], ("f64", "12.34".to_string()));
+        assert_eq!(point.fields[3], ("bool", "true".to_string()));
+        assert_eq!(point.tags[0], ("tag1", "tag-value-1"));
+        assert_eq!(point.tags[1], ("tag2", "tag-value-2"));
+        assert_eq!(point.tags[2], ("tag3", "tag-value-3"));
+    }
 }
diff --git a/metrics/src/metrics.rs b/metrics/src/metrics.rs
index 48c97101f4..1dfbb5e5ab 100644
--- a/metrics/src/metrics.rs
+++ b/metrics/src/metrics.rs
@@ -87,6 +87,9 @@ pub fn serialize_points(points: &Vec<DataPoint>, host_id: &str) -> String {
         for (name, value) in &point.fields {
             len += name.len() + value.len() + EXTRA_LEN;
         }
+        for (name, value) in &point.tags {
+            len += name.len() + value.len() + EXTRA_LEN;
+        }
         len += point.name.len();
         len += TIMESTAMP_LEN;
         len += host_id.len() + HOST_ID_LEN;
@@ -94,6 +97,9 @@ pub fn serialize_points(points: &Vec<DataPoint>, host_id: &str) -> String {
     let mut line = String::with_capacity(len);
     for point in points {
         let _ = write!(line, "{},host_id={}", &point.name, host_id);
+        for (name, value) in point.tags.iter() {
+            let _ = write!(line, ",{}={}", name, value);
+        }
 
         let mut first = true;
         for (name, value) in point.fields.iter() {