chore: addressing review feedback

prompting-client/src/bin/set_log_level.rs

@@ -1,22 +1,43 @@
+//! This is a helper command for setting our logging filter within the daemon while it is running.
+//! Doing this involves having to send a GRPC message to the daemon which inolves more plumbing
+//! than we'd ideally like but it allows us to update the filter without having to restart the
+//! daemon.
+//!
+//! The docs on the command itself cover how to set simple logging levels but the tracing framework
+//! we are using actually supports a rich syntax for how these filters are written. As and when we
+//! need to use this in anger, the docs can be found here:
+//!   https://docs.rs/tracing-subscriber/latest/tracing_subscriber/filter/struct.EnvFilter.html
 use clap::Parser;
-use prompting_client::cli_actions::set_log_level;
+use prompting_client::cli_actions::set_logging_filter;
 use std::process::exit;
 
-/// Set the logging level for a running instance of the prompting client daemon
+/// Set the logging level for a running instance of the prompting client daemon.
+///
+/// Simple usage of this command involves specifying a level based filter for what logs are written
+/// to the system journal by the prompting-client daemon. The supported levels are (in order of
+/// least to most verbose):
+///
+///   - error
+///   - warn
+///   - info
+///   - debug
+///   - trace
+///
+/// The default level is "info".
 #[derive(Debug, Parser)]
 #[clap(about, long_about = None)]
 struct Args {
-    /// The filter to use for setting the logging level
+    /// The filter to use for determining what gets logged
     #[clap(short, long, value_name = "FILTER")]
-    level: String,
+    filter: String,
 }
 
 #[tokio::main]
 async fn main() {
-    let Args { level } = Args::parse();
+    let Args { filter } = Args::parse();
 
-    match set_log_level(level).await {
-        Ok(current) => println!("logging level set to: {current}"),
+    match set_logging_filter(filter).await {
+        Ok(current) => println!("logging filter set to: {current}"),
         Err(e) => {
             eprintln!("{e}");
             exit(1);

prompting-client/src/cli_actions/log_level.rs

@@ -9,12 +9,12 @@ use tokio::net::UnixStream;
 use tonic::transport::{Channel, Endpoint, Uri};
 use tower::service_fn;
 
-pub async fn set_log_level(level: String) -> Result<String> {
+pub async fn set_logging_filter(filter: String) -> Result<String> {
     let mut client = client_from_env().await;
 
-    match client.set_logging_level(level).await {
+    match client.set_logging_filter(filter).await {
         Ok(resp) => Ok(resp.into_inner().current),
-        Err(e) => Err(Error::UnableToUpdateLogLevel {
+        Err(e) => Err(Error::UnableToUpdateLogFilter {
             reason: e.to_string(),
         }),
     }

prompting-client/src/cli_actions/mod.rs

@@ -3,5 +3,5 @@ mod log_level;
 mod scripted;
 
 pub use echo_loop::run_echo_loop;
-pub use log_level::set_log_level;
+pub use log_level::set_logging_filter;
 pub use scripted::run_scripted_client_loop;

prompting-client/src/daemon/server.rs

@@ -6,7 +6,7 @@ use crate::{
         apparmor_prompting::{
             self, get_current_prompt_response::Prompt, home_prompt::PatternOption,
             prompt_reply::PromptReply::HomePromptReply, prompt_reply_response::PromptReplyType,
-            HomePatternType, MetaData, PromptReply, SetLoggingLevelResponse,
+            HomePatternType, MetaData, PromptReply, SetLoggingFilterResponse,
         },
         AppArmorPrompting, AppArmorPromptingServer, GetCurrentPromptResponse, HomePrompt,
         PromptReplyResponse, ResolveHomePatternTypeResponse,
@@ -54,7 +54,7 @@ pub fn new_server_and_listener<R, S>(
 ) -> (AppArmorPromptingServer<Service<R, S>>, UnixListener)
 where
     R: ReplyToPrompt + Clone,
-    S: SetLogLevel,
+    S: SetLogFilter,
 {
     let service = Service::new(
         client.clone(),
@@ -67,11 +67,11 @@ where
     (AppArmorPromptingServer::new(service), listener)
 }
 
-pub trait SetLogLevel: Send + Sync + 'static {
-    fn set_filter(&self, level: &str) -> crate::Result<()>;
+pub trait SetLogFilter: Send + Sync + 'static {
+    fn set_filter(&self, filter: &str) -> crate::Result<()>;
 }
 
-impl<L, S> SetLogLevel for Arc<Handle<L, S>>
+impl<L, S> SetLogFilter for Arc<Handle<L, S>>
 where
     L: From<EnvFilter> + Send + Sync + 'static,
     S: 'static,
@@ -80,11 +80,11 @@ where
         info!(?filter, "attempting to update logging filter");
         let f = filter
             .parse::<EnvFilter>()
-            .map_err(|_| Error::UnableToUpdateLogLevel {
+            .map_err(|_| Error::UnableToUpdateLogFilter {
                 reason: format!("{filter:?} is not a valid logging filter"),
             })?;
 
-        self.reload(f).map_err(|e| Error::UnableToUpdateLogLevel {
+        self.reload(f).map_err(|e| Error::UnableToUpdateLogFilter {
             reason: format!("failed to set logging filter: {e}"),
         })?;
 
@@ -95,7 +95,7 @@ where
 pub struct Service<R, S>
 where
     R: ReplyToPrompt,
-    S: SetLogLevel,
+    S: SetLogFilter,
 {
     client: R,
     reload_handle: S,
@@ -106,7 +106,7 @@ where
 impl<R, S> Service<R, S>
 where
     R: ReplyToPrompt,
-    S: SetLogLevel,
+    S: SetLogFilter,
 {
     pub fn new(
         client: R,
@@ -133,7 +133,7 @@ where
 impl<R, S> AppArmorPrompting for Service<R, S>
 where
     R: ReplyToPrompt,
-    S: SetLogLevel,
+    S: SetLogFilter,
 {
     async fn get_current_prompt(
         &self,
@@ -210,14 +210,14 @@ where
         ))
     }
 
-    async fn set_logging_level(
+    async fn set_logging_filter(
         &self,
-        level: Request<String>,
-    ) -> Result<Response<SetLoggingLevelResponse>, Status> {
-        let current = log_filter(&level.into_inner());
+        filter: Request<String>,
+    ) -> Result<Response<SetLoggingFilterResponse>, Status> {
+        let current = log_filter(&filter.into_inner());
 
         match self.reload_handle.set_filter(&current) {
-            Ok(_) => Ok(Response::new(SetLoggingLevelResponse { current })),
+            Ok(_) => Ok(Response::new(SetLoggingFilterResponse { current })),
             Err(e) => Err(Status::new(
                 Code::InvalidArgument,
                 format!("unable to set logging level: {e}"),
@@ -406,7 +406,7 @@ mod tests {
     }
 
     struct MockReloadHandle;
-    impl SetLogLevel for MockReloadHandle {
+    impl SetLogFilter for MockReloadHandle {
         fn set_filter(&self, level: &str) -> crate::Result<()> {
             panic!("attempt to set log level to {level}");
         }

prompting-client/src/lib.rs

@@ -81,8 +81,8 @@ pub enum Error {
     #[error("{interface} is not currently supported for apparmor prompting")]
     UnsupportedInterface { interface: String },
 
-    #[error("unable to update log level: {reason}")]
-    UnableToUpdateLogLevel { reason: String },
+    #[error("unable to update log filter: {reason}")]
+    UnableToUpdateLogFilter { reason: String },
 }
 
 pub type Result<T> = std::result::Result<T, Error>;

prompting-client/src/protos/apparmor_prompting.rs

@@ -148,7 +148,7 @@ pub struct ResolveHomePatternTypeResponse {
 }
 #[allow(clippy::derive_partial_eq_without_eq)]
 #[derive(Clone, PartialEq, ::prost::Message)]
-pub struct SetLoggingLevelResponse {
+pub struct SetLoggingFilterResponse {
     #[prost(string, tag = "1")]
     pub current: ::prost::alloc::string::String,
 }
@@ -423,11 +423,11 @@ pub mod app_armor_prompting_client {
                 );
             self.inner.unary(req, path, codec).await
         }
-        pub async fn set_logging_level(
+        pub async fn set_logging_filter(
             &mut self,
             request: impl tonic::IntoRequest<::prost::alloc::string::String>,
         ) -> std::result::Result<
-            tonic::Response<super::SetLoggingLevelResponse>,
+            tonic::Response<super::SetLoggingFilterResponse>,
             tonic::Status,
         > {
             self.inner
@@ -441,14 +441,14 @@ pub mod app_armor_prompting_client {
                 })?;
             let codec = tonic::codec::ProstCodec::default();
             let path = http::uri::PathAndQuery::from_static(
-                "/apparmor_prompting.AppArmorPrompting/SetLoggingLevel",
+                "/apparmor_prompting.AppArmorPrompting/SetLoggingFilter",
             );
             let mut req = request.into_request();
             req.extensions_mut()
                 .insert(
                     GrpcMethod::new(
                         "apparmor_prompting.AppArmorPrompting",
-                        "SetLoggingLevel",
+                        "SetLoggingFilter",
                     ),
                 );
             self.inner.unary(req, path, codec).await
@@ -483,11 +483,11 @@ pub mod app_armor_prompting_server {
             tonic::Response<super::ResolveHomePatternTypeResponse>,
             tonic::Status,
         >;
-        async fn set_logging_level(
+        async fn set_logging_filter(
             &self,
             request: tonic::Request<::prost::alloc::string::String>,
         ) -> std::result::Result<
-            tonic::Response<super::SetLoggingLevelResponse>,
+            tonic::Response<super::SetLoggingFilterResponse>,
             tonic::Status,
         >;
     }
@@ -706,14 +706,14 @@ pub mod app_armor_prompting_server {
                     };
                     Box::pin(fut)
                 }
-                "/apparmor_prompting.AppArmorPrompting/SetLoggingLevel" => {
+                "/apparmor_prompting.AppArmorPrompting/SetLoggingFilter" => {
                     #[allow(non_camel_case_types)]
-                    struct SetLoggingLevelSvc<T: AppArmorPrompting>(pub Arc<T>);
+                    struct SetLoggingFilterSvc<T: AppArmorPrompting>(pub Arc<T>);
                     impl<
                         T: AppArmorPrompting,
                     > tonic::server::UnaryService<::prost::alloc::string::String>
-                    for SetLoggingLevelSvc<T> {
-                        type Response = super::SetLoggingLevelResponse;
+                    for SetLoggingFilterSvc<T> {
+                        type Response = super::SetLoggingFilterResponse;
                         type Future = BoxFuture<
                             tonic::Response<Self::Response>,
                             tonic::Status,
@@ -724,7 +724,10 @@ pub mod app_armor_prompting_server {
                         ) -> Self::Future {
                             let inner = Arc::clone(&self.0);
                             let fut = async move {
-                                <T as AppArmorPrompting>::set_logging_level(&inner, request)
+                                <T as AppArmorPrompting>::set_logging_filter(
+                                        &inner,
+                                        request,
+                                    )
                                     .await
                             };
                             Box::pin(fut)
@@ -736,7 +739,7 @@ pub mod app_armor_prompting_server {
                     let max_encoding_message_size = self.max_encoding_message_size;
                     let inner = self.inner.clone();
                     let fut = async move {
-                        let method = SetLoggingLevelSvc(inner);
+                        let method = SetLoggingFilterSvc(inner);
                         let codec = tonic::codec::ProstCodec::default();
                         let mut grpc = tonic::server::Grpc::new(codec)
                             .apply_compression_config(

protos/apparmor-prompting.proto

@@ -9,7 +9,7 @@ service AppArmorPrompting {
     rpc GetCurrentPrompt (google.protobuf.Empty) returns (GetCurrentPromptResponse);
     rpc ReplyToPrompt (PromptReply) returns (PromptReplyResponse);
     rpc ResolveHomePatternType (google.protobuf.StringValue) returns (ResolveHomePatternTypeResponse);
-    rpc SetLoggingLevel (google.protobuf.StringValue) returns (SetLoggingLevelResponse);
+    rpc SetLoggingFilter (google.protobuf.StringValue) returns (SetLoggingFilterResponse);
 }
 
 message PromptReply {
@@ -91,6 +91,6 @@ message ResolveHomePatternTypeResponse {
    HomePatternType home_pattern_type = 1; 
 }
 
-message SetLoggingLevelResponse {
+message SetLoggingFilterResponse {
     string current = 1;
 }