Support constant expressions as instrument field names

tracing-attributes/src/attr.rs

@@ -1,5 +1,5 @@
 use std::collections::HashSet;
-use syn::{punctuated::Punctuated, Expr, Ident, LitInt, LitStr, Path, Token};
+use syn::{punctuated::Punctuated, Block, Expr, Ident, LitInt, LitStr, Path, Token};
 
 use proc_macro2::TokenStream;
 use quote::{quote, quote_spanned, ToTokens};
@@ -294,7 +294,7 @@ pub(crate) struct Fields(pub(crate) Punctuated<Field, Token![,]>);
 
 #[derive(Clone, Debug)]
 pub(crate) struct Field {
-    pub(crate) name: Punctuated<Ident, Token![.]>,
+    pub(crate) name: FieldName,
     pub(crate) value: Option<Expr>,
     pub(crate) kind: FieldKind,
 }
@@ -306,6 +306,21 @@ pub(crate) enum FieldKind {
     Value,
 }
 
+#[derive(Clone, Debug)]
+pub(crate) enum FieldName {
+    Expr(Block),
+    Punctuated(Punctuated<Ident, Token![.]>),
+}
+
+impl ToTokens for FieldName {
+    fn to_tokens(&self, tokens: &mut TokenStream) {
+        match self {
+            FieldName::Expr(expr) => expr.to_tokens(tokens),
+            FieldName::Punctuated(punctuated) => punctuated.to_tokens(tokens),
+        }
+    }
+}
+
 impl Parse for Fields {
     fn parse(input: ParseStream<'_>) -> syn::Result<Self> {
         let _ = input.parse::<kw::fields>();
@@ -332,7 +347,15 @@ impl Parse for Field {
             input.parse::<Token![?]>()?;
             kind = FieldKind::Debug;
         };
-        let name = Punctuated::parse_separated_nonempty_with(input, Ident::parse_any)?;
+        // Parse name as either an expr between braces or a dotted identifier.
+        let name = if input.peek(syn::token::Brace) {
+            FieldName::Expr(input.parse::<Block>()?)
+        } else {
+            FieldName::Punctuated(Punctuated::parse_separated_nonempty_with(
+                input,
+                Ident::parse_any,
+            )?)
+        };
         let value = if input.peek(Token![=]) {
             input.parse::<Token![=]>()?;
             if input.peek(Token![%]) {

tracing-attributes/src/expand.rs

@@ -9,6 +9,7 @@ use syn::{
     Path, ReturnType, Signature, Stmt, Token, Type, TypePath,
 };
 
+use crate::attr::FieldName;
 use crate::{
     attr::{Field, Fields, FormatMode, InstrumentArgs, Level},
     MaybeItemFn, MaybeItemFnRef,
@@ -190,8 +191,16 @@ fn gen_block<B: ToTokens>(
                 // and allow them to be formatted by the custom field.
                 if let Some(ref fields) = args.fields {
                     fields.0.iter().all(|Field { ref name, .. }| {
-                        let first = name.first();
-                        first != name.last() || !first.iter().any(|name| name == &param)
+                        match name {
+                            // TODO(#3158): Implement this variant when the compiler supports const
+                            //     evaluation (https://rustc-dev-guide.rust-lang.org/const-eval).
+                            FieldName::Expr(_) => true,
+                            FieldName::Punctuated(punctuated) => {
+                                let first = punctuated.first();
+                                first != punctuated.last()
+                                    || !first.iter().any(|name| name == &param)
+                            }
+                        }
                     })
                 } else {
                     true

tracing-attributes/src/lib.rs

@@ -107,14 +107,16 @@ mod expand;
 /// - multiple argument names can be passed to `skip`.
 /// - arguments passed to `skip` do _not_ need to implement `fmt::Debug`.
 ///
-/// Additional fields (key-value pairs with arbitrary data) can be passed to
+/// Additional fields (key-value pairs with arbitrary data) can be passed
 /// to the generated span through the `fields` argument on the
-/// `#[instrument]` macro. Strings, integers or boolean literals are accepted values
+/// `#[instrument]` macro. Arbitrary expressions are accepted as value
 /// for each field. The name of the field must be a single valid Rust
-/// identifier, nested (dotted) field names are not supported.
+/// identifier, or a constant expression that evaluates to one, enclosed in curly
+/// braces. Note that nested (dotted) field names are supported.
 ///
-/// Note that overlap between the names of fields and (non-skipped) arguments
-/// will result in a compile error.
+/// Note that defining a field with the same name as a (non-skipped)
+/// argument will implicitly skip the argument, or even panic if using
+/// constants as field names.
 ///
 /// # Examples
 /// Instrumenting a function:
@@ -214,8 +216,16 @@ mod expand;
 ///
 /// ```
 /// # use tracing_attributes::instrument;
-/// #[instrument(fields(foo="bar", id=1, show=true))]
-/// fn my_function(arg: usize) {
+/// #[derive(Debug)]
+/// struct Argument;
+/// impl Argument {
+///     fn bar(&self) -> &'static str {
+///         "bar"
+///     }
+/// }
+/// const FOOBAR: &'static str = "foo.bar";
+/// #[instrument(fields(foo="bar", id=1, show=true, {FOOBAR}=%arg.bar()))]
+/// fn my_function(arg: Argument) {
 ///     // ...
 /// }
 /// ```

tracing-attributes/tests/fields.rs

@@ -37,6 +37,38 @@ fn fn_string(s: String) {
 #[instrument(fields(keywords.impl.type.fn = _arg), skip(_arg))]
 fn fn_keyword_ident_in_field(_arg: &str) {}
 
+const CONST_FIELD_NAME: &str = "foo.bar";
+
+#[instrument(fields({CONST_FIELD_NAME} = "baz"))]
+fn fn_const_field_name() {}
+
+const fn get_const_fn_field_name() -> &'static str {
+    "foo.bar"
+}
+
+#[instrument(fields({get_const_fn_field_name()} = "baz"))]
+fn fn_const_fn_field_name() {}
+
+struct FieldNames {}
+impl FieldNames {
+    const FOO_BAR: &'static str = "foo.bar";
+}
+
+#[instrument(fields({FieldNames::FOO_BAR} = "baz"))]
+fn fn_struct_const_field_name() {}
+
+#[instrument(fields({"foo"} = "bar"))]
+fn fn_string_field_name() {}
+
+// TODO(#3158): To be consistent with event! and span! macros, the argument's value should be
+//     dropped, but this is only possible to implement when the compiler supports const evaluation
+//     (https://rustc-dev-guide.rust-lang.org/const-eval). For now the duplicated field would
+//     trigger a panic.
+// const CLASHY_FIELD_NAME: &str = "s";
+//
+// #[instrument(fields({CLASHY_FIELD_NAME} = "foo"))]
+// fn fn_clashy_const_field_name(s: &str) { let _ = s; }
+
 #[derive(Debug)]
 struct HasField {
     my_field: &'static str,
@@ -159,6 +191,55 @@ fn keyword_ident_in_field_name() {
     run_test(span, || fn_keyword_ident_in_field("test"));
 }
 
+#[test]
+fn expr_const_field_name() {
+    let span = expect::span().with_fields(expect::field("foo.bar").with_value(&"baz").only());
+    run_test(span, || {
+        fn_const_field_name();
+    });
+}
+
+#[test]
+fn expr_const_fn_field_name() {
+    let span = expect::span().with_fields(expect::field("foo.bar").with_value(&"baz").only());
+    run_test(span, || {
+        fn_const_fn_field_name();
+    });
+}
+
+#[test]
+fn struct_const_field_name() {
+    let span = expect::span().with_fields(expect::field("foo.bar").with_value(&"baz").only());
+    run_test(span, || {
+        fn_struct_const_field_name();
+    });
+}
+
+#[test]
+fn string_field_name() {
+    let span = expect::span().with_fields(expect::field("foo").with_value(&"bar").only());
+    run_test(span, || {
+        fn_string_field_name();
+    });
+}
+
+// TODO(#3158): To be consistent with event! and span! macros, the argument's value should be
+//     dropped, but this is only possible to implement when the compiler supports const evaluation
+//     (https://rustc-dev-guide.rust-lang.org/const-eval). For now the duplicated field would
+//     trigger a panic.
+// #[test]
+// fn clashy_const_field_name() {
+//     let span = expect::span().with_fields(
+//         expect::field("s")
+//             .with_value(&"foo")
+//             .and(expect::field("s").with_value(&"hello world"))
+//             .only(),
+//     );
+//     run_test(span, || {
+//         fn_clashy_const_field_name("hello world");
+//     });
+// }
+
 fn run_test<F: FnOnce() -> T, T>(span: NewSpan, fun: F) {
     let (collector, handle) = collector::mock()
         .new_span(span)

tracing-core/src/field.rs

@@ -730,11 +730,46 @@ impl Clone for Field {
     }
 }
 
+// Compares two strings for equality in a constant context.
+const fn str_eq(left: &str, right: &str) -> bool {
+    if left.len() != right.len() {
+        return false;
+    }
+
+    let left_bytes = left.as_bytes();
+    let right_bytes = right.as_bytes();
+    let mut i = left_bytes.len();
+    while i > 0 {
+        if left_bytes[i - 1] != right_bytes[i - 1] {
+            return false;
+        }
+        i -= 1;
+    }
+
+    true
+}
+
+// Asserts field names are distinct.
+const fn assert_distinct(names: &[&str]) {
+    let mut i = names.len();
+    while i > 0 {
+        let mut j = i - 1;
+        while j > 0 {
+            if str_eq(names[i - 1], names[j - 1]) {
+                panic!("duplicated field name");
+            }
+            j -= 1;
+        }
+        i -= 1;
+    }
+}
+
 // ===== impl FieldSet =====
 
 impl FieldSet {
     /// Constructs a new `FieldSet` with the given array of field names and callsite.
     pub const fn new(names: &'static [&'static str], callsite: callsite::Identifier) -> Self {
+        assert_distinct(names);
         Self { names, callsite }
     }