Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit ae4438e

Browse files
committedMar 17, 2025
refactor!: Adjust CLI surface
Due to initialize_logging being removed and being replaced by stackable_telemetry, the CLI interface needs to be adjusted as well. The old --tracing-target argument is removed in favour of more granular arguments to enable different outputs and exporters.
1 parent 2fec648 commit ae4438e

File tree

2 files changed

+77
-45
lines changed

2 files changed

+77
-45
lines changed
 

‎crates/stackable-operator/CHANGELOG.md

+18-1
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,23 @@ All notable changes to this project will be documented in this file.
44

55
## [Unreleased]
66

7+
### Added
8+
9+
- Add more granular telemetry related arguments to `ProductOperatorRun` ([#977]).
10+
- `--enable-console-output`: Enables output of `tracing` events to the console (stdout)
11+
- `--enable-file-output`: Enables output `tracing` events to a rotating log file
12+
- `--enable-otlp-traces`: Enables exporting of traces via OTLP
13+
- `--enable-otlp-logs`: Enables exporting of logs via OTLP
14+
15+
### Removed
16+
17+
- BREAKING: Remove `--tracing-target` argument and field from `ProductOperatorRun`.
18+
Use the new, more granular arguments instead ([#977]).
19+
- BREAKING: Remove `initialize_logging` helper function from `stackable_operator::logging` ([#977]).
20+
- Remove `opentelemetry-jaeger` dependency ([#977]).
21+
22+
[#977]: https://github.com/stackabletech/operator-rs/pull/977
23+
724
## [0.87.3] - 2025-03-14
825

926
### Added
@@ -48,7 +65,7 @@ All notable changes to this project will be documented in this file.
4865

4966
### Added
5067

51-
- BREAKING: Add `region` field to S3ConnectionSpec (defaults to `us-east-1`) ([#959]).
68+
- Add `region` field to S3ConnectionSpec ([#959]).
5269

5370
[#959]: https://github.com/stackabletech/operator-rs/pull/959
5471

‎crates/stackable-operator/src/cli.rs

+59-44
Original file line numberDiff line numberDiff line change
@@ -115,10 +115,7 @@ use clap::Args;
115115
use product_config::ProductConfigManager;
116116
use snafu::{ResultExt, Snafu};
117117

118-
use crate::{
119-
logging::TracingTarget, namespace::WatchNamespace,
120-
utils::cluster_info::KubernetesClusterInfoOpts,
121-
};
118+
use crate::{namespace::WatchNamespace, utils::cluster_info::KubernetesClusterInfoOpts};
122119

123120
pub const AUTHOR: &str = "Stackable GmbH - info@stackable.tech";
124121

@@ -217,9 +214,8 @@ pub struct ProductOperatorRun {
217214
#[arg(long, env, default_value = "")]
218215
pub watch_namespace: WatchNamespace,
219216

220-
/// Tracing log collector system
221-
#[arg(long, env, default_value_t, value_enum)]
222-
pub tracing_target: TracingTarget,
217+
#[command(flatten)]
218+
pub telemetry_arguments: TelemetryArguments,
223219

224220
#[command(flatten)]
225221
pub cluster_info_opts: KubernetesClusterInfoOpts,
@@ -245,41 +241,58 @@ impl ProductConfigPath {
245241
/// Load the [`ProductConfigManager`] from the given path, falling back to the first
246242
/// path that exists from `default_search_paths` if none is given by the user.
247243
pub fn load(&self, default_search_paths: &[impl AsRef<Path>]) -> Result<ProductConfigManager> {
248-
ProductConfigManager::from_yaml_file(resolve_path(
249-
self.path.as_deref(),
250-
default_search_paths,
251-
)?)
252-
.context(ProductConfigLoadSnafu)
244+
let resolved_path = Self::resolve_path(self.path.as_deref(), default_search_paths)?;
245+
ProductConfigManager::from_yaml_file(resolved_path).context(ProductConfigLoadSnafu)
253246
}
254-
}
255247

256-
/// Check if the path can be found anywhere:
257-
/// 1) User provides path `user_provided_path` to file -> 'Error' if not existing.
258-
/// 2) User does not provide path to file -> search in `default_paths` and
259-
/// take the first existing file.
260-
/// 3) `Error` if nothing was found.
261-
fn resolve_path<'a>(
262-
user_provided_path: Option<&'a Path>,
263-
default_paths: &'a [impl AsRef<Path> + 'a],
264-
) -> Result<&'a Path> {
265-
// Use override if specified by the user, otherwise search through defaults given
266-
let search_paths = if let Some(path) = user_provided_path {
267-
vec![path]
268-
} else {
269-
default_paths.iter().map(|path| path.as_ref()).collect()
270-
};
271-
for path in &search_paths {
272-
if path.exists() {
273-
return Ok(path);
248+
/// Check if the path can be found anywhere
249+
///
250+
/// 1. User provides path `user_provided_path` to file. Return [`Error`] if not existing.
251+
/// 2. User does not provide path to file -> search in `default_paths` and
252+
/// take the first existing file.
253+
/// 3. Return [`Error`] if nothing was found.
254+
fn resolve_path<'a>(
255+
user_provided_path: Option<&'a Path>,
256+
default_paths: &'a [impl AsRef<Path> + 'a],
257+
) -> Result<&'a Path> {
258+
// Use override if specified by the user, otherwise search through defaults given
259+
let search_paths = if let Some(path) = user_provided_path {
260+
vec![path]
261+
} else {
262+
default_paths.iter().map(|path| path.as_ref()).collect()
263+
};
264+
for path in &search_paths {
265+
if path.exists() {
266+
return Ok(path);
267+
}
274268
}
269+
RequiredFileMissingSnafu {
270+
search_path: search_paths
271+
.into_iter()
272+
.map(PathBuf::from)
273+
.collect::<Vec<_>>(),
274+
}
275+
.fail()
275276
}
276-
RequiredFileMissingSnafu {
277-
search_path: search_paths
278-
.into_iter()
279-
.map(PathBuf::from)
280-
.collect::<Vec<_>>(),
281-
}
282-
.fail()
277+
}
278+
279+
#[derive(Debug, Default, PartialEq, Eq, Args)]
280+
pub struct TelemetryArguments {
281+
/// Disable console output.
282+
#[arg(long, env)]
283+
no_console_output: bool,
284+
285+
/// Enable logging to rolling files located in the specified directory
286+
#[arg(long, env, value_name = "DIRECTORY")]
287+
rolling_logs: Option<PathBuf>,
288+
289+
/// Enable exporting traces via OTLP.
290+
#[arg(long, env)]
291+
otlp_traces: bool,
292+
293+
/// Enable exporting logs via OTLP.
294+
#[arg(long, env)]
295+
otlp_logs: bool,
283296
}
284297

285298
#[cfg(test)]
@@ -300,6 +313,7 @@ mod tests {
300313
#[test]
301314
fn verify_cli() {
302315
use clap::CommandFactory;
316+
ProductOperatorRun::command().print_long_help().unwrap();
303317
ProductOperatorRun::command().debug_assert()
304318
}
305319

@@ -342,7 +356,7 @@ mod tests {
342356

343357
let file = File::create(full_path_to_create).expect("create temporary file");
344358

345-
let found_path = resolve_path(
359+
let found_path = ProductConfigPath::resolve_path(
346360
full_user_provided_path.as_deref(),
347361
&full_default_locations_ref,
348362
)?;
@@ -358,13 +372,14 @@ mod tests {
358372
#[test]
359373
#[should_panic]
360374
fn resolve_path_user_path_not_existing() {
361-
resolve_path(Some(USER_PROVIDED_PATH.as_ref()), &[DEPLOY_FILE_PATH]).unwrap();
375+
ProductConfigPath::resolve_path(Some(USER_PROVIDED_PATH.as_ref()), &[DEPLOY_FILE_PATH])
376+
.unwrap();
362377
}
363378

364379
#[test]
365380
fn resolve_path_nothing_found_errors() {
366381
if let Err(Error::RequiredFileMissing { search_path }) =
367-
resolve_path(None, &[DEPLOY_FILE_PATH, DEFAULT_FILE_PATH])
382+
ProductConfigPath::resolve_path(None, &[DEPLOY_FILE_PATH, DEFAULT_FILE_PATH])
368383
{
369384
assert_eq!(
370385
search_path,
@@ -396,8 +411,8 @@ mod tests {
396411
ProductOperatorRun {
397412
product_config: ProductConfigPath::from("bar".as_ref()),
398413
watch_namespace: WatchNamespace::One("foo".to_string()),
399-
tracing_target: TracingTarget::None,
400414
cluster_info_opts: Default::default(),
415+
telemetry_arguments: Default::default(),
401416
}
402417
);
403418

@@ -408,8 +423,8 @@ mod tests {
408423
ProductOperatorRun {
409424
product_config: ProductConfigPath::from("bar".as_ref()),
410425
watch_namespace: WatchNamespace::All,
411-
tracing_target: TracingTarget::None,
412426
cluster_info_opts: Default::default(),
427+
telemetry_arguments: Default::default(),
413428
}
414429
);
415430

@@ -421,8 +436,8 @@ mod tests {
421436
ProductOperatorRun {
422437
product_config: ProductConfigPath::from("bar".as_ref()),
423438
watch_namespace: WatchNamespace::One("foo".to_string()),
424-
tracing_target: TracingTarget::None,
425439
cluster_info_opts: Default::default(),
440+
telemetry_arguments: Default::default(),
426441
}
427442
);
428443
}

0 commit comments

Comments
 (0)
Please sign in to comment.