Potential improvements for slog.

[dpc]

Hi,

I'm working on slog - structured logging for rust and I'm using this crate have some feature request that would make code in slog-syslog easier.

First, is Box in Box<Logger> really necessary? I wish syslog::Logger was a trait, or was struct Logger<W : std::io::Write> so I can make it to write to Vec<u8>, TcpSstream or just anything implementing std::io::Write.

Second, I wish instead of 3 methods for different formats, there was just Format trait, that serializes stuff writing it to W : io::Write. This way Logger could be struct Logger<W : std::io::Write, F : Format> which would be extensible, easier to use and future-proof.

Anyway, thanks for your crate, initial implementation in slog seems to work. Please let me know what you think.

[Geal]

For the Box, it's a remnant of very old Rust code ( 23b16125c0817c3022aa37fab4bca93fa2767141 ). I started syslog when Rust was still in flux, so I should clean up those old patterns, yes.

Could you elaborate about making Logger a struct Logger<W : std::io::Write>? It should not be too hard to add it to the LoggerBackend enum ( https://github.com/Geal/rust-syslog/blob/master/src/lib.rs#L70-L75 ), but why would you pass a Write instance for a TcpStream when there's already support for TCP? Do you have data to interleave with the logs?

The Format trait is definitely a good idea, I'll look into it.

BTW, I just released the version 3.2.0, with a few bug fixes for log levels and a new init function for systems where the syslog file is in a non standard folder.

[dpc]

It just seem to me that a trait is a better way to handle multiple possibilities. It leaves them open ended. What eg. if user already has a tcp connection, and would like to use it to stream formatted syslog logs? Or someone is doing something completely custom and has it's own struct that does some magic?

It seems to me that syslog crate could be broken down into three separate parts:

• formatting: one trait, multiple implementations, each for different standard
• getting the typical syslog connections;
• actual logger that can work with W where W : LogWrite, with provided implementations for UdpSocket and all types implementing io::Write.

[Geal]

FYI: I'm starting a big refactoring of the library and will push a version 4.0. Most of the design comes from a time when I was learning Rust, I can make it better now :)

Rough plan for now:

• [x] removing the Box (it was there for log::set_logger)
• [x] remove the Arc<Mutex<_>> here and there. I can't remember why I insisted on having methods use &self instead of &mut self
• [x] use error-chain in a few places
• [x] make Logger depend on a Write as you said, but also keep the helper methods for unix sockets and such
• [x] make the formatter a trait too
• [ ] maybe wait for the log refactor to finish and sync to it

[Geal]

right, I remember now why I used &self: the Log trait requires it: https://docs.rs/log/0.3.8/log/trait.Log.html

[Geal]

@dpc ok, the library is now in a more interesting state. Would you like to test it out and see how it can fit with slog? As you requested, the backend to write to, and the formatter, are now generic parameters of the logger.

[dpc]

Awesome. I'll definitely give it a try, though it won't be sooner than in two weeks.

[dpc]

I have found a bit of time to take a look. I think the changes are OK (from slog perspective at least).

It's hard for me to say much more as I did not do my homework about syslog. Eg. I don't understand why formatter types are hidden now. How is "the RFC 3164 or RFC 5424" is supposed to be selected? Is it an automatic choice, etc.?