telemetry [2]: introduce typed command classification

crates/trident/src/lib.rs

@@ -32,6 +32,7 @@ use trident_api::{
 
 pub mod agentconfig;
 pub mod cli;
+mod command_kind;
 mod datastore;
 mod diagnostics;
 mod engine;

crates/trident/src/logging/mod.rs

@@ -5,6 +5,7 @@ pub(super) mod background_log;
 pub(super) mod background_uploader;
 pub(super) mod logfwd;
 pub(super) mod logstream;
+pub(super) mod operation_context;
 pub(super) mod tracestream;
 
 #[derive(Debug, Serialize, Deserialize)]

crates/trident/src/logging/operation_context.rs

@@ -0,0 +1,20 @@
+use trident_api::config::Operations;
+
+/// Maps a base command name plus its requested `Operations` to the same
+/// naming convention gRPC's `servicing_request` already uses for
+/// stage/finalize granularity (e.g. `"install"` vs `"install_stage"` vs
+/// `"install_finalize"`), so `command`/`operation_id` telemetry is
+/// consistent regardless of whether the command came from the CLI or from
+/// gRPC/daemon.
+pub fn command_name(base: &str, ops: &Operations) -> String {
+    match (ops.has_stage(), ops.has_finalize()) {
+        (true, true) => base.to_string(),
+        (true, false) => format!("{base}_stage"),
+        (false, true) => format!("{base}_finalize"),
+        // Neither stage nor finalize was requested (an empty
+        // `--allowed-operations` list); nothing can actually be staged or
+        // finalized, so name this like the existing no-op naming convention
+        // rather than a full install/update.
+        (false, false) => format!("{base}_noop"),
+    }
+}

crates/trident/src/main.rs

@@ -148,10 +148,8 @@ fn run_trident(
                 let agent_config = AgentConfig::load()?;
                 // For non-install and non-update (update will check and has special handling for ACL
                 // scenario) commands, we expect the datastore to exist
-                if !matches!(
-                    args.command,
-                    Commands::Install { .. } | Commands::Update { .. }
-                ) && !agent_config.datastore_path().exists()
+                if !args.command.kind().may_initialize_datastore()
+                    && !agent_config.datastore_path().exists()
                 {
                     return Err(TridentError::new(InvalidInputError::HostNotProvisioned))
                         .message("Datastore file does not exist");

crates/trident/src/server/tridentserver/mod.rs

@@ -25,6 +25,7 @@ use trident_proto::v1::{
 
 use crate::{
     agentconfig::AgentConfig,
+    command_kind::CommandKind,
     logging::logfwd::LogForwarder,
     server::{activitytracker::ActivityTracker, support::stream::StreamWithLock},
     ExitKind, Logstream, TraceStream,
@@ -190,7 +191,7 @@ impl TridentServer {
     /// of responses is produced.
     fn servicing_request<F>(
         &self,
-        name: &'static str,
+        kind: CommandKind,
         reboot_decision: RebootDecision,
         f: F,
     ) -> Result<Response<ServicingResponseStream>, Status>
@@ -200,17 +201,46 @@ impl TridentServer {
             + panic::UnwindSafe
             + 'static,
     {
-        info!("Received servicing request '{}'", name);
+        info!("Received servicing request '{}'", kind.as_str());
 
         // Try to acquire the connection lock in write mode
         let guard = self.try_acquire_write_lock()?;
 
+        // Reject requests that cannot themselves stage a new install/update
+        // (see `CommandKind::may_initialize_datastore`) when no datastore
+        // exists yet -- mirrors the CLI's `HostNotProvisioned` check in
+        // `main.rs`. Without this, e.g. a `commit`/`rollback` RPC arriving
+        // against an unprovisioned host falls through to
+        // `DataStore::open_or_create` in the service handler and silently
+        // creates an empty datastore instead of failing outright.
+        // Untelemetered, same as the lock-busy rejections above: this is
+        // admission control, not a distinct servicing outcome.
+        if !kind.may_initialize_datastore()
+            && !self
+                .agent_config
+                .datastore_path()
+                .try_exists()
+                .map_err(|e| {
+                    error!("Failed to check datastore existence: {e}");
+                    Status::internal("failed to check datastore state")
+                })?
+        {
+            warn!(
+                "Rejected request '{}': datastore does not exist",
+                kind.as_str()
+            );
+            return Err(Status::failed_precondition("Host is not provisioned"));
+        }
+
         // Create the gRPC response channel
         let (tx, rx) = mpsc::unbounded_channel();
 
         // Try to acquire the servicing lock
         let Some(servicing_guard) = self.servicing_manager.try_lock_servicing() else {
-            warn!("Request '{}' blocked because servicing is active", name);
+            warn!(
+                "Request '{}' blocked because servicing is active",
+                kind.as_str()
+            );
             return Err(Status::unavailable("Servicing is active"));
         };
 
@@ -232,6 +262,7 @@ impl TridentServer {
         let logstream = self.logstream.clone();
         let tracestream = self.tracestream.clone();
         let manager = self.servicing_manager.clone();
+        let name_for_async = kind.as_str().to_string();
 
         // Spawn the servicing task
         tokio::spawn(async move {
@@ -249,9 +280,15 @@ impl TridentServer {
             }
 
             if let Some(ref err) = completed_message.error {
-                error!("Servicing request '{}' failed: {}", name, err.message);
+                error!(
+                    "Servicing request '{}' failed: {}",
+                    name_for_async, err.message
+                );
             } else {
-                info!("Servicing request '{}' completed successfully", name);
+                info!(
+                    "Servicing request '{}' completed successfully",
+                    name_for_async
+                );
             }
 
             // Stop log forwarding
@@ -276,7 +313,7 @@ impl TridentServer {
             // already been stopped.
             drop(tx);
 
-            info!("Request '{}' completed", name);
+            info!("Request '{}' completed", name_for_async);
         });
 
         // Return the streaming response with the lock guard

crates/trident/src/server/tridentserver/services/commit.rs

@@ -12,6 +12,7 @@ use trident_proto::v1preview::{
 };
 
 use crate::{
+    command_kind::CommandKind,
     server::{
         tridentserver::{datastore, ServicingResponseStream},
         TridentServer,
@@ -35,21 +36,26 @@ impl CommitService for TridentServer {
         let logstream = self.logstream.clone();
         let tracestream = self.tracestream.clone();
 
-        self.servicing_request("commit", super::reboot_allowed(&req.reboot), move || {
-            let mut trident: Trident = Trident::new(None, &data_store_path, logstream, tracestream)
-                .message("Failed to initialize Trident")?;
+        self.servicing_request(
+            CommandKind::commit(),
+            super::reboot_allowed(&req.reboot),
+            move || {
+                let mut trident: Trident =
+                    Trident::new(None, &data_store_path, logstream, tracestream)
+                        .message("Failed to initialize Trident")?;
 
-            let mut datastore =
-                DataStore::open_or_create(&data_store_path).message("Failed to open datastore")?;
+                let mut datastore = DataStore::open_or_create(&data_store_path)
+                    .message("Failed to open datastore")?;
 
-            let image_hash = datastore::stored_image_hash(&datastore);
+                let image_hash = datastore::stored_image_hash(&datastore);
 
-            trident
-                .commit(&mut datastore)
-                .map(|(exit_kind, servicing_type)| {
-                    (exit_kind, image_hash, Some(servicing_type.into()))
-                })
-        })
+                trident
+                    .commit(&mut datastore)
+                    .map(|(exit_kind, servicing_type)| {
+                        (exit_kind, image_hash, Some(servicing_type.into()))
+                    })
+            },
+        )
     }
 }
 
@@ -61,7 +67,7 @@ impl CommitServicePreview for TridentServer {
         &self,
         _request: Request<CheckRootRequest>,
     ) -> Result<Response<Self::CheckRootStream>, Status> {
-        self.servicing_request("check_root", RebootDecision::Error, || {
+        self.servicing_request(CommandKind::check_root(), RebootDecision::Error, || {
             Err(TridentError::new(InternalError::Internal(
                 "Not implemented: check_root",
             )))

crates/trident/src/server/tridentserver/services/install.rs

@@ -10,6 +10,7 @@ use trident_proto::v1preview::{
 };
 
 use crate::{
+    command_kind::CommandKind,
     server::{
         tridentserver::{RebootDecision, ServicingResponseStream},
         TridentServer,
@@ -44,7 +45,7 @@ impl InstallService for TridentServer {
         let tracestream = self.tracestream.clone();
 
         self.servicing_request(
-            "install",
+            CommandKind::install(),
             super::reboot_allowed(&finalize.reboot),
             move || {
                 let mut trident = Trident::new(
@@ -82,22 +83,26 @@ impl InstallService for TridentServer {
         let logstream = self.logstream.clone();
         let tracestream = self.tracestream.clone();
 
-        self.servicing_request("install_stage", RebootDecision::Error, move || {
-            let mut trident = Trident::new(
-                Some(HostConfigurationSource::RawString(host_config.config)),
-                &data_store_path,
-                logstream,
-                tracestream,
-            )
-            .message("Failed to initialize Trident")?;
-
-            let mut datastore =
-                DataStore::open_or_create(&data_store_path).message("Failed to open datastore")?;
-
-            trident
-                .install(&mut datastore, Operation::Stage.into(), false, None)
-                .map(|(k, h, st)| (k, h, Some(st.into())))
-        })
+        self.servicing_request(
+            CommandKind::install_stage(),
+            RebootDecision::Error,
+            move || {
+                let mut trident = Trident::new(
+                    Some(HostConfigurationSource::RawString(host_config.config)),
+                    &data_store_path,
+                    logstream,
+                    tracestream,
+                )
+                .message("Failed to initialize Trident")?;
+
+                let mut datastore = DataStore::open_or_create(&data_store_path)
+                    .message("Failed to open datastore")?;
+
+                trident
+                    .install(&mut datastore, Operation::Stage.into(), false, None)
+                    .map(|(k, h, st)| (k, h, Some(st.into())))
+            },
+        )
     }
 
     type InstallFinalizeStream = ServicingResponseStream;
@@ -112,7 +117,7 @@ impl InstallService for TridentServer {
         let tracestream = self.tracestream.clone();
 
         self.servicing_request(
-            "install_finalize",
+            CommandKind::install_finalize(),
             super::reboot_allowed(&finalize.reboot),
             move || {
                 let mut trident = Trident::new(None, &data_store_path, logstream, tracestream)

crates/trident/src/server/tridentserver/services/rebuild_raid.rs

@@ -6,6 +6,7 @@ use trident_proto::v1preview::{
 };
 
 use crate::{
+    command_kind::CommandKind,
     server::{
         tridentserver::{datastore, RebootDecision, ServicingResponseStream},
         TridentServer,
@@ -24,20 +25,24 @@ impl RebuildRaidService for TridentServer {
         let logstream = self.logstream.clone();
         let tracestream = self.tracestream.clone();
 
-        self.servicing_request("rebuild_raid", RebootDecision::Error, move || {
-            let mut trident = Trident::new(None, &data_store_path, logstream, tracestream)
-                .message("Failed to initialize Trident")?;
+        self.servicing_request(
+            CommandKind::rebuild_raid(),
+            RebootDecision::Error,
+            move || {
+                let mut trident = Trident::new(None, &data_store_path, logstream, tracestream)
+                    .message("Failed to initialize Trident")?;
 
-            let mut datastore =
-                DataStore::open_or_create(&data_store_path).message("Failed to open datastore")?;
+                let mut datastore = DataStore::open_or_create(&data_store_path)
+                    .message("Failed to open datastore")?;
 
-            let image_hash = datastore::stored_image_hash(&datastore);
+                let image_hash = datastore::stored_image_hash(&datastore);
 
-            trident
-                .rebuild_raid(&mut datastore)
-                .message("Failed to rebuild RAID arrays")?;
+                trident
+                    .rebuild_raid(&mut datastore)
+                    .message("Failed to rebuild RAID arrays")?;
 
-            Ok((ExitKind::Done, image_hash, None))
-        })
+                Ok((ExitKind::Done, image_hash, None))
+            },
+        )
     }
 }

crates/trident/src/server/tridentserver/services/rollback.rs

@@ -24,6 +24,7 @@ use crate::engine::manual_rollback::utils::{
     ManualRollbackRequestKind,
 };
 use crate::{
+    command_kind::CommandKind,
     server::{
         tridentserver::{RebootDecision, ServicingResponseStream},
         TridentServer,
@@ -71,7 +72,7 @@ impl RollbackService for TridentServer {
         let tracestream = self.tracestream.clone();
 
         self.servicing_request(
-            "rollback",
+            CommandKind::rollback(),
             super::reboot_allowed(&finalize.reboot),
             move || {
                 let mut trident: Trident =
@@ -109,25 +110,32 @@ impl RollbackService for TridentServer {
         let logstream = self.logstream.clone();
         let tracestream = self.tracestream.clone();
 
-        self.servicing_request("rollback_stage", RebootDecision::Error, move || {
-            let mut trident: Trident = Trident::new(None, &data_store_path, logstream, tracestream)
-                .message("Failed to initialize Trident")?;
+        self.servicing_request(
+            CommandKind::rollback_stage(),
+            RebootDecision::Error,
+            move || {
+                let mut trident: Trident =
+                    Trident::new(None, &data_store_path, logstream, tracestream)
+                        .message("Failed to initialize Trident")?;
 
-            let mut datastore =
-                DataStore::open_or_create(&data_store_path).message("Failed to open datastore")?;
+                let mut datastore = DataStore::open_or_create(&data_store_path)
+                    .message("Failed to open datastore")?;
 
-            let (invoke_if_next_is_runtime, invoke_available_ab) =
-                manual_rollback_flags(req.kind())?;
-
-            trident
-                .rollback(
-                    &mut datastore,
-                    invoke_if_next_is_runtime,
-                    invoke_available_ab,
-                    Operation::Stage.into(),
-                )
-                .map(|(exit_kind, servicing_type)| (exit_kind, None, Some(servicing_type.into())))
-        })
+                let (invoke_if_next_is_runtime, invoke_available_ab) =
+                    manual_rollback_flags(req.kind())?;
+
+                trident
+                    .rollback(
+                        &mut datastore,
+                        invoke_if_next_is_runtime,
+                        invoke_available_ab,
+                        Operation::Stage.into(),
+                    )
+                    .map(|(exit_kind, servicing_type)| {
+                        (exit_kind, None, Some(servicing_type.into()))
+                    })
+            },
+        )
     }
 
     type RollbackFinalizeStream = ServicingResponseStream;
@@ -142,7 +150,7 @@ impl RollbackService for TridentServer {
         let tracestream = self.tracestream.clone();
 
         self.servicing_request(
-            "rollback_finalize",
+            CommandKind::rollback_finalize(),
             super::reboot_allowed(&finalize.reboot),
             move || {
                 let mut trident: Trident =