chore(instrumentation): Reduce noise from instrumented functions

NickLarsenNZ · NickLarsenNZ · commit 367d5a3a0d67 · 2025-03-20T13:47:27.000+01:00
diff --git a/rust/stackable-cockpit/src/engine/docker/mod.rs b/rust/stackable-cockpit/src/engine/docker/mod.rs
@@ -19,7 +19,7 @@ pub enum Error {
 }
 
 /// Checks if Docker is running on the system
-#[instrument]
+#[instrument(skip_all)]
 pub async fn check_if_docker_is_running() -> Result<()> {
     debug!("Checking if Docker is running");
 
diff --git a/rust/stackable-cockpit/src/engine/kind/mod.rs b/rust/stackable-cockpit/src/engine/kind/mod.rs
@@ -64,7 +64,7 @@ impl Cluster {
     }
 
     /// Create a new local cluster by calling the kind binary.
-    #[instrument]
+    #[instrument(skip_all)]
     pub async fn create(&self) -> Result<()> {
         info!("Creating local cluster using kind");
 
@@ -109,7 +109,7 @@ impl Cluster {
     }
 
     /// Creates a kind cluster if it doesn't exist already.
-    #[instrument]
+    #[instrument(skip_all)]
     pub async fn create_if_not_exists(&self) -> Result<()> {
         info!("Creating cluster if it doesn't exist using kind");
 
@@ -131,7 +131,7 @@ impl Cluster {
     }
 
     /// Check if a kind cluster with the provided name already exists.
-    #[instrument]
+    #[instrument(skip_all)]
     async fn check_if_cluster_exists(cluster_name: &str) -> Result<bool> {
         debug!("Checking if kind cluster exists");
 
diff --git a/rust/stackable-cockpit/src/engine/minikube/mod.rs b/rust/stackable-cockpit/src/engine/minikube/mod.rs
@@ -41,7 +41,7 @@ impl Cluster {
     }
 
     /// Create a new local cluster by calling the Minikube binary
-    #[instrument]
+    #[instrument(skip_all)]
     pub async fn create(&self) -> Result<(), Error> {
         info!("Creating local cluster using Minikube");
 
@@ -70,7 +70,7 @@ impl Cluster {
     }
 
     /// Creates a Minikube cluster if it doesn't exist already.
-    #[instrument]
+    #[instrument(skip_all)]
     pub async fn create_if_not_exists(&self) -> Result<(), Error> {
         info!("Creating cluster if it doesn't exist using Minikube");
 
@@ -92,7 +92,7 @@ impl Cluster {
     }
 
     /// Check if a kind cluster with the provided name already exists.
-    #[instrument]
+    #[instrument(skip_all)]
     async fn check_if_cluster_exists(cluster_name: &str) -> Result<bool, Error> {
         debug!("Checking if Minikube cluster exists");
 
diff --git a/rust/stackable-cockpit/src/helm.rs b/rust/stackable-cockpit/src/helm.rs
@@ -182,7 +182,7 @@ pub struct ChartVersion<'a> {
 ///
 /// This function expects the fully qualified Helm release name. In case of our
 /// operators this is: `<PRODUCT_NAME>-operator`.
-#[instrument]
+#[instrument(skip(values_yaml), fields(with_values = values_yaml.is_some()))]
 pub fn install_release_from_repo_or_registry(
     release_name: &str,
     ChartVersion {
@@ -239,8 +239,8 @@ pub fn install_release_from_repo_or_registry(
         let chart_version = chart_version.unwrap_or(HELM_DEFAULT_CHART_VERSION);
 
         debug!(
-            "Installing Helm release {} ({}) from chart {}",
-            release_name, chart_version, full_chart_name
+            release_name,
+            chart_version, full_chart_name, "Installing Helm release"
         );
 
         install_release(
@@ -260,6 +260,7 @@ pub fn install_release_from_repo_or_registry(
 ///
 /// This function expects the fully qualified Helm release name. In case of our
 /// operators this is: `<PRODUCT_NAME>-operator`.
+#[instrument(fields(with_values = values_yaml.is_some()))]
 fn install_release(
     release_name: &str,
     chart_name: &str,
@@ -388,10 +389,10 @@ pub fn add_repo(repository_name: &str, repository_url: &str) -> Result<(), Error
 }
 
 /// Retrieves the Helm index file from the repository URL.
-#[instrument]
+#[instrument(skip_all, fields(%repo_url))]
 pub async fn get_helm_index<T>(repo_url: T) -> Result<ChartSourceMetadata, Error>
 where
-    T: AsRef<str> + std::fmt::Debug,
+    T: AsRef<str> + std::fmt::Display + std::fmt::Debug,
 {
     debug!("Get Helm repo index file");
 
diff --git a/rust/stackable-cockpit/src/platform/demo/spec.rs b/rust/stackable-cockpit/src/platform/demo/spec.rs
@@ -134,6 +134,11 @@ impl DemoSpec {
         Ok(())
     }
 
+    #[instrument(skip_all, fields(
+        stack_name = %self.stack,
+        operator_namespace = %install_parameters.operator_namespace,
+        product_namespace = %install_parameters.product_namespace,
+    ))]
     pub async fn install(
         &self,
         stack_list: StackList,
@@ -177,7 +182,11 @@ impl DemoSpec {
             .await
     }
 
-    #[instrument(skip_all)]
+    #[instrument(skip_all, fields(
+        stack_name = %self.stack,
+        operator_namespace = %install_params.operator_namespace,
+        product_namespace = %install_params.product_namespace,
+    ))]
     async fn prepare_manifests(
         &self,
         install_params: DemoInstallParameters,
diff --git a/rust/stackable-cockpit/src/platform/manifests.rs b/rust/stackable-cockpit/src/platform/manifests.rs
@@ -62,7 +62,7 @@ pub enum Error {
 
 pub trait InstallManifestsExt {
     // TODO (Techassi): This step shouldn't care about templating the manifests nor fetching them from remote
-    #[instrument(skip_all)]
+    #[instrument(skip_all, fields(%product_namespace))]
     #[allow(async_fn_in_trait)]
     async fn install_manifests(
         manifests: &[ManifestSpec],
@@ -72,12 +72,12 @@ pub trait InstallManifestsExt {
         client: &Client,
         transfer_client: &xfer::Client,
     ) -> Result<(), Error> {
-        debug!("Installing demo / stack manifests");
+        debug!("Installing manifests");
 
         for manifest in manifests {
             match manifest {
                 ManifestSpec::HelmChart(helm_file) => {
-                    debug!("Installing manifest from Helm chart {}", helm_file);
+                    debug!(helm_file, "Installing manifest from Helm chart");
 
                     // Read Helm chart YAML and apply templating
                     let helm_file = helm_file.into_path_or_url().context(ParsePathOrUrlSnafu {
@@ -89,10 +89,7 @@ pub trait InstallManifestsExt {
                         .await
                         .context(FileTransferSnafu)?;
 
-                    info!(
-                        "Installing Helm chart {} ({})",
-                        helm_chart.name, helm_chart.version
-                    );
+                    info!(helm_chart.name, helm_chart.version, "Installing Helm chart",);
 
                     // Assumption: that all manifest helm charts refer to repos not registries
                     helm::add_repo(&helm_chart.repo.name, &helm_chart.repo.url).context(
@@ -122,7 +119,7 @@ pub trait InstallManifestsExt {
                     })?;
                 }
                 ManifestSpec::PlainYaml(manifest_file) => {
-                    debug!("Installing YAML manifest from {}", manifest_file);
+                    debug!(manifest_file, "Installing YAML manifest");
 
                     // Read YAML manifest and apply templating
                     let path_or_url =
diff --git a/rust/stackable-cockpit/src/platform/operator/mod.rs b/rust/stackable-cockpit/src/platform/operator/mod.rs
@@ -178,7 +178,14 @@ impl OperatorSpec {
     }
 
     /// Installs the operator using Helm.
-    #[instrument(skip_all)]
+    #[instrument(skip_all, fields(
+        %namespace,
+        name = %self.name,
+        // NOTE (@NickLarsenNZ): Option doesn't impl Display, so we need to call
+        // display for the inner type if it exists. Otherwise we gte the Debug
+        // impl for the whole Option.
+        version = self.version.as_ref().map(|v| tracing::field::display(v)),
+    ))]
     pub fn install(
         &self,
         namespace: &str,
@@ -213,10 +220,10 @@ impl OperatorSpec {
     }
 
     /// Uninstalls the operator using Helm.
-    #[instrument]
+    #[instrument(skip_all, fields(%namespace))]
     pub fn uninstall<T>(&self, namespace: T) -> Result<(), helm::Error>
     where
-        T: AsRef<str> + std::fmt::Debug,
+        T: AsRef<str> + std::fmt::Display + std::fmt::Debug,
     {
         match helm::uninstall_release(&self.helm_name(), namespace.as_ref(), true) {
             Ok(status) => {
diff --git a/rust/stackable-cockpit/src/platform/release/spec.rs b/rust/stackable-cockpit/src/platform/release/spec.rs
@@ -3,7 +3,7 @@ use indexmap::IndexMap;
 use serde::{Deserialize, Serialize};
 use snafu::{ResultExt, Snafu};
 use tokio::task::JoinError;
-use tracing::{info, instrument};
+use tracing::{info, instrument, Instrument, Span};
 
 #[cfg(feature = "openapi")]
 use utoipa::ToSchema;
@@ -50,7 +50,7 @@ pub struct ReleaseSpec {
 
 impl ReleaseSpec {
     /// Installs a release by installing individual operators.
-    #[instrument(skip_all)]
+    #[instrument(skip_all, fields(%namespace, product.included = tracing::field::Empty, product.excluded = tracing::field::Empty))]
     pub async fn install(
         &self,
         include_products: &[String],
@@ -60,6 +60,13 @@ impl ReleaseSpec {
     ) -> Result<()> {
         info!("Installing release");
 
+        include_products.iter().for_each(|product| {
+            Span::current().record("product.included", product);
+        });
+        exclude_products.iter().for_each(|product| {
+            Span::current().record("product.excluded", product);
+        });
+
         let namespace = namespace.to_string();
         futures::stream::iter(self.filter_products(include_products, exclude_products))
             .map(|(product_name, product)| {
diff --git a/rust/stackable-cockpit/src/platform/stack/spec.rs b/rust/stackable-cockpit/src/platform/stack/spec.rs
@@ -151,7 +151,13 @@ impl StackSpec {
     }
 
     // TODO (Techassi): Can we get rid of the release list and just use the release spec instead
-    #[instrument(skip(self, release_list, client, transfer_client))]
+    #[instrument(skip_all, fields(
+        stack_name = %install_parameters.stack_name,
+        // NOTE (@NickLarsenNZ): Option doesn't impl Display, so we need to call
+        // display for the inner type if it exists. Otherwise we gte the Debug
+        // impl for the whole Option.
+        demo_name = install_parameters.demo_name.as_ref().map(|v| tracing::field::display(v)),
+    ))]
     pub async fn install(
         &self,
         release_list: release::ReleaseList,
@@ -192,12 +198,12 @@ impl StackSpec {
             .await
     }
 
-    #[instrument(skip(self, release_list))]
+    #[instrument(skip_all, fields(release = %self.release, %operator_namespace))]
     pub async fn install_release(
         &self,
         release_list: release::ReleaseList,
         operator_namespace: &str,
-        product_namespace: &str,
+        _product_namespace: &str, // TODO (@NickLarsenNZ): remove this field
         chart_source: &ChartSourceType,
     ) -> Result<(), Error> {
         info!("Trying to install release {}", self.release);
diff --git a/rust/stackable-cockpit/src/utils/check.rs b/rust/stackable-cockpit/src/utils/check.rs
@@ -9,7 +9,7 @@ pub fn binary_present<T: AsRef<OsStr>>(name: T) -> bool {
 }
 
 /// Returns if ALL binaries in the list are present in the $PATH.
-#[instrument]
+#[instrument(skip_all)]
 pub fn binaries_present(names: &[&str]) -> bool {
     debug!("Checking if required binaries are present on the system");
 
@@ -24,7 +24,7 @@ pub fn binaries_present(names: &[&str]) -> bool {
 
 /// Returns [`None`] if all binaries in the list are present in the $PATH and
 /// if not, returns [`Some`] containing the name of the missing binary.
-#[instrument]
+#[instrument(skip_all)]
 pub fn binaries_present_with_name(names: &[&str]) -> Option<String> {
     debug!("Checking if required binaries are present on the system");
 
diff --git a/rust/stackablectl/src/cli/mod.rs b/rust/stackablectl/src/cli/mod.rs
@@ -135,9 +135,8 @@ impl Cli {
 
     /// Adds the default (or custom) Helm repository URLs. Internally this calls the Helm SDK written in Go through the
     /// `go-helm-wrapper`.
-    #[instrument]
     pub fn add_helm_repos(&self) -> Result<(), helm::Error> {
-        debug!("Add Helm repos");
+        tracing::info!("Add Helm repos");
 
         // Stable repository
         helm::add_repo(HELM_REPO_NAME_STABLE, &self.repos.helm_repo_stable)?;
@@ -151,7 +150,6 @@ impl Cli {
         Ok(())
     }
 
-    #[instrument]
     pub fn cache_settings(&self) -> Result<Settings, CacheSettingsError> {
         if self.no_cache {
             Ok(Settings::disabled())
@@ -167,7 +165,7 @@ impl Cli {
         }
     }
 
-    #[instrument]
+    #[instrument(skip_all)]
     pub async fn run(&self) -> Result<String, Error> {
         // FIXME (Techassi): There might be a better way to handle this with
         // the match later in this function.
diff --git a/rust/stackablectl/src/cmds/demo.rs b/rust/stackablectl/src/cmds/demo.rs
@@ -160,7 +160,7 @@ pub enum CmdError {
 }
 
 impl DemoArgs {
-    #[instrument]
+    #[instrument(skip_all, fields(with_cache = !cli.no_cache))]
     pub async fn run(&self, cli: &Cli, cache: Cache) -> Result<String, CmdError> {
         debug!("Handle demo args");
 
@@ -211,7 +211,7 @@ impl DemoArgs {
 }
 
 /// Print out a list of demos, either as a table (plain), JSON or YAML
-#[instrument]
+#[instrument(skip_all)]
 async fn list_cmd(args: &DemoListArgs, cli: &Cli, list: demo::List) -> Result<String, CmdError> {
     info!("Listing demos");
 
@@ -259,7 +259,7 @@ async fn list_cmd(args: &DemoListArgs, cli: &Cli, list: demo::List) -> Result<St
 }
 
 /// Describe a specific demo by printing out a table (plain), JSON or YAML
-#[instrument]
+#[instrument(skip_all)]
 async fn describe_cmd(
     args: &DemoDescribeArgs,
     cli: &Cli,
@@ -315,7 +315,11 @@ async fn describe_cmd(
 }
 
 /// Install a specific demo
-#[instrument(skip(list, transfer_client))]
+#[instrument(skip_all, fields(
+    demo_name = %args.demo_name,
+    skip_release = args.skip_release,
+    %release_branch
+))]
 async fn install_cmd(
     args: &DemoInstallArgs,
     cli: &Cli,
diff --git a/rust/stackablectl/src/cmds/operator.rs b/rust/stackablectl/src/cmds/operator.rs
diff --git a/rust/stackablectl/src/cmds/stack.rs b/rust/stackablectl/src/cmds/stack.rs
diff --git a/rust/stackablectl/src/cmds/stacklet.rs b/rust/stackablectl/src/cmds/stacklet.rs

Original file line number	Diff line number	Diff line change
`@@ -19,7 +19,7 @@ pub enum Error {`
`19`	`19`	`}`
`20`	`20`
`21`	`21`	`/// Checks if Docker is running on the system`
`22`		`-#[instrument]`
	`22`	`+#[instrument(skip_all)]`
`23`	`23`	`pub async fn check_if_docker_is_running() -> Result<()> {`
`24`	`24`	`debug!("Checking if Docker is running");`
`25`	`25`
Original file line number	Diff line number	Diff line change
`@@ -64,7 +64,7 @@ impl Cluster {`
`64`	`64`	`}`
`65`	`65`
`66`	`66`	`/// Create a new local cluster by calling the kind binary.`
`67`		`- #[instrument]`
	`67`	`+ #[instrument(skip_all)]`
`68`	`68`	`pub async fn create(&self) -> Result<()> {`
`69`	`69`	`info!("Creating local cluster using kind");`
`70`	`70`
`@@ -109,7 +109,7 @@ impl Cluster {`
`109`	`109`	`}`
`110`	`110`
`111`	`111`	`/// Creates a kind cluster if it doesn't exist already.`
`112`		`- #[instrument]`
	`112`	`+ #[instrument(skip_all)]`
`113`	`113`	`pub async fn create_if_not_exists(&self) -> Result<()> {`
`114`	`114`	`info!("Creating cluster if it doesn't exist using kind");`
`115`	`115`
`@@ -131,7 +131,7 @@ impl Cluster {`
`131`	`131`	`}`
`132`	`132`
`133`	`133`	`/// Check if a kind cluster with the provided name already exists.`
`134`		`- #[instrument]`
	`134`	`+ #[instrument(skip_all)]`
`135`	`135`	`async fn check_if_cluster_exists(cluster_name: &str) -> Result<bool> {`
`136`	`136`	`debug!("Checking if kind cluster exists");`
`137`	`137`
Original file line number	Diff line number	Diff line change
`@@ -41,7 +41,7 @@ impl Cluster {`
`41`	`41`	`}`
`42`	`42`
`43`	`43`	`/// Create a new local cluster by calling the Minikube binary`
`44`		`- #[instrument]`
	`44`	`+ #[instrument(skip_all)]`
`45`	`45`	`pub async fn create(&self) -> Result<(), Error> {`
`46`	`46`	`info!("Creating local cluster using Minikube");`
`47`	`47`
`@@ -70,7 +70,7 @@ impl Cluster {`
`70`	`70`	`}`
`71`	`71`
`72`	`72`	`/// Creates a Minikube cluster if it doesn't exist already.`
`73`		`- #[instrument]`
	`73`	`+ #[instrument(skip_all)]`
`74`	`74`	`pub async fn create_if_not_exists(&self) -> Result<(), Error> {`
`75`	`75`	`info!("Creating cluster if it doesn't exist using Minikube");`
`76`	`76`
`@@ -92,7 +92,7 @@ impl Cluster {`
`92`	`92`	`}`
`93`	`93`
`94`	`94`	`/// Check if a kind cluster with the provided name already exists.`
`95`		`- #[instrument]`
	`95`	`+ #[instrument(skip_all)]`
`96`	`96`	`async fn check_if_cluster_exists(cluster_name: &str) -> Result<bool, Error> {`
`97`	`97`	`debug!("Checking if Minikube cluster exists");`
`98`	`98`