Tracing appender tests #678
Conversation
|
Few additional notes:
|
Clippy doesn't catch the .bytes() warnings locally for me unfortunantely, so I have to rely on the clippy checker here. I need to also find a way to make sure Edit: Realized that I needed to run |
|
Looks like one of the tests hangs, Edit: |
|
CI failures look like crates.io was returning 500s for a bit; I restarted them. |
|
Rebased |
tracing-appender/src/lib.rs
Outdated
|
|
||
| #[derive(Debug)] | ||
| pub(crate) enum Msg { | ||
| Line(Vec<u8>), | ||
| Shutdown, | ||
| } |
There was a problem hiding this comment.
I forgot about this: dropping a sender is equivalent to a shutdown notice.
(I'm not treating this as public, as this is not in the public API and we can tidy up later.)
|
I'd really prefer it if the change that fixes the bug could land in a separate PR from the change that adds tests, so that the Git history is clearer — @zekisherif, would you mind moving that into a separate branch? |
Sure I can do that. |
|
Once this is merged in, I'll raise a PR for the bug fix |
I think we should merge the bugfix first to avoid having a broken build on master. |
## Motivation Fixes a race condition which occurs on dropping of `WorkerGuard`. If the worker thread missed seeing the shutdown signal in time, it would end up blocking on trying to call `recv()` on the crossbeam channel and block indefinitely. This bug was identified in #678 ## Solution Signal that the worker should stop by sending a `ShutDown` message through the crossbeam channel. Co-authored-by: Zeki Sherif <zekshi@amazon.com>
Co-Authored-By: Eliza Weisman <eliza@buoyant.io>
Co-authored-by: Eliza Weisman <eliza@buoyant.io>
Following the release process for https://github.com/tokio-rs/tracing/blob/master/CONTRIBUTING.md and these changes are the only things left to commit before release. I'd like to hopefully get this released today so we can start using this crate internally. Note: I need to get PR #703 and PR #678 merged before release. Co-authored-by: Zeki Sherif <zekshi@amazon.com> Co-authored-by: Eliza Weisman <eliza@buoyant.io>
Motivation
Adding tests for PR #673