From 864a481270c428ea95427c8396593c93eb7e723a Mon Sep 17 00:00:00 2001 From: Hayden Stainsby Date: Mon, 29 Jul 2024 09:15:02 +0200 Subject: [PATCH] fix(subscriber): remove unused `AggregatorHandle` and fix other lints A number of new or updated Clippy lints in Rust 1.80.0 need to be fixed. An update to the `dead_code` pointed out that the `AggregatorHandle` is not used, and it is not constructable from outside the crate because it has a private field. This struct was introduced in #451 as part of the `Server::into_parts` method. Originally, this method was going to return the `AggregatorHandle`, which wrapped the join handle from the task where the `Aggregator` had been spawned. This was later replaced by returning the `Aggregator` itself, which the user had the obligation to spawn themselves. However, it seems that the `AggregatorHandle` wasn't removed, even though it was never used. A new lint is the one for unexpected `--cfg` items. We now need to declare those in `Cargo.toml`. An update to `needless_borrows_for_generic_args` causes a false positive changing a `&mut` to a move, which we can't do as the same value is used afterwards. --- console-subscriber/Cargo.toml | 4 ++++ console-subscriber/README.md | 8 +++---- console-subscriber/src/lib.rs | 37 +------------------------------- console-subscriber/src/record.rs | 3 +++ 4 files changed, 12 insertions(+), 40 deletions(-) diff --git a/console-subscriber/Cargo.toml b/console-subscriber/Cargo.toml index 82fb52cd0..f33718e04 100644 --- a/console-subscriber/Cargo.toml +++ b/console-subscriber/Cargo.toml @@ -63,6 +63,10 @@ futures = "0.3" http = "1.1" tower-http = { version = "0.5", features = ["cors"] } +[lints.rust.unexpected_cfgs] +level = "warn" +check-cfg = [ 'cfg(tokio_unstable)', 'cfg(console_without_tokio_unstable)' ] + [package.metadata.docs.rs] all-features = true rustdoc-args = ["--cfg", "docsrs"] diff --git a/console-subscriber/README.md b/console-subscriber/README.md index e9546d23d..4d8d77dbe 100644 --- a/console-subscriber/README.md +++ b/console-subscriber/README.md @@ -94,12 +94,12 @@ runtime][Tokio] is considered *experimental*. In order to use level]. + If you're using the [`console_subscriber::init()`][init] or - [`console_subscriber::Builder`][builder] APIs, these targets are enabled - automatically. + [`console_subscriber::Builder`][builder] APIs, these targets are enabled + automatically. + If you are manually configuring the `tracing` subscriber using the - [`EnvFilter`] or [`Targets`] filters from [`tracing-subscriber`], add - `"tokio=trace,runtime=trace"` to your filter configuration. + [`EnvFilter`] or [`Targets`] filters from [`tracing-subscriber`], add + `"tokio=trace,runtime=trace"` to your filter configuration. + Also, ensure you have not enabled any of the [compile time filter features][compile_time_filters] in your `Cargo.toml`. diff --git a/console-subscriber/src/lib.rs b/console-subscriber/src/lib.rs index fc2552f82..aa0085796 100644 --- a/console-subscriber/src/lib.rs +++ b/console-subscriber/src/lib.rs @@ -15,10 +15,7 @@ use std::{ use thread_local::ThreadLocal; #[cfg(unix)] use tokio::net::UnixListener; -use tokio::{ - sync::{mpsc, oneshot}, - task::JoinHandle, -}; +use tokio::sync::{mpsc, oneshot}; #[cfg(unix)] use tokio_stream::wrappers::UnixListenerStream; use tracing_core::{ @@ -1187,38 +1184,6 @@ pub struct ServerParts { pub aggregator: Aggregator, } -/// Aggregator handle. -/// -/// This object is returned from [`Server::into_parts`]. It can be -/// used to abort the aggregator task. -/// -/// The aggregator collects the traces that implement the async runtime -/// being observed and prepares them to be served by the gRPC server. -/// -/// Normally, if the server, started with [`Server::serve`] or -/// [`Server::serve_with`] stops for any reason, the aggregator is aborted, -/// hoewver, if the server was started with the [`InstrumentServer`] returned -/// from [`Server::into_parts`], then it is the responsibility of the user -/// of the API to stop the aggregator task by calling [`abort`] on this -/// object. -/// -/// [`abort`]: fn@crate::AggregatorHandle::abort -pub struct AggregatorHandle { - join_handle: JoinHandle<()>, -} - -impl AggregatorHandle { - /// Aborts the task running this aggregator. - /// - /// To avoid having a disconnected aggregator running forever, this - /// method should be called when the [`tonic::transport::Server`] started - /// with the [`InstrumentServer`] also returned from [`Server::into_parts`] - /// stops running. - pub fn abort(&mut self) { - self.join_handle.abort(); - } -} - #[tonic::async_trait] impl proto::instrument::instrument_server::Instrument for Server { type WatchUpdatesStream = diff --git a/console-subscriber/src/record.rs b/console-subscriber/src/record.rs index d58c7b4d3..8fb2239c9 100644 --- a/console-subscriber/src/record.rs +++ b/console-subscriber/src/record.rs @@ -83,6 +83,9 @@ fn record_io(file: File, rx: Receiver) -> io::Result<()> { use std::io::{BufWriter, Write}; fn write(mut file: &mut BufWriter, val: &T) -> io::Result<()> { + // Clippy throws a false positive here. We can't actually pass the owned `file` to + // `to_writer` because we need it again in the line blow. + #[allow(clippy::needless_borrows_for_generic_args)] serde_json::to_writer(&mut file, val)?; file.write_all(b"\n") }