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: ret(skip) & err(skip) #2645

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

JonathanWoollett-Light
Copy link
Contributor

@JonathanWoollett-Light JonathanWoollett-Light commented Jul 10, 2023

Introduces an additional format mode when emitting events with the instrument attribute macro when returning from a function.

Motivation

It is often useful to emit events when a function returns (this can be done with the ret or err options for the instrument attribute macro). Presently we can only emit these events when we print the return values (with Debug or Display). There are circumstances where the returns values are not appropriate to print (consider a function that returns a slice that may be very log) or they cannot be printed (a type from a foreign library that neither implements Debug nor Display). In this case it is still useful to print an event on the function returning even if not printing the return value.

The example program

use std::fmt;
use tracing::instrument;

const TEXT: &str = "
Lorem ipsum dolor sit amet, consectetur adipiscing elit. Sed porta nulla id tellus tincidunt ultricies. Sed ullamcorper turpis vel lorem semper mollis. Nulla et ante ut sem convallis gravida ac pharetra massa. Suspendisse potenti. Sed gravida enim ac quam rhoncus commodo. Aliquam felis dolor, rhoncus id tincidunt id, rutrum sed nisl. Integer ac vehicula metus. Vestibulum nibh lorem, tincidunt ut elementum sit amet, ullamcorper et velit.

In hac habitasse platea dictumst. Curabitur tortor ipsum, rutrum quis placerat quis, hendrerit non felis. Fusce pellentesque felis at mauris porta congue. Aliquam a augue aliquet, condimentum massa in, rutrum purus. Praesent eget commodo risus, quis pretium sapien. Sed velit erat, auctor eu ligula non, tristique ornare eros. Nulla ornare dignissim lacus, dignissim aliquam tellus porta et.

Vivamus placerat nisi ut leo semper, ac sodales orci tincidunt. Ut auctor lobortis lacus mollis dictum. Vivamus vel libero eu tortor convallis ornare sed at erat. Pellentesque varius magna at felis hendrerit posuere. Suspendisse vestibulum consequat turpis id tempor. Interdum et malesuada fames ac ante ipsum primis in faucibus. Donec at maximus urna. Proin ac metus et lacus consequat imperdiet non nec sapien. Sed nec purus elit. Phasellus a dolor pellentesque, mattis elit accumsan, condimentum massa. Sed id dolor facilisis odio fringilla tempus. Aenean eget congue diam. Nulla blandit elementum nibh, auctor pulvinar lacus mattis vel. Sed feugiat luctus mollis. Nulla suscipit posuere tellus id fermentum.

Nullam porta accumsan purus at eleifend. Duis pellentesque feugiat lacus, nec faucibus risus hendrerit nec. Pellentesque vulputate eleifend ex. Nullam scelerisque consectetur odio, faucibus rhoncus lacus bibendum sit amet. Cras molestie convallis ante in interdum. Nam sit amet sem non ante tristique varius. Class aptent taciti sociosqu ad litora torquent per conubia nostra, per inceptos himenaeos. Ut quis iaculis mauris, id porttitor magna. Integer ut magna varius ex venenatis vehicula eu et ipsum. Nam sit amet velit eu ante porttitor tempus ac eu massa. Integer eget velit augue. Aenean diam felis, consectetur id scelerisque eget, convallis eget dui. Nunc nec enim ante. Nulla at nibh dapibus, suscipit leo eu, egestas sem.

Sed vitae odio finibus, ullamcorper justo eget, aliquet mi. Mauris dignissim lacus odio, eget blandit felis blandit non. Integer sem urna, lobortis nec est quis, tempus hendrerit purus. Vestibulum ante ipsum primis in faucibus orci luctus et ultrices posuere cubilia curae; Mauris id luctus mi. Sed accumsan quis mauris sit amet molestie. Suspendisse magna velit, dictum a tellus vitae, accumsan dignissim leo. Praesent eu lacus vel tellus finibus condimentum. Integer eget ullamcorper ex, sit amet tempor nulla. Morbi sagittis sed justo non aliquet. Proin imperdiet nunc ut ante ullamcorper rhoncus. Pellentesque a facilisis nibh.
";

fn main() {
    tracing_subscriber::fmt().init();
    let _y = as_slice(TEXT);
}

#[instrument(skip(s), ret(skip))]
pub fn as_slice(s: &str) -> &[u8] {
    s.as_bytes()
}

outputs

2023-07-10T11:01:52.450623Z  INFO as_slice: test_bin:

Solution

Adding a variant to FormatMode that sets the return value for the event as Empty.

Limitations

Due to the wider design this does not allow for emitting events when the return value contains a reference, this should be fixed in the future.

But even without this, this change is useful.

Introduces an additional format mode when emitting events with the `instrument` attribute macro when returning from a function.
@LegNeato
Copy link

Can you differentiate between a bare return and return with a skipped value? If not, would it make sense to indicate the return value was explicitly skipped in the output?

@JonathanWoollett-Light
Copy link
Contributor Author

JonathanWoollett-Light commented Aug 1, 2023

Can you differentiate between a bare return and return with a skipped value? If not, would it make sense to indicate the return value was explicitly skipped in the output?

Currently you cannot differentiate.

I think potentially a change could be made such that:

#[instrument(skip(s), ret)]
pub fn as_slice(s: &str) {
    todo!()
}

outputs:

2023-07-10T11:01:52.450623Z  INFO as_slice: test_bin: ()

where as

#[instrument(skip(s), ret(skip))]
pub fn as_slice(s: &str) {
    todo!()
}

outputs:

2023-07-10T11:01:52.450623Z  INFO as_slice: test_bin:

I think this makes sense as a way to differentiate.

But I think this change is not necessary in this PR and can be made later. I think the initial feature of being able to emit an event (and log) without printing the return value is valuable in isolation (without changing anything more about how ret works).

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