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

Add special "void span" to discard all child spans #2738

Closed
andrewbanchich opened this issue Oct 6, 2023 · 6 comments
Closed

Add special "void span" to discard all child spans #2738

andrewbanchich opened this issue Oct 6, 2023 · 6 comments

Comments

@andrewbanchich
Copy link

Feature Request

Add a special "void span" where all spans created within it are noops / removed.

Motivation

I am working on a way to ensure sensitive data is not accidentally logged by creating a wrapper type, which provides a map() method for users to work with the underlying secret. However, it is possible for a user to pass a function to map() which is instrumented with a span.

I would like to be able to change it to something like

fn map<U, F>(self, f: F) -> Secret<U>
    where
        F: FnOnce(T) -> U,
    {
        let void_span = void!();
        let _enter = void_span.enter();

        Self::new(f(x))
    }

to ensure nothing is logged.

Proposal

Alternatives

There are several approaches to redacting sensitive data, but it is usually necessary to have some section of the code need to work with that data directly. It would be nice to be able to minimize the scope of where that occurs, as well as ensure that no logging happens there.

@andrewbanchich andrewbanchich changed the title Add custom trait for logging representation Add void! span Oct 6, 2023
@andrewbanchich andrewbanchich changed the title Add void! span Add special void! span to ignore all child spans Oct 6, 2023
@andrewbanchich andrewbanchich changed the title Add special void! span to ignore all child spans Add special "void span" to discard all child spans Oct 6, 2023
@hawkw
Copy link
Member

hawkw commented Oct 6, 2023

You can achieve the desired effect of dropping all spans and events within a scope by setting the default subscriber to NoSubscriber within that scope. This will ensure that no spans or events are recorded within that scope.

For example, you could write the following:

fn map<U, F>(self, f: F) -> Secret<U>
where
    F: FnOnce(T) -> U,
{
    use tracing::subscriber::{self, NoSubscriber};
    let _no_subscriber = subscriber::set_default(NoSubcriber::default());

    Self::new(f(x))
}

@andrewbanchich
Copy link
Author

Fantastic! Thank you so much.

@andrewbanchich
Copy link
Author

Related to this - is there any way to conditionally log an error! if there are any child spans produced within this?

@andrewbanchich
Copy link
Author

andrewbanchich commented Oct 7, 2023

I think I figured it out: I just created a new type and copied the Subscriber impl from NoSubscriber, except set enabled() to true, added an eprintln!("REDACTED") to event(), and returned false from try_close().

Does that sound right? Is using eprintln! here the right way of doing it?

@davidbarsky
Copy link
Member

davidbarsky commented Oct 7, 2023

I'm guessing that this is Cedar-adjacent: I think you're better off panicking if you want a construct like this.

@andrewbanchich
Copy link
Author

Why would you recommend panicking? I'd like to avoid panics, and just redact any logs from the this span, but also know that something attempted to log.

hawkw pushed a commit that referenced this issue Mar 1, 2024
…tion (#2883)


## Motivation

Const argumetns in `level!` macros do not work when in the first
position.

This also seems to have fixed #2748 where literals for fields names like
`info!("foo" = 2)` could not be used outside the `event!` macro.


Fixes #2837
Fixes #2738

## Solution

Previsously, `level!($(args:tt))` was forwarded to `event!(target: ...,
level: ..., { $(args:tt) })` but the added curly braces seem to have
prevented the `event` macro from correctly understanding the arguments
and it tried to pass them to `format!`.

With this change there may have some performance impact when expanding
the macros in most cases where the braces could have been added as it
will take one more step.

These are the two relevant `event!` blocks I believe, the new tests used
to expand to the first one (with empty fields), now they expand to the
latter:
```
    (target: $target:expr, $lvl:expr, { $($fields:tt)* }, $($arg:tt)+ ) => (
        $crate::event!(
            target: $target,
            $lvl,
            { message = $crate::__macro_support::format_args!($($arg)+), $($fields)* }
        )
    );
    (target: $target:expr, $lvl:expr, $($arg:tt)+ ) => (
        $crate::event!(target: $target, $lvl, { $($arg)+ })
    );
```
dpc pushed a commit to dpc/tracing that referenced this issue Apr 20, 2024
…tion (tokio-rs#2883)


## Motivation

Const argumetns in `level!` macros do not work when in the first
position.

This also seems to have fixed tokio-rs#2748 where literals for fields names like
`info!("foo" = 2)` could not be used outside the `event!` macro.


Fixes tokio-rs#2837
Fixes tokio-rs#2738

## Solution

Previsously, `level!($(args:tt))` was forwarded to `event!(target: ...,
level: ..., { $(args:tt) })` but the added curly braces seem to have
prevented the `event` macro from correctly understanding the arguments
and it tried to pass them to `format!`.

With this change there may have some performance impact when expanding
the macros in most cases where the braces could have been added as it
will take one more step.

These are the two relevant `event!` blocks I believe, the new tests used
to expand to the first one (with empty fields), now they expand to the
latter:
```
    (target: $target:expr, $lvl:expr, { $($fields:tt)* }, $($arg:tt)+ ) => (
        $crate::event!(
            target: $target,
            $lvl,
            { message = $crate::__macro_support::format_args!($($arg)+), $($fields)* }
        )
    );
    (target: $target:expr, $lvl:expr, $($arg:tt)+ ) => (
        $crate::event!(target: $target, $lvl, { $($arg)+ })
    );
```
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

No branches or pull requests

3 participants