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

question: How to flush on a async logger? #35

Open
gabyx opened this issue Mar 12, 2024 · 7 comments · May be fixed by slog-rs/slog#332
Open

question: How to flush on a async logger? #35

gabyx opened this issue Mar 12, 2024 · 7 comments · May be fixed by slog-rs/slog#332

Comments

@gabyx
Copy link

gabyx commented Mar 12, 2024

How can I instruct the log = create_logger_cli(false) to be flushed manually (I have a password prompt on stdout which should first flush the whole log such that the prompt is at the right place)
Basically log.flush() which would block until all records have been processed by the thread? Is there another better method?

pub fn create_logger_cli(timestamp: bool) -> Arc<Logger> {
    let decorator = slog_term::TermDecorator::new().build();

    let mut d1 = slog_term::FullFormat::new(decorator);

    if !timestamp {
        d1 = d1.use_custom_timestamp(no_out);
    }

    let drain = d1.build().fuse();

    let drain = slog_async::Async::new(drain)
        .chan_size(5_000_000)
        .build()
        .fuse();

    return Arc::new(slog::Logger::root(drain, o!()));
}
@dpc
Copy link
Contributor

dpc commented Mar 12, 2024

Can't really flush an async logger, as it can't not control the receiver on the other side of a channel.

The closest thing we could have is maybe waiting for the channel to empty. But I'm not sure if we should.

Generally displaying password prompts via logging is not a good idea. One should be using stderr for that, or some completely different things altogether.

@gabyx
Copy link
Author

gabyx commented Mar 12, 2024

Jeah sure. I guessed that its kind of tricky, waiting till the channel has emptied is close, but probably still no guaranteed that the underlaying drains have been flushed... only the channel is empty...

Thanks for the response. I find another solution.

@dpc
Copy link
Contributor

dpc commented Mar 12, 2024

Ah. I'm silly. You are logging with slog-async, and then you might want to display prompt etc. on stderr . It seems like a legitimate use case...

Under the hood we are using

async/lib.rs

Line 336 in c2040ac

tl_sender: thread_local::ThreadLocal::new(),
to send messages, from crossbeam-channel.

It does support is_empty(): https://docs.rs/crossbeam-channel/latest/crossbeam_channel/struct.Sender.html#method.is_empty

So i guess we (I mean... you, if you care enough to submit a PR :D ) could add a new method to the trait / API that would attempt to "flush" (e.g. wait until is_empty() == true with some polling interval). If it's a trait, add a default impl that just return a custom IO error "unsupported" for backward compat.

@Techcable for awareness.

@gabyx
Copy link
Author

gabyx commented Mar 13, 2024

Yes, you understood that correctly. Yes, sounds interesting to have a look at over the weekend. Thanks for pointing into the directions! If there are more thoughts let me know.

@gabyx
Copy link
Author

gabyx commented Mar 16, 2024

@dpc: Some question for undestanding the problem better: Do you think it makes sense to make a new trait Flushable which a Drain instance can have where it would implement flush in lib slog/lib.rs.
And then make an implementation for Async as you suggested =).

Thanks alot

@gabyx
Copy link
Author

gabyx commented Mar 16, 2024

Ah maybe its better to add this functionality to the Drain with a default implementation?

@dpc
Copy link
Contributor

dpc commented Mar 16, 2024

Most ecosystem expects Drain already, so it would have be a new method on an existing trait with an default impl that does something reasonable (like signals lack of support for flushing).

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 a pull request may close this issue.

2 participants