Make Non-blocking shutdown timeout configurable

[Amberley-Sz]

Updated the shutdown timeout for the non-blocking tracing-appender to be configurable, and added integration tests of the configurable timeout.

Motivation

Found a solution to this issue

Solution

Added a shutdown_timeout field in the builder, provided the default timeout to be 1 second but also allow users to custom a timeout through the builder pattern.

Can have another PR for master branch if necessary.

[davidbarsky]

The changes look good to me, can you open a corresponding PR to the master branch as well?

[Amberley-Sz]

The changes look good to me, can you open a corresponding PR to the master branch as well?

New PR on master branch here: #3242

[Amberley-Sz]

@davidbarsky This PR and the one on master branch are ready to merge

[kaffarell]

These links don't work, we should probably change this to:

    /// Returns the default priority mappings:
    ///
    /// - [`Level::ERROR`][]: [`Priority::Error`] (3)
    /// - [`Level::WARN`][]: [`Priority::Warning`] (4)
    /// - [`Level::INFO`][]: [`Priority::Notice`] (5)
    /// - [`Level::DEBUG`][]: [`Priority::Informational`] (6)
    /// - [`Level::TRACE`][]: [`Priority::Debug`] (7)
    ///
    /// [`Level::ERROR`]: tracing_core::Level::ERROR
    /// [`Level::WARN`]: tracing_core::Level::WARN
    /// [`Level::INFO`]: tracing_core::Level::INFO
    /// [`Level::DEBUG`]: tracing_core::Level::DEBUG
    /// [`Level::TRACE`]: tracing_core::Level::TRACE

Thanks!

[Amberley-Sz]

Changing to this would cause clippy warnings saying that link definitions are not shown in rendered documentation. Would changing to

    /// Returns the default priority mappings:
    ///
    /// - `tracing::Level::ERROR`: [`Priority::Error`] (3)
    /// - `tracing::Level::WARN`: [`Priority::Warning`] (4)
    /// - `tracing::Level::INFO`: [`Priority::Notice`] (5)
    /// - `tracing::Level::DEBUG`: [`Priority::Informational`] (6)
    /// - `tracing::Level::TRACE`: [`Priority::Debug`] (7)

be a better idea?

[kaffarell]

Umm that's weird I don't get any clippy warnings. Which rust version do you use? Anyways I'd avoid your example because it doesn't actually link to the levels.

[Amberley-Sz]

I'm using rustc 1.86.0
When I run cargo clippy it shows

Checking tracing-journald v0.3.1 (/local/home/asuzhan/tracing/tracing-journald)
warning: link reference defined in list item
   --> tracing-journald/src/lib.rs:518:11
    |
518 |     /// - [`tracing::Level::ERROR`]: [`Priority::Error`] (3)
    |           ^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: link definitions are not shown in rendered documentation
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#doc_nested_refdefs
    = note: `#[warn(clippy::doc_nested_refdefs)]` on by default
help: for an intra-doc link, add `[]` between the label and the colon
    |
518 |     /// - [`tracing::Level::ERROR`][]: [`Priority::Error`] (3)
    |                                    ++

warning: link reference defined in list item
   --> tracing-journald/src/lib.rs:519:11
    |
519 |     /// - [`tracing::Level::WARN`]: [`Priority::Warning`] (4)
    |           ^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: link definitions are not shown in rendered documentation
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#doc_nested_refdefs
help: for an intra-doc link, add `[]` between the label and the colon
    |
519 |     /// - [`tracing::Level::WARN`][]: [`Priority::Warning`] (4)
    |                                   ++

warning: link reference defined in list item
   --> tracing-journald/src/lib.rs:520:11
    |
520 |     /// - [`tracing::Level::INFO`]: [`Priority::Notice`] (5)
    |           ^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: link definitions are not shown in rendered documentation
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#doc_nested_refdefs
help: for an intra-doc link, add `[]` between the label and the colon
    |
520 |     /// - [`tracing::Level::INFO`][]: [`Priority::Notice`] (5)
    |                                   ++

warning: link reference defined in list item
   --> tracing-journald/src/lib.rs:521:11
    |
521 |     /// - [`tracing::Level::DEBUG`]: [`Priority::Informational`] (6)
    |           ^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: link definitions are not shown in rendered documentation
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#doc_nested_refdefs
help: for an intra-doc link, add `[]` between the label and the colon
    |
521 |     /// - [`tracing::Level::DEBUG`][]: [`Priority::Informational`] (6)
    |                                    ++

warning: link reference defined in list item
   --> tracing-journald/src/lib.rs:522:11
    |
522 |     /// - [`tracing::Level::TRACE`]: [`Priority::Debug`] (7)
    |           ^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: link definitions are not shown in rendered documentation
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#doc_nested_refdefs
help: for an intra-doc link, add `[]` between the label and the colon
    |
522 |     /// - [`tracing::Level::TRACE`][]: [`Priority::Debug`] (7)
    |                                    ++

warning: `tracing-journald` (lib) generated 5 warnings
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.22s

[kaffarell]

You didn't add my suggestion though.
Add:

/// Returns the default priority mappings:
///
/// - [`Level::ERROR`][]: [`Priority::Error`] (3)
/// - [`Level::WARN`][]: [`Priority::Warning`] (4)
/// - [`Level::INFO`][]: [`Priority::Notice`] (5)
/// - [`Level::DEBUG`][]: [`Priority::Informational`] (6)
/// - [`Level::TRACE`][]: [`Priority::Debug`] (7)
///
/// [`Level::ERROR`]: tracing_core::Level::ERROR
/// [`Level::WARN`]: tracing_core::Level::WARN
/// [`Level::INFO`]: tracing_core::Level::INFO
/// [`Level::DEBUG`]: tracing_core::Level::DEBUG
/// [`Level::TRACE`]: tracing_core::Level::TRACE

instead of the existing doc-comment.

[Amberley-Sz]

I see the point now! Thanks for the suggestion, updated!