Skip to content

Commit

Permalink
tmp: Split up process_cli
Browse files Browse the repository at this point in the history
This is part 1 of 2 for applying the changes recommended in #7600,
keeping the diff minimal by breaking process_cli into three pieces so we
can extract startup_context_from_env() from the middle.

This will be squashed into the prior commit before merging. Kept it
separate for now, just to make the changes easier to review.

Co-authored-by: Heikki Linnakangas <heikki@neon.tech>
  • Loading branch information
sharnoff and hlinnaka committed May 4, 2024
1 parent c947243 commit 8284db0
Showing 1 changed file with 49 additions and 28 deletions.
77 changes: 49 additions & 28 deletions compute_tools/src/bin/compute_ctl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,12 +71,18 @@ const BUILD_TAG_DEFAULT: &str = "latest";
fn main() -> Result<()> {
let (build_tag, clap_args) = init()?;

let (startup_context_guard, cli_result) = process_cli(&clap_args)?;
let cli_args = process_cli(&clap_args)?;

let wait_spec_result = wait_spec(build_tag, cli_result)?;
// Enter startup tracing context
let startup_context_guard = startup_context_from_env();

let cli_spec = try_spec_from_cli(&clap_args, &cli_args)?;

let wait_spec_result = wait_spec(build_tag, cli_args, cli_spec)?;

let (pg_handle, start_pg_result) = start_postgres(&clap_args, wait_spec_result)?;

// PostgreSQL is now running, if startup was successful. Wait until it exits.
let wait_pg_result = wait_postgres(pg_handle, startup_context_guard)?;

cleanup_and_exit(start_pg_result, wait_pg_result)
Expand All @@ -100,9 +106,7 @@ fn init() -> Result<(String, clap::ArgMatches)> {
Ok((build_tag, cli().get_matches()))
}

fn process_cli(
matches: &clap::ArgMatches,
) -> Result<(Option<opentelemetry::ContextGuard>, ProcessCliResult)> {
fn process_cli(matches: &clap::ArgMatches) -> Result<ProcessCliResult> {
let pgbin_default = "postgres";
let pgbin = matches
.get_one::<String>("pgbin")
Expand Down Expand Up @@ -134,6 +138,28 @@ fn process_cli(
let spec_json = matches.get_one::<String>("spec");
let spec_path = matches.get_one::<String>("spec-path");

Ok(ProcessCliResult {
connstr,
pgdata,
pgbin,
ext_remote_storage,
http_port,
spec_json,
spec_path,
})
}

struct ProcessCliResult<'clap> {
connstr: &'clap str,
pgdata: &'clap str,
pgbin: &'clap str,
ext_remote_storage: Option<&'clap str>,
http_port: u16,
spec_json: Option<&'clap String>,
spec_path: Option<&'clap String>,
}

fn startup_context_from_env() -> Option<opentelemetry::ContextGuard> {
// Extract OpenTelemetry context for the startup actions from the
// TRACEPARENT and TRACESTATE env variables, and attach it to the current
// tracing context.
Expand Down Expand Up @@ -170,7 +196,7 @@ fn process_cli(
if let Ok(val) = std::env::var("TRACESTATE") {
startup_tracing_carrier.insert("tracestate".to_string(), val);
}
let startup_context_guard = if !startup_tracing_carrier.is_empty() {
if !startup_tracing_carrier.is_empty() {
use opentelemetry::propagation::TextMapPropagator;
use opentelemetry::sdk::propagation::TraceContextPropagator;
let guard = TraceContextPropagator::new()
Expand All @@ -180,8 +206,17 @@ fn process_cli(
Some(guard)
} else {
None
};
}
}

fn try_spec_from_cli(
matches: &clap::ArgMatches,
ProcessCliResult {
spec_json,
spec_path,
..
}: &ProcessCliResult,
) -> Result<CliSpecParams> {
let compute_id = matches.get_one::<String>("compute-id");
let control_plane_uri = matches.get_one::<String>("control-plane-uri");

Expand Down Expand Up @@ -222,30 +257,13 @@ fn process_cli(
}
};

let result = ProcessCliResult {
// directly from CLI:
connstr,
pgdata,
pgbin,
ext_remote_storage,
http_port,
// others:
Ok(CliSpecParams {
spec,
live_config_allowed,
};

// TODO: Move startup_context_guard out of this function. It's here right now only because
// that's where it was before, and moving it would've made the diff big.
Ok((startup_context_guard, result))
})
}

struct ProcessCliResult<'clap> {
connstr: &'clap str,
pgdata: &'clap str,
pgbin: &'clap str,
ext_remote_storage: Option<&'clap str>,
http_port: u16,

struct CliSpecParams {
/// If a spec was provided via CLI or file, the [`ComputeSpec`]
spec: Option<ComputeSpec>,
live_config_allowed: bool,
Expand All @@ -259,9 +277,12 @@ fn wait_spec(
pgbin,
ext_remote_storage,
http_port,
..
}: ProcessCliResult,
CliSpecParams {
spec,
live_config_allowed,
}: ProcessCliResult,
}: CliSpecParams,
) -> Result<WaitSpecResult> {
let mut new_state = ComputeState::new();
let spec_set;
Expand Down

0 comments on commit 8284db0

Please sign in to comment.