Skip to content

Commit

Permalink
fix(subscriber): remove unused AggregatorHandle and fix other lints
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
hds committed Jul 29, 2024
1 parent 9205e15 commit 864a481
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 40 deletions.
4 changes: 4 additions & 0 deletions console-subscriber/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Expand Down
8 changes: 4 additions & 4 deletions console-subscriber/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down
37 changes: 1 addition & 36 deletions console-subscriber/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -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 =
Expand Down
3 changes: 3 additions & 0 deletions console-subscriber/src/record.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,9 @@ fn record_io(file: File, rx: Receiver<Event>) -> io::Result<()> {
use std::io::{BufWriter, Write};

fn write<T: Serialize>(mut file: &mut BufWriter<File>, 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")
}
Expand Down

0 comments on commit 864a481

Please sign in to comment.