Need &'static strs from semantic-conventions for tracing field recording

[kriswuollett]

I'd like to do something like the following with tracing:

event!(
    Level::INFO,
    { semconv::trace::CODE_FUNCTION } = "update_user",
    { semconv::trace::CODE_NAMESPACE } = "com.my-org",
    { my_org::semconv::USER_ID } = request.user_id,
    "Updating user"
);

The tracing crate requires values that look like Rust identifiers, string literals, or constant expressions. But the string for values like code function are wrapped up in a Key which although const does not satisfy the format_args that is used by tracing:

error: format argument must be a string literal
  --> sample/src/tracing_test/mod.rs:28:9
   |
28 |         { semconv::trace::CODE_FUNCTION } = "update_user",
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
help: you might be missing a string literal to format with
   |
28 |         "{} {} {} {}", { semconv::trace::CODE_FUNCTION } = "update_user",
   |         ++++++++++++++

I'd think that the easiest solution would be to create another submodule that is code generated with the just primary source of the key strings from which the module with the const Keys are derived, e.g.,:

const CODE_FUNCTION: &str = "code.function";

That way something like the above example would work for tracing as well as any other context in which const / &'static str is required.

Although the code generation can be on my end, automatically or manually, this seems basic enough and useful to others that I'd think it should be implemented here? Or is there some Rust technique or library I haven't learned of yet to make use of the existing constants instead of copy and pasting raw str values like "code.function" everywhere?

[djc]

Can you use the Key::as_str() method in this context? Maybe if we just made that method const?

[kriswuollett]

I forked the repo and used my local path to make these modifications in opentelemetry-rust:

    /// Returns a reference to the underlying key name
    pub const fn as_str(&self) -> &str {
        self.0.as_str()
    }

impl OtelString {
    const fn as_str(&self) -> &str {
        match self {
            OtelString::Owned(s) => panic!(),
            OtelString::Static(s) => s,
            OtelString::RefCounted(s) => panic!(),
        }
    }
}

The above panic!() in there is just to get it to compile due to:

error[E0015]: cannot call non-const fn `<Box<str> as AsRef<str>>::as_ref` in constant functions
   --> opentelemetry/src/common.rs:148:39
    |
148 |             OtelString::Owned(s) => s.as_ref(),
    |                                       ^^^^^^^^
    |
    = note: calls in constant functions are limited to constant functions, tuple structs and tuple variants

Even with the changes compiled the following statement:

event!(Level::INFO, { semconv::trace::CODE_FUNCTION.as_str() } = "do_work",  "hello_world");

does not work because:

error[E0716]: temporary value dropped while borrowed
  --> src/main.rs:12:2
   |
12 |     event!(Level::INFO, { semconv::trace::CODE_FUNCTION.as_str() } = "do_work",  "Initialized telemetry");
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |     |
   |     creates a temporary value which is freed while still in use
   |     temporary value is freed at the end of this statement
   |     cast requires that borrow lasts for \`'static\`
   |
   = note: this error originates in the macro \`$crate::fieldset\` which comes from the expansion of the macro \`event\` (in Nightly builds, run with -Z macro-backtrace for more info)

Even if that worked I'd probably protest a bit because of the copy-and-pasted .as_str() calls. :-)

Please see the following example to test usage. Its output should look something like:

% cargo run
   Compiling tracing-example v0.1.0 (/Users/kris/Code/kriswuollett/tracing-example)
    Finished dev [unoptimized + debuginfo] target(s) in 0.54s
     Running \`target/debug/tracing-example\`
2023-10-26T11:45:01.386046Z  INFO main ThreadId(01) tracing_example: Initialized telemetry
2023-10-26T11:45:01.386230Z  INFO main ThreadId(01) do_work: tracing_example: Preparing to print greeting to console code.function="do_work" com.tracing-example.user-name="Anonymous" code.function="do_work"
Hello, Anonymous!

[package]
name = "tracing-example"
version = "0.1.0"
edition = "2021"

[dependencies]
opentelemetry-semantic-conventions = { version = "0.12.0", default-features = false }
tracing = { version = "0.1.37", default-features = false }
tracing-subscriber = { version = "0.3.17", default-features = false, features = ["fmt"] }

use tracing::{event, span, Level};
use tracing_subscriber::{prelude::*};

pub mod semconv {
    pub const CODE_FUNCTION: &str = "code.function";
}

pub mod api_methods {
    pub const DO_WORK_METHOD: &str = "do_work";
}

pub mod api_attrs {
    pub const USER_NAME: &str = "com.tracing-example.user-name";
}

fn do_work(user_name: &str) {
    let span = span!(
        Level::INFO,
        api_methods::DO_WORK_METHOD,
        { semconv::CODE_FUNCTION } = api_methods::DO_WORK_METHOD
    );
    let _enter = span.enter();
    event!(
        Level::INFO,
        { semconv::CODE_FUNCTION } = api_methods::DO_WORK_METHOD,
        { api_attrs::USER_NAME } = user_name,
        "Preparing to print greeting to console"
    );
    println!("Hello, {user_name}!");
}

fn main() {
    let fmt = tracing_subscriber::fmt::layer()
        .with_level(true)
        .with_target(true)
        .with_thread_ids(true)
        .with_thread_names(true)
        .with_ansi(false)
        .compact();

	tracing_subscriber::registry().with(fmt).init();

	event!(Level::INFO, "Initialized telemetry");

    do_work("Anonymous");
}

[shaun-cox]

I'd be in favor of changing this and all the others from:

pub const CODE_FUNCTION: Key = Key::from_static_str("code.function");

to

pub const CODE_FUNCTION = "code.function";

and relying on the existing const fn Key::from_static_str and impl From<&'static str> for Key to compose the rest where needed.

I see no fundamental reason why the semantic convention consts need to be bound to Key.

If other @open-telemetry/rust-approvers are not opposed, would @kriswuollett be up for submitting a PR?

[TommyCpp]

I think it's a valid use cases.

I see no fundamental reason why the semantic convention consts need to be bound to Key

I think it's easier for users to just import a Key and attach it but if it's blocking other use case we don't have to maintain this behavior. A simple static string maybe a better choice

[kriswuollett]

@shaun-cox, yes I'd be happy to put together a PR for this.

[lalitb]

Just to confirm, these changes are solely applicable to the semantic conventions of traces and do not extend to resources, correct?

[kriswuollett]

Just to confirm, these changes are solely applicable to the semantic conventions of traces and do not extend to resources, correct?

No. Although the title of the issue says tracing, I did intend for it to mean any "key" that one would use in the fields for a library like the tracing crate and its span! and event! macros. I most definitely would want to use things like SERVICE_NAME too.

I think for something like this we'd need 2 PRs? One to add/deprecate w/ experimental feature to remove, and the second that removes the experimental feature and just removes deprecated definitions. I'll introduce myself on the community chat channel.

[djc]

I'm not a fan of any approach where we end up with two sets of constants. I think it's better to have whole bunch of &'static str consts and then make the path to build Keys from that as smooth as possible.

[lalitb]

I'm not a fan of any approach where we end up with two sets of constants. I think it's better to have whole bunch of &'static str consts and then make the path to build Keys from that as smooth as possible.

+1

[kriswuollett]

@djc, so the project is okay with a potentially breaking change where all the Keys are just directly replaced by &'static str in #1334?

[djc]

Yes, we generally don't try very hard to keep compatibility.

[hdost]

Yes, we generally don't try very hard to keep compatibility.

We're attempting to get to stable, but for now breaking changes more need to be noted in the Changelog for users to know.