Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(common): add tag! macro #453

Merged
merged 3 commits into from
May 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 6 additions & 6 deletions LICENSE-3rdparty.yml

Large diffs are not rendered by default.

10 changes: 5 additions & 5 deletions bin_tests/src/bin/crashtracker_bin_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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"),
],
},
)?;
Expand Down
4 changes: 2 additions & 2 deletions crashtracker/src/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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(),
Expand Down
10 changes: 5 additions & 5 deletions crashtracker/src/telemetry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ mod tests {

use crate::SigInfo;
use chrono::DateTime;
use ddcommon::{tag::Tag, Endpoint};
use ddcommon::{tag, Endpoint};

use super::TelemetryCrashUploader;

Expand All @@ -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"),
],
}
}
Expand Down
2 changes: 1 addition & 1 deletion ddcommon-ffi/src/tags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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())),
}
}

Expand Down
1 change: 1 addition & 0 deletions ddcommon/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
119 changes: 92 additions & 27 deletions ddcommon/src/tag.rs
Original file line number Diff line number Diff line change
@@ -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 {
morrisonlevi marked this conversation as resolved.
Show resolved Hide resolved
($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 {
Expand All @@ -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.
morrisonlevi marked this conversation as resolved.
Show resolved Hide resolved
fn from_value<'a, IntoCow>(chunk: IntoCow) -> anyhow::Result<Self>
where
IntoCow: Into<Cow<'a, str>>,
{
Expand All @@ -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>,
Expand Down Expand Up @@ -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();
}
}
}
Expand All @@ -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() {
Expand All @@ -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]
Expand All @@ -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"),
];

Expand Down
4 changes: 2 additions & 2 deletions ddtelemetry/examples/tm-metrics-worker-test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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();

Expand Down
4 changes: 2 additions & 2 deletions ddtelemetry/examples/tm-worker-test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use std::{
time::{Duration, Instant},
};

use ddcommon::tag::Tag;
use ddcommon::tag;
use ddtelemetry::{data, worker};

macro_rules! timeit {
Expand Down Expand Up @@ -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();

Expand Down
3 changes: 2 additions & 1 deletion ddtelemetry/src/metrics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,7 @@ impl MetricContexts {

#[cfg(test)]
mod tests {
use ddcommon::tag;
use std::fmt::Debug;

use super::*;
Expand Down Expand Up @@ -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());
Expand Down
5 changes: 2 additions & 3 deletions profiling-ffi/src/exporter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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(
Expand Down
6 changes: 2 additions & 4 deletions profiling/tests/form.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
15 changes: 3 additions & 12 deletions sidecar/src/dogstatsd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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![]),
]);

Expand Down
Loading