tracing-syslog crate #1137
tracing-syslog crate #1137
Conversation
| impl std::ops::BitOr for Options { | ||
| type Output = Self; | ||
| fn bitor(self, rhs: Self) -> Self::Output { | ||
| Self(self.0 | rhs.0) | ||
| } | ||
| } |
There was a problem hiding this comment.
Hmm, not sure if this carries its weight. Why do you think this is valuable?
There was a problem hiding this comment.
So you can set multiple options at the same time:
use tracing_syslog::Options;
// Log PID with messages and log to stderr as well as `syslog`.
let opts = Options::LOG_PID | Options::LOG_PERROR;(added this example to Option's docs)
| fn drop(&mut self) { | ||
| unsafe { libc::closelog() }; | ||
| } |
There was a problem hiding this comment.
Why's this safe? What invarients need to be upheld? If you can, can you add comments explaining why?
There was a problem hiding this comment.
I haven't decided yet how to best handle the case where multiple Syslogs are created (and thus openlog() is called multiple times). Recently created loggers will clobber the identity, facility, and options of previously created loggers, but what's worse is that when one is dropped and calls closelog() it will destroy the identity, facility, and options of the other ones. For these reasons, I think it might be best to have an AtomicBool or similar to keep track of whether or not a logger is currently initialized, and panic if a user tries to initialize multiple concurrently.
| #[cfg(unix)] | ||
| mod syslog; | ||
| #[cfg(unix)] | ||
| pub use syslog::*; |
There was a problem hiding this comment.
It'd be great to have doc comments with examples and tests here.
There was a problem hiding this comment.
I've never been sure about this: for a crate that has a primary type (like Syslog in this case), should the majority of the docs go with that type or in lib.rs?
|
Is there a way to include rustdoc aliases that won't break compilation on pre-1.48 toolchains? |
Yes, we currently use several RustDoc features that are only available on nightly. You can do it like this: #[cfg_attr(docsrs, doc(alias = "..."))]and then add something like this to Cargo.toml: tracing/tracing-core/Cargo.toml Lines 39 to 41 in 27b2877 |
|
Is panicking when the log contents contain an interior nul-terminator the right decision or should we skip logging the message and instead log a warning about it? If we just had static logging messages I'd be okay with panicking, but given that they include dynamic content I wouldn't want to crash someone's program because it was passed a value with a weird debug representation. |
We should probably just |
subscriber: add `MakeWriter::make_writer_for` ## Motivation In some cases, it might be desirable to configure the writer used for writing out trace data based on the metadata of the span or event being written. For example, we might want to send different levels to different outputs, write logs from different targets to separate files, or wrap formatted output in ANSI color codes based on levels. Currently, it's not possible for the `MakeWriter` trait to model this kind of behavior --- it has one method, `make_writer`, which is completely unaware of *where* the data being written came from. In particular, this came up in PR #1137, when discussing a proposal that writing to syslog could be implemented as a `MakeWriter` implementation rather than as a `Subscribe` implementation, so that all the formatting logic from `tracing_subscriber::fmt` could be reused. See [here][1] for details. ## Solution This branch adds a new `make_writer_for` method to `MakeWriter`, taking a `Metadata`. Implementations can opt in to metadata-specific behavior by implementing this method. The method has a default implementation that just calls `self.make_writer()` and ignores the metadata, so it's only necessary to implement this when per-metadata behavior is required. This isn't a breaking change to existing implementations. There are a couple downsides to this approach: it's possible for callers to skip the metadata-specific behavior by calling `make_writer` rather than `make_writer_for`, and the impls for closures can't easily provide metadata-specific behavior. Since the upcoming release is going to be a breaking change anyway, we may want to just make the breaking change of having `MakeWriter::make_writer` _always_ take a `Metadata`, which solves these problems. However, that can't be backported to v0.1.x as easily. Additionally, that would mean that functions like `io::stdout` no longer implement `MakeWriter`; they would have to be wrapped in a wrapper type or closure that ignores metadata. [1]: #1137 (comment) Signed-off-by: Eliza Weisman <[email protected]>
This backports PR #1141 from `master`. subscriber: add `MakeWriter::make_writer_for` ## Motivation In some cases, it might be desirable to configure the writer used for writing out trace data based on the metadata of the span or event being written. For example, we might want to send different levels to different outputs, write logs from different targets to separate files, or wrap formatted output in ANSI color codes based on levels. Currently, it's not possible for the `MakeWriter` trait to model this kind of behavior --- it has one method, `make_writer`, which is completely unaware of *where* the data being written came from. In particular, this came up in PR #1137, when discussing a proposal that writing to syslog could be implemented as a `MakeWriter` implementation rather than as a `Subscribe` implementation, so that all the formatting logic from `tracing_subscriber::fmt` could be reused. See [here][1] for details. ## Solution This branch adds a new `make_writer_for` method to `MakeWriter`, taking a `Metadata`. Implementations can opt in to metadata-specific behavior by implementing this method. The method has a default implementation that just calls `self.make_writer()` and ignores the metadata, so it's only necessary to implement this when per-metadata behavior is required. This isn't a breaking change to existing implementations. There are a couple downsides to this approach: it's possible for callers to skip the metadata-specific behavior by calling `make_writer` rather than `make_writer_for`, and the impls for closures can't easily provide metadata-specific behavior. Since the upcoming release is going to be a breaking change anyway, we may want to just make the breaking change of having `MakeWriter::make_writer` _always_ take a `Metadata`, which solves these problems. However, that can't be backported to v0.1.x as easily. Additionally, that would mean that functions like `io::stdout` no longer implement `MakeWriter`; they would have to be wrapped in a wrapper type or closure that ignores metadata. [1]: #1137 (comment) Signed-off-by: Eliza Weisman <[email protected]>
This backports PR #1141 from `master`. subscriber: add `MakeWriter::make_writer_for` ## Motivation In some cases, it might be desirable to configure the writer used for writing out trace data based on the metadata of the span or event being written. For example, we might want to send different levels to different outputs, write logs from different targets to separate files, or wrap formatted output in ANSI color codes based on levels. Currently, it's not possible for the `MakeWriter` trait to model this kind of behavior --- it has one method, `make_writer`, which is completely unaware of *where* the data being written came from. In particular, this came up in PR #1137, when discussing a proposal that writing to syslog could be implemented as a `MakeWriter` implementation rather than as a `Subscribe` implementation, so that all the formatting logic from `tracing_subscriber::fmt` could be reused. See [here][1] for details. ## Solution This branch adds a new `make_writer_for` method to `MakeWriter`, taking a `Metadata`. Implementations can opt in to metadata-specific behavior by implementing this method. The method has a default implementation that just calls `self.make_writer()` and ignores the metadata, so it's only necessary to implement this when per-metadata behavior is required. This isn't a breaking change to existing implementations. There are a couple downsides to this approach: it's possible for callers to skip the metadata-specific behavior by calling `make_writer` rather than `make_writer_for`, and the impls for closures can't easily provide metadata-specific behavior. Since the upcoming release is going to be a breaking change anyway, we may want to just make the breaking change of having `MakeWriter::make_writer` _always_ take a `Metadata`, which solves these problems. However, that can't be backported to v0.1.x as easily. Additionally, that would mean that functions like `io::stdout` no longer implement `MakeWriter`; they would have to be wrapped in a wrapper type or closure that ignores metadata. [1]: #1137 (comment) Signed-off-by: Eliza Weisman <[email protected]>
|
is switching to using make_writer_for the way forward with this PR? |
Thanks for reminding me @janimo. Yes, |
|
@max-heller @hawkw does this need more changes? |
|
Hello there, |
|
Hi everyone! This PR is in draft for a long time. Does this need more changes? Can I help with some blocks, which doesn't allow release crate officially? |
I think I speak for Eliza when I say this: |
|
I will also mention an important detail: we're looking at removing crates from the tracing monorepo. We'd be okay with creating a dedicated repositories for those crates under the Tokio organization if the original author of that crate doesn't want to be sole maintainer. In the case of tracing-syslog: we're not rejecting the code nor are we saying that it shouldn't exist. We're primarily concerned that changes to |
|
Is there any updates on this? My project could really use |
This is a work-in-progress crate that has not yet been published to crates.io. The source was taken from tokio-rs/tracing#1137 Future updates should remove this crate, once we can pull it from somewhere else.
I would really like to have a |
This is a work-in-progress crate that has not yet been published to crates.io. The source was taken from tokio-rs/tracing#1137 Future updates should remove this crate, once we can pull it from somewhere else.
|
Sorry I've been unresponsive, I've been busy with school and other things and didn't get the chance to test any of this. I got the chance to test it a bit and it seems to work at least for simple use cases, so I've moved the code to max-heller/tracing-syslog. If those that were interested could try it out and let me know if it works for your use-cases, I can publish it to crates.io. Going forward, I'd be willing to help share responsibility for maintenance as @hawkw suggests, so once it's in a usable state we could move it to the |
|
Finally published the crate as |
This backports PR tokio-rs#1141 from `master`. subscriber: add `MakeWriter::make_writer_for` ## Motivation In some cases, it might be desirable to configure the writer used for writing out trace data based on the metadata of the span or event being written. For example, we might want to send different levels to different outputs, write logs from different targets to separate files, or wrap formatted output in ANSI color codes based on levels. Currently, it's not possible for the `MakeWriter` trait to model this kind of behavior --- it has one method, `make_writer`, which is completely unaware of *where* the data being written came from. In particular, this came up in PR tokio-rs#1137, when discussing a proposal that writing to syslog could be implemented as a `MakeWriter` implementation rather than as a `Subscribe` implementation, so that all the formatting logic from `tracing_subscriber::fmt` could be reused. See [here][1] for details. ## Solution This branch adds a new `make_writer_for` method to `MakeWriter`, taking a `Metadata`. Implementations can opt in to metadata-specific behavior by implementing this method. The method has a default implementation that just calls `self.make_writer()` and ignores the metadata, so it's only necessary to implement this when per-metadata behavior is required. This isn't a breaking change to existing implementations. There are a couple downsides to this approach: it's possible for callers to skip the metadata-specific behavior by calling `make_writer` rather than `make_writer_for`, and the impls for closures can't easily provide metadata-specific behavior. Since the upcoming release is going to be a breaking change anyway, we may want to just make the breaking change of having `MakeWriter::make_writer` _always_ take a `Metadata`, which solves these problems. However, that can't be backported to v0.1.x as easily. Additionally, that would mean that functions like `io::stdout` no longer implement `MakeWriter`; they would have to be wrapped in a wrapper type or closure that ignores metadata. [1]: tokio-rs#1137 (comment) Signed-off-by: Eliza Weisman <[email protected]>
Adds a
tracing-syslogcrate to provide support for logging tosyslogvia tracing without resorting to enablingtracing/logand using a crate likeGeal/rust-syslogMotivation
It would be great to be able to log directly to
syslogwithtracing. Users have wanted to do so in the past.Fixes #796
Solution
Adds a
tracing-syslogcrate implementing a simple logger that writes tosyslogusinglibc::syslog.