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

Backport #1378 and #1385 to v0.1.x #1418

Merged
merged 2 commits into from
Jun 1, 2021
Merged

Backport #1378 and #1385 to v0.1.x #1418

merged 2 commits into from
Jun 1, 2021

Conversation

Folyd
Copy link
Contributor

@Folyd Folyd commented May 31, 2021

Backport #1378 and #1385 to v0.1.x.

Folyd added 2 commits June 1, 2021 00:41
… `fmt::Debug` (tokio-rs#1378)

The default behavior of `tracing::instrument` attribution will record
all of the function arguments as `fmt::Debug`, which is overwhelmed and
unnecessary for those primitive types, such as `bool`, `u8`, `i8`,
`u16`, `i16`, `u32`, `i32`, `u64`, `i64`, `usize`, and `isize`. Another
concerning reason is that we‘ll lose the type info of those primitive
types when record by a `Visitor`, while those type infos is essential to
some people. For example, I need to show my spans in Jaeger UI.

Make the `tracing::instrument` records other function arguments as
`fmt::Debug ` while not for those primitive types.

However, I'm not good at naming. Maybe the `RecordType` enum and its
variant aren't a good name? I'd love to seek suggestions. Thanks.
Impl `Value` for `&mut T` where `T: Value`. 

tokio-rs#1378 improve the `tracing::instrument` macro's recording behavior: all
primitive types implementing the `Value` trait will be recorded as
fields of that type instead of `fmt::Debug`. This PR is a prerequisite
for those `&mut` function arguments to achieve that.
@Folyd Folyd requested review from hawkw and a team as code owners May 31, 2021 16:43
Copy link
Member

@hawkw hawkw left a comment

Choose a reason for hiding this comment

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

thanks, lgtm!

@hawkw hawkw merged commit d936628 into tokio-rs:v0.1.x Jun 1, 2021
@Folyd Folyd deleted the v0.1.x branch June 1, 2021 16:29
kaffarell added a commit to kaffarell/tracing that referenced this pull request Oct 30, 2023
There was an error when backporting tokio-rs#1378 (here: tokio-rs#1418) and a trailing
dot (.) was forgotten (which was breaking the link). Fixed also the link to
`std::fmt::Debug`.
kaffarell added a commit to kaffarell/tracing that referenced this pull request Oct 30, 2023
There was an error when backporting tokio-rs#1378 (here: tokio-rs#1418) and a trailing
dot (.) was forgotten (which was breaking the link). Fixed also the link to
`std::fmt::Debug`.
davidbarsky pushed a commit that referenced this pull request Oct 30, 2023
There was an error when backporting #1378 (here: #1418) and a trailing
dot (.) was forgotten (which was breaking the link). Fixed also the link to
`std::fmt::Debug`.
kaffarell added a commit to kaffarell/tracing that referenced this pull request May 22, 2024
There was an error when backporting tokio-rs#1378 (here: tokio-rs#1418) and a trailing
dot (.) was forgotten (which was breaking the link). Fixed also the link to
`std::fmt::Debug`.
davidbarsky pushed a commit that referenced this pull request Oct 2, 2024
There was an error when backporting #1378 (here: #1418) and a trailing
dot (.) was forgotten (which was breaking the link). Fixed also the link to
`std::fmt::Debug`.
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.

2 participants