Geal / rust-syslog

Send syslog messages from Rust
MIT License
110 stars 55 forks source link

Making `err()`, `notice()` &c take a `&mut self` (on Logger) is inconvenient #68

Open sp1ff opened 2 years ago

sp1ff commented 2 years ago

Tokio tracing is a structured logging and diagnostics system that is particularly async-friendly. In particular, it makes a very clear distinction between producing events and consuming them, to the extent that the crate offers no support for consuming them (just producing). tokio-subscriber is one (popular) crate for setting up the consumer-side. It allows one to build consumers up out of "layers" each of which can handle events in different ways.

I'm attempting to use rust-syslog to write such a Layer that sends Events to syslog. My scheme was to have my Layer implementation have a Logger field. In Layer::on_event() (the method I need to implement) I would format the Event and then invoke self.logger.err(msg) (or self.logger.info(msg) &c).

Thing is, on_event() takes an immutable &self. This means that if I store an instance of your Logger in my Layer, I can't send the message off to syslog (since the Logger's methods such as err(), notice() and so forth all require a mutable reference to self).

I searched this repo's issues and TBH I'm surprised this hasn't come up before-- I, at least, was surprised that sending a message through my Logger was a mutating event. One can write to a std::fs::File without needing a &mut self, e.g. Furthermore, I would note that a lot of your backends don't strictly need a mutable reference-- most of them wind up going through a &self (datagram, UdpSocket &c)-- it's only where you use Write & friends that you need the exclusive borrow.

Not sure what I really expect, since changing the the Logger interface would be a majorly breaking change. I guess I just wanted to alert you to this.

FWIW, I tracing-subscriber offers a Layer implementation that writes to file, so I took a peek:

    // format the message into `buf`...
    // Pseudo-code
    let mut writer = &self.file;
   let _ = io::Write::write_all(&mut writer, buf.as_bytes());

The trick here is that Write is implemented for both File and &File (same for TcpStream, btw) and File's impl internally works with &self-- the &mut self is an artifact of the Write interface. So on that call to write_all() they're passing a mutable reference to an immutable reference to File to the Write impl on &File... which promptly drops the mutable borrow and proceeds happily with just a &self.

Reddit discussion here.