From 48bdf87e53f93e8aa45ed58cab007777bd1f6fb4 Mon Sep 17 00:00:00 2001 From: houseme Date: Sat, 11 Jul 2026 03:44:07 +0800 Subject: [PATCH] refactor(obs): drop the dial9 S3 config left stranded by the upload revert (#4700) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #4663 added a full S3 upload path (config fields + env parsing + wiring), then reverted the wiring when its dependency turned out to carry RUSTSEC advisories (D9-14). The revert stopped at enabled.rs and left the config layer half torn down: `Dial9Config` still carried `s3_bucket`/`s3_prefix` with no consumer, plus a dead `s3_upload_requested()`, and `Dial9SessionGuard::config()` was likewise never called. All three are zero-consumer dead code. Since S3 upload cannot be built at all, the config type should not model it. Drop the two fields and both accessors. `from_env` still reads the S3 env vars and warns about them (an operator who set them deserves to know why they do nothing — that warning is deliberate and stays), but drops the values instead of storing configuration nothing reads. While here, collapse `from_env`'s two `record_config` calls into one by extracting `from_env_enabled`, so the enabled/disabled paths share a single publish point. No behavior change: the S3 warning fires identically, and every other field is untouched. Net -13 lines, and `Dial9Config` drops from 8 fields to 6. Co-authored-by: heihutu --- crates/obs/src/telemetry/dial9/config.rs | 64 ++++++++++------------- crates/obs/src/telemetry/dial9/enabled.rs | 5 -- 2 files changed, 28 insertions(+), 41 deletions(-) diff --git a/crates/obs/src/telemetry/dial9/config.rs b/crates/obs/src/telemetry/dial9/config.rs index 9f2057e3f..ee7889fc5 100644 --- a/crates/obs/src/telemetry/dial9/config.rs +++ b/crates/obs/src/telemetry/dial9/config.rs @@ -84,12 +84,6 @@ pub struct Dial9Config { /// Number of rotated files to keep pub rotation_count: usize, - - /// Optional S3 bucket for uploading sealed trace segments - pub s3_bucket: Option, - - /// Optional key prefix for uploaded segments - pub s3_prefix: Option, } impl Default for Dial9Config { @@ -100,27 +94,28 @@ impl Default for Dial9Config { file_prefix: DEFAULT_RUNTIME_DIAL9_FILE_PREFIX.to_string(), max_file_size: DEFAULT_RUNTIME_DIAL9_MAX_FILE_SIZE, rotation_count: DEFAULT_RUNTIME_DIAL9_ROTATION_COUNT, - s3_bucket: None, - s3_prefix: None, } } } impl Dial9Config { - /// Create configuration from environment variables. + /// Create configuration from environment variables, recording the resulting + /// trace location so metrics can report it even on a disabled binary. pub fn from_env() -> Self { - let enabled = is_enabled(); - - if !enabled { - let config = Self::default(); - dial9_runtime_state().record_config(&config); - return config; - } + let config = if is_enabled() { + Self::from_env_enabled() + } else { + Self::default() + }; + dial9_runtime_state().record_config(&config); + config + } + /// Build the enabled configuration from the environment, warning about any + /// knob that is set but cannot be honoured. + fn from_env_enabled() -> Self { let raw_max_file_size = get_env_opt_u64(ENV_RUNTIME_DIAL9_MAX_FILE_SIZE); let raw_rotation_count = get_env_opt_usize(ENV_RUNTIME_DIAL9_ROTATION_COUNT); - let max_file_size = sanitize_max_file_size(raw_max_file_size.unwrap_or(DEFAULT_RUNTIME_DIAL9_MAX_FILE_SIZE)); - let rotation_count = sanitize_rotation_count(raw_rotation_count.unwrap_or(DEFAULT_RUNTIME_DIAL9_ROTATION_COUNT)); if raw_max_file_size == Some(0) { warn!( @@ -143,22 +138,24 @@ impl Dial9Config { ); } - let s3_bucket = get_env_opt_str(ENV_RUNTIME_DIAL9_S3_BUCKET).filter(|s| !s.is_empty()); - let s3_prefix = get_env_opt_str(ENV_RUNTIME_DIAL9_S3_PREFIX).filter(|s| !s.is_empty()); - warn_unusable_s3_upload(s3_bucket.as_deref(), s3_prefix.as_deref()); + // S3 upload is not built (see `warn_unusable_s3_upload`); read the knobs + // only to warn, and drop them rather than carrying dead configuration. + warn_unusable_s3_upload( + get_env_opt_str(ENV_RUNTIME_DIAL9_S3_BUCKET) + .filter(|s| !s.is_empty()) + .as_deref(), + get_env_opt_str(ENV_RUNTIME_DIAL9_S3_PREFIX) + .filter(|s| !s.is_empty()) + .as_deref(), + ); - let config = Self { - enabled, + Self { + enabled: true, output_dir: get_env_str(ENV_RUNTIME_DIAL9_OUTPUT_DIR, DEFAULT_RUNTIME_DIAL9_OUTPUT_DIR), file_prefix: get_env_str(ENV_RUNTIME_DIAL9_FILE_PREFIX, DEFAULT_RUNTIME_DIAL9_FILE_PREFIX), - max_file_size, - rotation_count, - s3_bucket, - s3_prefix, - }; - - dial9_runtime_state().record_config(&config); - config + max_file_size: sanitize_max_file_size(raw_max_file_size.unwrap_or(DEFAULT_RUNTIME_DIAL9_MAX_FILE_SIZE)), + rotation_count: sanitize_rotation_count(raw_rotation_count.unwrap_or(DEFAULT_RUNTIME_DIAL9_ROTATION_COUNT)), + } } /// Get the base path for trace files. @@ -170,11 +167,6 @@ impl Dial9Config { pub fn total_disk_budget(&self) -> u64 { total_rotation_size(self.max_file_size, self.rotation_count) } - - /// Whether S3 upload of sealed segments is both requested and usable. - pub fn s3_upload_requested(&self) -> bool { - self.s3_bucket.is_some() - } } /// S3 upload of sealed segments is not available in any build. diff --git a/crates/obs/src/telemetry/dial9/enabled.rs b/crates/obs/src/telemetry/dial9/enabled.rs index 6d1cd40cb..2efbd3009 100644 --- a/crates/obs/src/telemetry/dial9/enabled.rs +++ b/crates/obs/src/telemetry/dial9/enabled.rs @@ -51,11 +51,6 @@ impl Dial9SessionGuard { pub fn is_active(&self) -> bool { self.guard.is_enabled() } - - /// The configuration this session was built from. - pub fn config(&self) -> &Dial9Config { - &self.config - } } impl std::fmt::Debug for Dial9SessionGuard {