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

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

Open
sp1ff opened this issue Jun 2, 2022 · 0 comments
Open

Comments

@sp1ff
Copy link

sp1ff commented Jun 2, 2022

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.

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

1 participant