Skip to content

Conversation

@SpriteOvO
Copy link
Owner

@SpriteOvO SpriteOvO commented Aug 28, 2025

Closes #71.

This PR addresses the issues mentioned by #71 -- downstream implementors are required to write a large amount of boilerplate code and manually add dependencies (such as atomic crate for Atomic) when creating their own sinks. This is currently the main blocker for implementing third-party crates (I'm currently working on and planning to implement spdlog-telegram, spdlog-opentelemetry, etc. crates).

The benefits of this PR can be simply seen in the diff of file spdlog/examples/05_sink.rs, and I will elaborate on more details.

Explanation

3 new items are introduced in this PR.

pub struct SinkProp {
    level_filter: Atomic<LevelFilter>,
    formatter: RwLock<Box<dyn Formatter>>,
    error_handler: SinkErrorHandler,
}

impl SinkProp {
    pub fn level_filter(&self) -> LevelFilter { /* ... */ }
    pub fn set_level_filter(&self, level_filter: LevelFilter) { /* ... */ }
    pub fn formatter<'a>(&'a self) -> impl Deref<Target = dyn Formatter> + 'a { /* ... */ }
    pub fn set_formatter(&self, formatter: Box<dyn Formatter>) { /* ... */ }
    pub fn error_handler(&self) -> Option<ErrorHandler> { /* ... */ }
    pub fn set_error_handler(&self, handler: Option<ErrorHandler>) { /* ... */ }
}
pub trait GetSinkProp {
    fn prop(&self) -> &SinkProp;
}
pub trait SinkAccess {
    fn level_filter(&self) -> LevelFilter;
    fn set_level_filter(&self, level_filter: LevelFilter);
    fn set_formatter(&self, formatter: Box<dyn Formatter>);
    fn set_error_handler(&self, handler: Option<ErrorHandler>);
}

The definition of Sink has been changed to

pub trait Sink: SinkAccess + Sync + Send {
    fn should_log(&self, level: Level) -> bool { /* default impl */ }
    fn log(&self, record: &Record) -> Result<()>;
    fn flush(&self) -> Result<()>;
}

Clearly, some original methods of Sink have been extracted into a separate trait SinkAccess, which has a blanket implementation:

impl<S: GetSinkProp> SinkAccess for S {
    fn level_filter(&self) -> LevelFilter { self.prop().level_filter() }
    fn set_level_filter(&self, level_filter: LevelFilter) { self.prop().set_level_filter(level_filter); }
    fn set_formatter(&self, formatter: Box<dyn Formatter>) { self.prop().set_formatter(formatter); }
    fn set_error_handler(&self, handler: Option<ErrorHandler>) { self.prop().set_error_handler(handler); }
}

Based on the above design, downstream implementors have 2 approaches to implement a custom sink.

Easy approach

When users do not need complex customization, they can directly use SinkProp, and the blanket implementation of GetSinkProp will automatically handle the boilerplate code part (SinkAccess).

struct MySink {
    prop: SinkProp,
    // ...
}

impl GetSinkProp for MySink {
    fn prop(&self) -> &SinkProp { &self.prop }
}

impl Sink for MySink {
    fn log(&self, record: &Record) -> Result<()> { /* ... */ }
    fn flush(&self) -> Result<()> { /* ... */ }
}

Advanced approach

When complex customizations are required, users directly implement SinkAccess for their Sink, and SinkProp and GetSinkProp are not needed here.

struct MySink {
    // ... define their own properties
    // ...
}

impl SinkAccess for MySink {
    fn level_filter(&self) -> LevelFilter { /* access their own property */ }
    fn set_level_filter(&self, level_filter: LevelFilter) { /* access their own property */ }
    fn set_formatter(&self, formatter: Box<dyn Formatter>) { /* access their own property */ }
    fn set_error_handler(&self, handler: Option<ErrorHandler>) { /* access their own property */ }
}

impl Sink for MySink {
    fn log(&self, record: &Record) -> Result<()> { /* ... */ }
    fn flush(&self) -> Result<()> { /* ... */ }
}

Drawbacks

  • First, this involves breaking changes. Users will need to do some migration work when they upgrade to the new version.
  • Splitting a single Sink into three new items brings complexity and some cognitive overhead for users. We need well-written documentation to explain them.

Unresolved questions

  • Name decision for struct SinkProp, trait GetSinkProp and trait SinkAccess.
  • Is it worth changing the parameter type of XXXSinkBuilder::formatter and SinkProp::set_formatter from Box<dyn Formatter> to F: impl Formatter (by placing Box::new inside the function) to make it easier for callers? (This will bring about an additional breaking change.)

@SpriteOvO
Copy link
Owner Author

@Lancern What do you think about the current design?

@SpriteOvO SpriteOvO requested a review from Lancern August 28, 2025 10:19
Comment on lines +101 to +114
pub fn formatter<'a>(&'a self) -> impl Deref<Target = dyn Formatter> + 'a {
RwLockMappableReadGuard::map(self.formatter.read(), |f| &**f)
}
Copy link
Owner Author

Choose a reason for hiding this comment

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

RPIT is used here so that we can hide what kind of lock we are using as implementation details.

@SpriteOvO SpriteOvO force-pushed the sink-decouple branch 3 times, most recently from 18ef163 to b856f8c Compare August 29, 2025 08:42
@SpriteOvO SpriteOvO added this to the v0.5.0 milestone Sep 2, 2025
@SpriteOvO SpriteOvO force-pushed the sink-decouple branch 9 times, most recently from c001ade to 6bcb357 Compare September 3, 2025 21:28
@SpriteOvO SpriteOvO mentioned this pull request Sep 8, 2025
@SpriteOvO SpriteOvO force-pushed the main-dev branch 4 times, most recently from 2067176 to 69da3ed Compare September 9, 2025 04:39
@SpriteOvO SpriteOvO force-pushed the sink-decouple branch 4 times, most recently from eaa7fb3 to e2d5b1d Compare September 16, 2025 06:18
@SpriteOvO SpriteOvO marked this pull request as ready for review September 16, 2025 06:19
@SpriteOvO SpriteOvO merged commit 9d4c613 into main-dev Sep 16, 2025
84 checks passed
@SpriteOvO SpriteOvO deleted the sink-decouple branch September 16, 2025 06:44
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