Merge branch 'main' into dsn/fuzz-profile-api

bin_tests/src/bin/crashtracker_bin_test.rs

@@ -18,7 +18,7 @@ mod unix {
         self as crashtracker, CrashtrackerConfiguration, CrashtrackerMetadata,
         CrashtrackerReceiverConfig,
     };
-    use datadog_profiling::exporter::Tag;
+    use ddcommon::tag;
 
     #[inline(never)]
     unsafe fn deref_ptr(p: *mut u8) {
@@ -62,10 +62,10 @@ mod unix {
                 profiling_library_version: "1.0.0".to_owned(),
                 family: "native".to_owned(),
                 tags: vec![
-                    Tag::new("service", "foo").unwrap(),
-                    Tag::new("service_version", "bar").unwrap(),
-                    Tag::new("runtime-id", "xyz").unwrap(),
-                    Tag::new("language", "native").unwrap(),
+                    tag!("service", "foo"),
+                    tag!("service_version", "bar"),
+                    tag!("runtime-id", "xyz"),
+                    tag!("language", "native"),
                 ],
             },
         )?;

crashtracker/src/api.rs

@@ -110,7 +110,7 @@ fn test_crash() {
     use crate::{begin_profiling_op, StacktraceCollection};
     use chrono::Utc;
     use ddcommon::parse_uri;
-    use ddcommon::tag::Tag;
+    use ddcommon::tag;
     use ddcommon::Endpoint;
     use std::time::Duration;
 
@@ -152,7 +152,7 @@ fn test_crash() {
     init(config, receiver_config, metadata).expect("not to fail");
     begin_profiling_op(crate::ProfilingOpTypes::CollectingSample).expect("Not to fail");
 
-    let tag = Tag::new("apple", "banana").expect("tag");
+    let tag = tag!("apple", "banana");
     let metadata2 = CrashtrackerMetadata::new(
         "libname".to_string(),
         "version".to_string(),

crashtracker/src/telemetry.rs

@@ -196,7 +196,7 @@ mod tests {
 
     use crate::SigInfo;
     use chrono::DateTime;
-    use ddcommon::{tag::Tag, Endpoint};
+    use ddcommon::{tag, Endpoint};
 
     use super::TelemetryCrashUploader;
 
@@ -223,10 +223,10 @@ mod tests {
             profiling_library_version: "1.0.0".to_owned(),
             family: "native".to_owned(),
             tags: vec![
-                Tag::new("service", "foo").unwrap(),
-                Tag::new("service_version", "bar").unwrap(),
-                Tag::new("runtime-id", "xyz").unwrap(),
-                Tag::new("language", "native").unwrap(),
+                tag!("service", "foo"),
+                tag!("service_version", "bar"),
+                tag!("runtime-id", "xyz"),
+                tag!("language", "native"),
             ],
         }
     }

ddcommon-ffi/src/tags.rs

@@ -42,7 +42,7 @@ pub unsafe extern "C" fn ddog_Vec_Tag_push(
             vec.push(tag);
             PushTagResult::Ok
         }
-        Err(err) => PushTagResult::Err(Error::from(err.as_ref())),
+        Err(err) => PushTagResult::Err(Error::from(err.to_string())),
     }
 }
 

ddcommon/Cargo.toml

@@ -38,6 +38,7 @@ rustls-native-certs = { version = "0.6" }
 tokio = { version = "1.23", features = ["rt", "macros"] }
 tokio-rustls = { version = "0.23" }
 serde = { version = "1.0", features = ["derive"] }
+static_assertions = "1.1.0"
 
 [dev-dependencies]
 indexmap = "2.2"

ddcommon/src/tag.rs

@@ -1,15 +1,72 @@
 // Copyright 2021-Present Datadog, Inc. https://www.datadoghq.com/
 // SPDX-License-Identifier: Apache-2.0
 
+use serde::{Deserialize, Serialize};
 use std::borrow::Cow;
 use std::fmt::{Debug, Display, Formatter};
 
-use serde::{Deserialize, Serialize};
+pub use static_assertions::{const_assert, const_assert_ne};
 
 #[derive(Clone, Eq, PartialEq, Hash, Serialize, Deserialize)]
 #[serde(transparent)]
 pub struct Tag {
-    value: String,
+    /// Many tags are made from literal strings, such as:
+    ///  - "language:native"
+    ///  - "src_library:libdatadog"
+    ///  - "type:timeout"
+    /// So being able to save allocations is nice.
+    value: Cow<'static, str>,
+}
+
+impl Tag {
+    /// Used by the `tag!` macro. Not meant to be used directly, please use
+    /// the macro instead.
+    /// # Safety
+    /// Do not use directly, use through the `tag!` macro which enforces the
+    /// safety invariants at compile time.
+    pub const unsafe fn from_static_unchecked(value: &'static str) -> Self {
+        Self {
+            value: Cow::Borrowed(value),
+        }
+    }
+}
+
+/// Creates a tag from a key and value known at compile-time, and fails to
+/// compile if it's known to be invalid (it may still emit an invalid tag, not
+/// all tag validation is currently done client-side). If the key or value
+/// aren't known at compile-time, then use [Tag::new].
+// todo: what's a good way to keep these in-sync with Tag::from_value?
+// This can be a little more strict because it's compitle-time evaluated.
+// https://docs.datadoghq.com/getting_started/tagging/#define-tags
+#[macro_export]
+macro_rules! tag {
+    ($key:expr, $val:expr) => {{
+        // Keys come in "value" or "key:value" format. This pattern is always
+        // the key:value format, which means the value should not be empty.
+        // todo: the implementation here differs subtly from Tag::from_value,
+        //       which checks that the whole thing doesn't end with a colon.
+        $crate::tag::const_assert!(!$val.is_empty());
+
+        const COMBINED: &'static str = concat!($key, ":", $val);
+
+        // Tags must start with a letter. This is more restrictive than is
+        // required (could be a unicode alphabetic char) and can be lifted
+        // if it's causing problems.
+        $crate::tag::const_assert!(COMBINED.as_bytes()[0].is_ascii_alphabetic());
+
+        // Tags can be up to 200 characters long and support Unicode letters
+        // (which includes most character sets, including languages such as
+        // Japanese).
+        // Presently, engineers interpretted this to be 200 bytes, not unicode
+        // characters. However, if the 200th character is unicode, it's
+        // allowed to spill over due to a historical bug. For now, we'll
+        // ignore this and hard-code 200 bytes.
+        $crate::tag::const_assert!(COMBINED.as_bytes().len() <= 200);
+
+        #[allow(unused_unsafe)]
+        let tag = unsafe { $crate::tag::Tag::from_static_unchecked(COMBINED) };
+        tag
+    }};
 }
 
 impl Debug for Tag {
@@ -32,9 +89,8 @@ impl Display for Tag {
 }
 
 impl Tag {
-    /// It's recommended to use Tag::new when possible, as tags that are in
-    /// the <KEY>:<VALUE> format are preferred.
-    pub fn from_value<'a, IntoCow>(chunk: IntoCow) -> Result<Self, Cow<'static, str>>
+    /// Validates a tag.
+    fn from_value<'a, IntoCow>(chunk: IntoCow) -> anyhow::Result<Self>
     where
         IntoCow: Into<Cow<'a, str>>,
     {
@@ -49,24 +105,22 @@ impl Tag {
          * are likely to be errors (such as passed in empty string).
          */
 
-        if chunk.is_empty() {
-            return Err("tag is empty".into());
-        }
+        anyhow::ensure!(!chunk.is_empty(), "tag is empty");
 
         let mut chars = chunk.chars();
-        if chars.next() == Some(':') {
-            return Err(format!("tag '{chunk}' begins with a colon").into());
-        }
-        if chars.last() == Some(':') {
-            return Err(format!("tag '{chunk}' ends with a colon").into());
-        }
+        anyhow::ensure!(
+            chars.next() != Some(':'),
+            "tag '{chunk}' begins with a colon"
+        );
+        anyhow::ensure!(chars.last() != Some(':'), "tag '{chunk}' ends with a colon");
 
-        Ok(Tag {
-            value: chunk.into_owned(),
-        })
+        let value = Cow::Owned(chunk.into_owned());
+        Ok(Tag { value })
     }
 
-    pub fn new<K, V>(key: K, value: V) -> Result<Self, Cow<'static, str>>
+    /// Creates a tag from a key and value. It's preferred to use the `tag!`
+    /// macro when the key and value are both known at compile-time.
+    pub fn new<K, V>(key: K, value: V) -> anyhow::Result<Self>
     where
         K: AsRef<str>,
         V: AsRef<str>,
@@ -103,7 +157,7 @@ pub fn parse_tags(str: &str) -> (Vec<Tag>, Option<String>) {
                 } else {
                     error_message += ", ";
                 }
-                error_message += err.as_ref();
+                error_message += &err.to_string();
             }
         }
     }
@@ -118,7 +172,16 @@ pub fn parse_tags(str: &str) -> (Vec<Tag>, Option<String>) {
 
 #[cfg(test)]
 mod tests {
-    use super::{parse_tags, Tag};
+    use super::*;
+
+    #[test]
+    fn test_is_send() {
+        // fails to compile if false
+        fn is_send<T: Send>(_t: T) -> bool {
+            true
+        }
+        assert!(is_send(tag!("src_library", "libdatadog")));
+    }
 
     #[test]
     fn test_empty_key() {
@@ -134,20 +197,22 @@ mod tests {
     fn test_bad_utf8() {
         // 0b1111_0xxx is the start of a 4-byte sequence, but there aren't any
         // more chars, so it  will get converted into the utf8 replacement
-        // character. This results in a string with a space (32) and a
-        // replacement char, so it should be an error (no valid chars).
-        // However, we don't enforce many things about tags yet, so we let it
-        // slide.
-        let bytes = &[32, 0b1111_0111];
+        // character. This results in a string with an "a" and a replacement
+        // char, so it should be an error (no valid chars). However, we don't
+        // enforce many things about tags yet client-side, so we let it slide.
+        let bytes = &[b'a', 0b1111_0111];
         let key = String::from_utf8_lossy(bytes);
         let t = Tag::new(key, "value").unwrap();
-        assert_eq!(" \u{FFFD}:value", t.to_string());
+        assert_eq!("a\u{FFFD}:value", t.to_string());
     }
 
     #[test]
     fn test_value_has_colon() {
         let result = Tag::new("env", "staging:east").expect("values can have colons");
         assert_eq!("env:staging:east", result.to_string());
+
+        let result = tag!("env", "staging:east");
+        assert_eq!("env:staging:east", result.to_string());
     }
 
     #[test]
@@ -156,7 +221,7 @@ mod tests {
         // that profile tags will then differ or cause failures compared to
         // trace tags. These require cross-team, cross-language collaboration.
         let cases = [
-            (" begins with non-letter".to_string(), "value"),
+            ("_begins_with_non-letter".to_string(), "value"),
             ("the-tag-length-is-over-200-characters".repeat(6), "value"),
         ];
 

ddtelemetry/examples/tm-metrics-worker-test.rs

@@ -3,7 +3,7 @@
 
 use std::{error::Error, time::Duration, time::Instant};
 
-use ddcommon::tag::Tag;
+use ddcommon::tag;
 use ddtelemetry::{data, worker};
 
 macro_rules! timeit {
@@ -64,7 +64,7 @@ fn main() -> Result<(), Box<dyn Error>> {
     handle.add_point(1.0, &dist_metric, Vec::new()).unwrap();
     handle.add_point(2.0, &dist_metric, Vec::new()).unwrap();
 
-    let tags = vec![Tag::from_value("foo:bar").unwrap()];
+    let tags = vec![tag!("foo", "bar")];
     handle.add_point(2.0, &ping_metric, tags.clone()).unwrap();
     handle.add_point(1.8, &dist_metric, tags).unwrap();
 

ddtelemetry/examples/tm-worker-test.rs

@@ -6,7 +6,7 @@ use std::{
     time::{Duration, Instant},
 };
 
-use ddcommon::tag::Tag;
+use ddcommon::tag;
 use ddtelemetry::{data, worker};
 
 macro_rules! timeit {
@@ -67,7 +67,7 @@ fn main() -> Result<(), Box<dyn Error>> {
     handle.add_point(1.0, &dist_metric, Vec::new()).unwrap();
     handle.add_point(2.0, &dist_metric, Vec::new()).unwrap();
 
-    let tags = vec![Tag::from_value("foo:bar").unwrap()];
+    let tags = vec![tag!("foo", "bar")];
     handle.add_point(2.0, &ping_metric, tags.clone()).unwrap();
     handle.add_point(1.8, &dist_metric, tags).unwrap();
 

ddtelemetry/src/metrics.rs

@@ -220,6 +220,7 @@ impl MetricContexts {
 
 #[cfg(test)]
 mod tests {
+    use ddcommon::tag;
     use std::fmt::Debug;
 
     use super::*;
@@ -279,7 +280,7 @@ mod tests {
             false,
             MetricNamespace::Tracers,
         );
-        let extra_tags = vec![Tag::from_value("service:foobar").unwrap()];
+        let extra_tags = vec![tag!("service", "foobar")];
 
         buckets.add_point(context_key_1, 0.1, Vec::new());
         buckets.add_point(context_key_1, 0.2, Vec::new());

profiling-ffi/src/exporter.rs

@@ -461,6 +461,7 @@ pub unsafe extern "C" fn ddog_CancellationToken_drop(token: Option<&mut Cancella
 #[cfg(test)]
 mod tests {
     use super::*;
+    use ddcommon::tag;
     use ddcommon_ffi::Slice;
     use serde_json::json;
 
@@ -510,9 +511,7 @@ mod tests {
     // which Miri cannot evaluate.
     #[cfg_attr(miri, ignore)]
     fn profile_exporter_new_and_delete() {
-        let mut tags = ddcommon_ffi::Vec::default();
-        let host = Tag::new("host", "localhost").expect("static tags to be valid");
-        tags.push(host);
+        let tags = vec![tag!("host", "localhost")].into();
 
         let result = unsafe {
             ddog_prof_Exporter_new(

profiling/tests/form.rs

@@ -60,13 +60,11 @@ fn multipart(
 mod tests {
     use crate::multipart;
     use datadog_profiling::exporter::*;
+    use ddcommon::tag;
     use serde_json::json;
 
     fn default_tags() -> Vec<Tag> {
-        vec![
-            Tag::new("service", "php").expect("static tags to be valid"),
-            Tag::new("host", "bits").expect("static tags to be valid"),
-        ]
+        vec![tag!("service", "php"), tag!("host", "bits")]
     }
 
     fn parsed_event_json(request: Request) -> serde_json::Value {

sidecar/src/dogstatsd.rs

@@ -165,8 +165,7 @@ mod test {
     use crate::dogstatsd::{create_client, Flusher};
     #[cfg(unix)]
     use ddcommon::connector::uds::socket_path_to_uri;
-    use ddcommon::tag::Tag;
-    use ddcommon::Endpoint;
+    use ddcommon::{tag, Endpoint};
     use http::Uri;
     use std::net;
     use std::time::Duration;
@@ -189,20 +188,12 @@ mod test {
             api_key: None,
         });
         flusher.send(vec![
-            Count(
-                "test_count".to_string(),
-                3,
-                vec![Tag::new("foo", "bar").unwrap()],
-            ),
+            Count("test_count".to_string(), 3, vec![tag!("foo", "bar")]),
             Count("test_neg_count".to_string(), -2, vec![]),
             Distribution("test_distribution".to_string(), 4.2, vec![]),
             Gauge("test_gauge".to_string(), 7.6, vec![]),
             Histogram("test_histogram".to_string(), 8.0, vec![]),
-            Set(
-                "test_set".to_string(),
-                9,
-                vec![Tag::new("the", "end").unwrap()],
-            ),
+            Set("test_set".to_string(), 9, vec![tag!("the", "end")]),
             Set("test_neg_set".to_string(), -1, vec![]),
         ]);