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.

One of them pointed out that the `AggregatorHandle` is not used, and is
also not constructable from outside the crate. 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 0024529
Show file tree
Hide file tree
Showing 4 changed files with 11 additions and 36 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
32 changes: 0 additions & 32 deletions console-subscriber/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1187,38 +1187,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 0024529

Please sign in to comment.