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

Use env!("CARGO_CRATE_NAME") in the example to simplify the tracing setup code #2884

Merged
merged 2 commits into from
Aug 24, 2024

Conversation

hongweipeng
Copy link
Contributor

…etup code

Motivation

The example code is full of tracing registration codes, and the log level is hard-coded. I think it can be replaced with the module_path! macro.

Solution

Simplify the code.

@hongweipeng hongweipeng force-pushed the simplify-example-tracing-setting branch from 0111e70 to 63a42c6 Compare August 21, 2024 15:40
@jplatte
Copy link
Member

jplatte commented Aug 21, 2024

Interesting idea. Two suggestions:

  • Use concat! instead of format!
  • Use the crate name instead of the module path so this still applies the log level to the entire crate's logs if moved into a submodule

What do you think? I think together this would look like concat!(env!("CARGO_CRATE_NAME"), "=debug")).

@ttys3
Copy link
Contributor

ttys3 commented Aug 22, 2024

CARGO_CRATE_NAME is better.

and for concat , I do not think it is necessary. keep using format! is much clear.

@hongweipeng hongweipeng force-pushed the simplify-example-tracing-setting branch from 63a42c6 to a83c795 Compare August 22, 2024 08:49
@hongweipeng hongweipeng changed the title Use the module_path! macro in the example to simplify the tracing setup code Use env!("CARGO_CRATE_NAME") in the example to simplify the tracing setup code Aug 22, 2024
@ttys3
Copy link
Contributor

ttys3 commented Aug 23, 2024

I'm afraid that concat!(env!("CARGO_CRATE_NAME"), "=debug,tower_http=debug") is much less clear compared to:

format!("{}=debug,tower_http=debug", env!("CARGO_CRATE_NAME"))

I do not believe there will be performance issues for our usage scenario.

tracing_subscriber::EnvFilter::try_from_default_env()
.unwrap_or_else(|_| "example_error_handling=debug,tower_http=debug".into()),
tracing_subscriber::EnvFilter::try_from_default_env().unwrap_or_else(|_| {
concat!(env!("CARGO_CRATE_NAME"), "=debug,tower_http=debug").into()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm afraid that concat!(env!("CARGO_CRATE_NAME"), "=debug,tower_http=debug") is much less clear compared to:

format!("{}=debug,tower_http=debug", env!("CARGO_CRATE_NAME"))

I do not believe there will be performance issues for our usage scenario.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it's a bit ugly. Feel free to change it back.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally, I'd say having

let crate_name = env!("CARGO_CRATE_NAME");
format!("{crate_name}=debug,tower_http=debug")

would be a more readable alternative, but feel free to ignore this, either is fine.

Copy link
Member

@jplatte jplatte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@jplatte jplatte merged commit ef1448a into tokio-rs:main Aug 24, 2024
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants