diff --git a/rust/helm-sys/src/lib.rs b/rust/helm-sys/src/lib.rs index 1e934586..9c7d66d4 100644 --- a/rust/helm-sys/src/lib.rs +++ b/rust/helm-sys/src/lib.rs @@ -56,6 +56,8 @@ pub fn uninstall_helm_release( } } +// TODO (@NickLarsenNZ): Add tracing to helm-sys, maybe? +// #[instrument] pub fn check_helm_release_exists(release_name: &str, namespace: &str) -> bool { let release_name = CString::new(release_name).unwrap(); let namespace = CString::new(namespace).unwrap(); diff --git a/rust/stackable-cockpit/src/engine/docker/mod.rs b/rust/stackable-cockpit/src/engine/docker/mod.rs index 1a7e515e..697b0797 100644 --- 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 index 96a159c5..85b8843f 100644 --- 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 { 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 index a8a929f5..dab6e959 100644 --- 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 { debug!("Checking if Minikube cluster exists"); diff --git a/rust/stackable-cockpit/src/helm.rs b/rust/stackable-cockpit/src/helm.rs index 25bca566..fd263aad 100644 --- 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: `-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: `-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(repo_url: T) -> Result where - T: AsRef + std::fmt::Debug, + T: AsRef + std::fmt::Display + std::fmt::Debug, { debug!("Get Helm repo index file"); diff --git a/rust/stackable-cockpit/src/oci.rs b/rust/stackable-cockpit/src/oci.rs index 0c8eea63..337cc876 100644 --- a/rust/stackable-cockpit/src/oci.rs +++ b/rust/stackable-cockpit/src/oci.rs @@ -2,7 +2,7 @@ use std::collections::HashMap; use serde::Deserialize; use snafu::{OptionExt, ResultExt, Snafu}; -use tracing::debug; +use tracing::{debug, instrument}; use url::Url; use urlencoding::encode; @@ -117,6 +117,8 @@ impl OciUrlExt for Url { } } +// TODO (@NickLarsenNZ): Look into why a HashMap is used here when the key is inside each entry in the value +#[instrument] pub async fn get_oci_index<'a>() -> Result, Error> { let mut source_index_files: HashMap<&str, ChartSourceMetadata> = HashMap::new(); @@ -133,12 +135,12 @@ pub async fn get_oci_index<'a>() -> Result }, ); } - let base_url = format!("https://{}/api/v2.0", HELM_OCI_BASE); + let base_url = format!("https://{HELM_OCI_BASE}/api/v2.0"); // fetch all operators let url = format!( - "{}/repositories?page_size={}&q=name=~sdp-charts/", - base_url, 100 + "{base_url}/repositories?page_size={page_size}&q=name=~sdp-charts/", + page_size = 100 ); // reuse connections @@ -153,16 +155,20 @@ pub async fn get_oci_index<'a>() -> Result .await .context(ParseRepositoriesSnafu)?; - debug!("OCI repos {:?}", repositories); + debug!( + count = repositories.len(), + "Received response for OCI repositories" + ); for repository in &repositories { // fetch all artifacts pro operator + // NOTE (@NickLarsenNZ): I think repository_name should be helm_chart_name. let (project_name, repository_name) = repository .name .split_once('/') .context(UnexpectedOciRepositoryNameSnafu)?; - debug!("OCI repo parts {} and {}", project_name, repository_name); + tracing::trace!(project_name, repository_name, "OCI repository parts"); let mut artifacts = Vec::new(); let mut page = 1; @@ -196,17 +202,7 @@ pub async fn get_oci_index<'a>() -> Result .replace("-arm64", "") .replace("-amd64", ""); - debug!( - "OCI resolved artifact {}, {}, {}", - release_version.to_string(), - repository_name.to_string(), - release_artifact.name.to_string() - ); - - debug!( - "Repo/Artifact/Tag: {:?} / {:?} / {:?}", - repository, artifact, release_artifact - ); + tracing::trace!(repository_name, release_version, "OCI resolved artifact"); let entry = ChartSourceEntry { name: repository_name.to_string(), diff --git a/rust/stackable-cockpit/src/platform/demo/spec.rs b/rust/stackable-cockpit/src/platform/demo/spec.rs index 59aaa8d9..af464125 100644 --- 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 index 86b604f4..b92814c1 100644 --- 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 index eedcf9f9..5adbb768 100644 --- a/rust/stackable-cockpit/src/platform/operator/mod.rs +++ b/rust/stackable-cockpit/src/platform/operator/mod.rs @@ -178,13 +178,20 @@ 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(tracing::field::display), + ))] pub fn install( &self, namespace: &str, chart_source: &ChartSourceType, ) -> Result<(), helm::Error> { - info!("Installing operator {}", self); + info!(operator = %self, "Installing operator"); let version = self.version.as_ref().map(|v| v.to_string()); let helm_name = self.helm_name(); @@ -213,10 +220,10 @@ impl OperatorSpec { } /// Uninstalls the operator using Helm. - #[instrument] + #[instrument(skip_all, fields(%namespace))] pub fn uninstall(&self, namespace: T) -> Result<(), helm::Error> where - T: AsRef + std::fmt::Debug, + T: AsRef + 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 index cf4bb8cd..eb815e55 100644 --- 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,11 @@ 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,29 +64,44 @@ 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)| { + let task_span = + tracing::debug_span!("install_operator", product_name = tracing::field::Empty); + let namespace = namespace.clone(); let chart_source = chart_source.clone(); // Helm installs currently `block_in_place`, so we need to spawn each job onto a separate task to // get useful parallelism. - tokio::spawn(async move { - info!("Installing {product_name}-operator"); - - // Create operator spec - let operator = OperatorSpec::new(&product_name, Some(product.version.clone())) - .context(OperatorSpecParseSnafu)?; - - // Install operator - operator - .install(&namespace, &chart_source) - .context(HelmInstallSnafu)?; - - info!("Installed {product_name}-operator"); - - Ok(()) - }) + tokio::spawn( + async move { + Span::current().record("product_name", &product_name); + info!("Installing {product_name}-operator"); + + // Create operator spec + let operator = + OperatorSpec::new(&product_name, Some(product.version.clone())) + .context(OperatorSpecParseSnafu)?; + + // Install operator + operator + .install(&namespace, &chart_source) + .context(HelmInstallSnafu)?; + + info!("Installed {product_name}-operator"); + + Ok(()) + } + .instrument(task_span), + ) }) .buffer_unordered(10) .map(|res| res.context(BackgroundTaskSnafu)?) diff --git a/rust/stackable-cockpit/src/platform/stack/spec.rs b/rust/stackable-cockpit/src/platform/stack/spec.rs index 2a20ba5d..3cd839fc 100644 --- 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(tracing::field::display), + ))] pub async fn install( &self, release_list: release::ReleaseList, @@ -192,15 +198,15 @@ 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); + info!(self.release, "Trying to install release"); // Get the release by name let release = release_list diff --git a/rust/stackable-cockpit/src/utils/chartsource.rs b/rust/stackable-cockpit/src/utils/chartsource.rs index 5a829e04..d84c1a13 100644 --- a/rust/stackable-cockpit/src/utils/chartsource.rs +++ b/rust/stackable-cockpit/src/utils/chartsource.rs @@ -4,6 +4,9 @@ use serde::Deserialize; #[derive(Clone, Debug, Deserialize)] pub struct ChartSourceMetadata { + // TODO (@NickLarsenNZ): Add a field for the repo name. + // This would remove the need for the nested HashMaps + // See: around rust/stackablectl/src/cmds/operator.rs:519 pub entries: HashMap>, } diff --git a/rust/stackable-cockpit/src/utils/check.rs b/rust/stackable-cockpit/src/utils/check.rs index 8abf551b..bb67806c 100644 --- a/rust/stackable-cockpit/src/utils/check.rs +++ b/rust/stackable-cockpit/src/utils/check.rs @@ -9,7 +9,7 @@ pub fn binary_present>(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 { debug!("Checking if required binaries are present on the system"); diff --git a/rust/stackablectl/CHANGELOG.md b/rust/stackablectl/CHANGELOG.md index 52247895..c5743084 100644 --- a/rust/stackablectl/CHANGELOG.md +++ b/rust/stackablectl/CHANGELOG.md @@ -8,9 +8,11 @@ All notable changes to this project will be documented in this file. - Use `rustls-native-certs` so that `stackablectl` can be used in environments with internal PKI ([#351]). - Use `heritage` label when looking up the `minio-console` stacklet ([#364]). +- Improve tracing and log output ([#365]). [#351]: https://github.com/stackabletech/stackable-cockpit/pull/351 [#364]: https://github.com/stackabletech/stackable-cockpit/pull/364 +[#365]: https://github.com/stackabletech/stackable-cockpit/pull/365 ## [24.11.3] - 2025-01-27 diff --git a/rust/stackablectl/src/cli/mod.rs b/rust/stackablectl/src/cli/mod.rs index 6ac718b1..7bef7840 100644 --- a/rust/stackablectl/src/cli/mod.rs +++ b/rust/stackablectl/src/cli/mod.rs @@ -3,7 +3,7 @@ use std::env; use clap::{Parser, Subcommand, ValueEnum}; use directories::ProjectDirs; use snafu::{ResultExt, Snafu}; -use tracing::{debug, instrument, Level}; +use tracing::{instrument, Level}; use stackable_cockpit::{ constants::{HELM_REPO_NAME_DEV, HELM_REPO_NAME_STABLE, HELM_REPO_NAME_TEST}, @@ -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,9 +150,9 @@ impl Cli { Ok(()) } - #[instrument] pub fn cache_settings(&self) -> Result { if self.no_cache { + tracing::debug!("Cache disabled"); Ok(Settings::disabled()) } else { let project_dir = ProjectDirs::from( @@ -163,11 +162,16 @@ impl Cli { ) .ok_or(CacheSettingsError::UserDir)?; - Ok(Settings::disk(project_dir.cache_dir())) + let cache_dir = project_dir.cache_dir(); + tracing::debug!( + cache_dir = %cache_dir.to_string_lossy(), + "Setting cache directory" + ); + Ok(Settings::disk(cache_dir)) } } - #[instrument] + #[instrument(skip_all)] pub async fn run(&self) -> Result { // 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 index 437977a6..b98ed723 100644 --- 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 { 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 { info!("Listing demos"); @@ -259,13 +259,13 @@ async fn list_cmd(args: &DemoListArgs, cli: &Cli, list: demo::List) -> Result Result { - info!("Describing demo {}", args.demo_name); + info!(demo_name = %args.demo_name, "Describing demo"); let demo = list.get(&args.demo_name).ok_or(CmdError::NoSuchDemo { name: args.demo_name.clone(), @@ -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, @@ -323,7 +327,7 @@ async fn install_cmd( transfer_client: &xfer::Client, release_branch: &str, ) -> Result { - info!(%release_branch, "Installing demo {}", args.demo_name); + info!(demo_name = %args.demo_name, "Installing demo"); // Init result output and progress output let mut output = cli.result(); diff --git a/rust/stackablectl/src/cmds/operator.rs b/rust/stackablectl/src/cmds/operator.rs index 3f10b869..4616b966 100644 --- a/rust/stackablectl/src/cmds/operator.rs +++ b/rust/stackablectl/src/cmds/operator.rs @@ -187,7 +187,7 @@ impl OperatorArgs { } } -#[instrument] +#[instrument(skip_all)] async fn list_cmd(args: &OperatorListArgs, cli: &Cli) -> Result { debug!("Listing operators"); @@ -244,9 +244,9 @@ async fn list_cmd(args: &OperatorListArgs, cli: &Cli) -> Result Result { - debug!("Describing operator {}", args.operator_name); + debug!(operator_name = %args.operator_name, "Describing operator"); // Build map which maps artifacts to a chart source let source_index_files = @@ -303,7 +303,7 @@ async fn describe_cmd(args: &OperatorDescribeArgs, cli: &Cli) -> Result Result { info!("Installing operator(s)"); @@ -328,6 +328,8 @@ async fn install_cmd(args: &OperatorInstallArgs, cli: &Cli) -> Result Result Result { info!("Uninstalling operator(s)"); @@ -381,9 +383,9 @@ fn uninstall_cmd(args: &OperatorUninstallArgs, cli: &Cli) -> Result Result { - debug!("Listing installed operators"); + info!("Listing installed operators"); type ReleaseList = IndexMap; @@ -465,7 +467,20 @@ async fn build_source_index_file_list<'a>( ChartSourceType::OCI => { source_index_files = oci::get_oci_index().await.context(OciSnafu)?; - debug!("OCI Repository entries: {:?}", source_index_files); + debug!(count = source_index_files.len(), "OCI Repository entries"); + + // TODO (@NickLarsenNZ): Look into why this is so deeply nested with duplicate data. + // source_index_files + // .iter() + // .for_each(|(&repo_name, chart_source_metadata)| { + // let x = chart_source_metadata.entries.len(); + // tracing::trace!(repo_name, x, "thing"); + // let _ = &chart_source_metadata + // .entries + // .iter() + // // y (below) is a Vec + // .for_each(|(x, y)| tracing::error!(x, "blah {:?}", y)); + // }); } ChartSourceType::Repo => { for helm_repo_name in [ @@ -493,7 +508,7 @@ async fn build_source_index_file_list<'a>( /// Iterates over all valid operators and creates a list of versions grouped /// by stable, test and dev lines based on the list of Helm repo index files. -#[instrument] +#[instrument(skip_all)] fn build_versions_list( helm_index_files: &HashMap<&str, ChartSourceMetadata>, ) -> Result, CmdError> { @@ -503,7 +518,13 @@ fn build_versions_list( for operator in operator::VALID_OPERATORS { for (helm_repo_name, helm_repo_index_file) in helm_index_files { - let versions = list_operator_versions_from_repo(operator, helm_repo_index_file)?; + let span = tracing::info_span!( + "build_versions_list_iter", + helm.repository.name = %helm_repo_name, + operator_name = %operator, + ); + let versions = + span.in_scope(|| list_operator_versions_from_repo(operator, helm_repo_index_file))?; let entry = versions_list.entry(operator.to_string()); let entry = entry.or_insert(OperatorVersionList(HashMap::new())); entry.0.insert(helm_repo_name.to_string(), versions); @@ -515,18 +536,15 @@ fn build_versions_list( /// Builds a list of versions for one operator (by name) which is grouped by /// stable, test and dev lines based on the list of Helm repo index files. -#[instrument] +#[instrument(skip_all, fields(%operator_name))] fn build_versions_list_for_operator( operator_name: T, helm_index_files: &HashMap<&str, ChartSourceMetadata>, ) -> Result where - T: AsRef + std::fmt::Debug, + T: AsRef + std::fmt::Display + std::fmt::Debug, { - debug!( - "Build versions list for operator {}", - operator_name.as_ref() - ); + debug!("Build versions list for operator"); let mut versions_list = OperatorVersionList(HashMap::new()); let operator_name = operator_name.as_ref(); @@ -540,23 +558,24 @@ where } /// Builds a list of operator versions based on the provided Helm repo. -#[instrument] +#[instrument(skip_all, fields(%operator_name))] fn list_operator_versions_from_repo( operator_name: T, helm_repo: &ChartSourceMetadata, ) -> Result, CmdError> where - T: AsRef + std::fmt::Debug, + T: AsRef + std::fmt::Display + std::fmt::Debug, { - debug!("Listing operator versions from repo"); + debug!("Listing operator versions from repository"); - let operator_name = utils::operator_chart_name(operator_name.as_ref()); + let chart_name = utils::operator_chart_name(operator_name.as_ref()); - match helm_repo.entries.get(&operator_name) { + match helm_repo.entries.get(&chart_name) { Some(entries) => { let mut versions = entries .iter() .map(|entry| { + tracing::trace!(helm.chart.name = %chart_name, helm.chart.version = %entry.version, "Found operator chart version"); Version::parse(&entry.version).with_context(|_| InvalidHelmChartVersionSnafu { version: entry.version.clone(), }) diff --git a/rust/stackablectl/src/cmds/release.rs b/rust/stackablectl/src/cmds/release.rs index e0dca6fd..698dcdd8 100644 --- a/rust/stackablectl/src/cmds/release.rs +++ b/rust/stackablectl/src/cmds/release.rs @@ -211,7 +211,7 @@ async fn describe_cmd( cli: &Cli, release_list: release::ReleaseList, ) -> Result { - info!("Describing release"); + info!(release = %args.release, "Describing release"); let release = release_list.get(&args.release); @@ -272,6 +272,7 @@ async fn install_cmd( cli: &Cli, release_list: release::ReleaseList, ) -> Result { + info!(release = %args.release, "Installing release"); match release_list.get(&args.release) { Some(release) => { let mut output = cli.result(); diff --git a/rust/stackablectl/src/cmds/stack.rs b/rust/stackablectl/src/cmds/stack.rs index 7391f1f9..7f357c49 100644 --- a/rust/stackablectl/src/cmds/stack.rs +++ b/rust/stackablectl/src/cmds/stack.rs @@ -193,7 +193,7 @@ impl StackArgs { } } -#[instrument] +#[instrument(skip_all)] fn list_cmd( args: &StackListArgs, cli: &Cli, @@ -243,13 +243,13 @@ fn list_cmd( } } -#[instrument] +#[instrument(skip_all)] fn describe_cmd( args: &StackDescribeArgs, cli: &Cli, stack_list: stack::StackList, ) -> Result { - info!("Describing stack {}", args.stack_name); + info!(stack_name = %args.stack_name, "Describing stack"); match stack_list.get(&args.stack_name) { Some(stack) => match args.output_type { @@ -312,7 +312,7 @@ async fn install_cmd( stack_list: stack::StackList, transfer_client: &xfer::Client, ) -> Result { - info!("Installing stack {}", args.stack_name); + info!(stack_name = %args.stack_name, "Installing stack"); let files = cli.get_release_files().context(PathOrUrlParseSnafu)?; let release_list = release::ReleaseList::build(&files, transfer_client) diff --git a/rust/stackablectl/src/cmds/stacklet.rs b/rust/stackablectl/src/cmds/stacklet.rs index 55c3b6e8..420d704f 100644 --- a/rust/stackablectl/src/cmds/stacklet.rs +++ b/rust/stackablectl/src/cmds/stacklet.rs @@ -90,7 +90,7 @@ impl StackletArgs { } } -#[instrument] +#[instrument(skip_all)] async fn list_cmd(args: &StackletListArgs, cli: &Cli) -> Result { info!("Listing installed stacklets"); @@ -203,7 +203,7 @@ async fn list_cmd(args: &StackletListArgs, cli: &Cli) -> Result Result { info!("Displaying stacklet credentials");