Skip to content

Commit

Permalink
log when we fall back to Rust
Browse files Browse the repository at this point in the history
Summary:
# Context

Our last attempt at removing the Python version of `eden redirect` failed (see the SEV attached to D55426809). Our next attempt should be done in a phased manner to avoid similar breakages.

# Solution

We will slowly remove all possible fallbacks to Python so that we can be sure no commands are running the Python version of `eden redirect` prior to deleting the code. New rollout plan is as follows:

1) Remove the Chef recipe that writes `/etc/eden/edenfsctl_rollout.json` (Done)
2) Remove "redirect" from the default list of experimental commands in the Rust code.
3) Add a fallback to Rust if Python parsing fails for experimental commands
4) Add an EdenFS event that's logged every time we fallback to the Python `eden redirect` codepath
5) Monitor Scuba data to ensure the Python codepath isn't being hit
6) Delete Python `eden redirect` altogether

# This diff

Adds scuba logging if we hit the fallback added in #3

Reviewed By: kmancini

Differential Revision: D56334269

fbshipit-source-id: 4360a7a3b976927465af619735a726d86196482b
  • Loading branch information
MichaelCuevas authored and facebook-github-bot committed Apr 24, 2024
1 parent d26a8f9 commit 5f3a66e
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 5 deletions.
2 changes: 1 addition & 1 deletion eden/fs/cli_rs/edenfs-commands/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ impl TopLevelSubcommand {
}
}

fn name(&self) -> &'static str {
pub fn name(&self) -> &'static str {
// TODO: Is there a way to extract the subcommand's name from clap?
// Otherwise, there is a risk of divergence with clap's own attributes.
match self {
Expand Down
31 changes: 27 additions & 4 deletions eden/fs/cli_rs/edenfsctl/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,11 @@ use edenfs_utils::strip_unc_prefix;
use fbinit::FacebookInit;
use tracing_subscriber::filter::EnvFilter;

#[cfg(not(fbcode_build))]
// For non-fbcode builds, CliUsageSample is not defined. Let's give it a dummy
// value so we can pass CliUsageSample through wrapper_main() and fallback().
struct CliUsageSample;

/// Value used in Python to indicate a command failed to parse
pub const PYTHON_EDENFSCTL_EX_USAGE: i32 = 64;

Expand Down Expand Up @@ -102,6 +107,7 @@ fn fallback(reason: Option<clap::Error>) -> Result<i32> {
let status = cmd
.status()
.with_context(|| format!("failed to execute: {:?}", cmd))?;

Ok(status.code().unwrap_or(1))
}

Expand Down Expand Up @@ -131,7 +137,7 @@ fn rust_main(cmd: edenfs_commands::MainCommand) -> Result<i32> {

/// This function takes care of the fallback logic, hijack supported subcommand
/// to Rust implementation and forward the rest to Python.
fn wrapper_main() -> Result<i32> {
fn wrapper_main(telemetry_sample: &mut CliUsageSample) -> Result<i32> {
if std::env::var("EDENFSCTL_ONLY_RUST").is_ok() {
let cmd = edenfs_commands::MainCommand::parse();
rust_main(cmd)
Expand All @@ -150,8 +156,25 @@ fn wrapper_main() -> Result<i32> {
// parse error, we should see if the Rust version
// exists. This helps prevent cases where rollouts
// are not working correctly.
Ok(PYTHON_EDENFSCTL_EX_USAGE) => rust_main(cmd),
res => res,
Ok(PYTHON_EDENFSCTL_EX_USAGE) => {
#[cfg(fbcode_build)]
{
// We expected to use Python but we were forced
// to fall back to Rust. Something is wrong.
telemetry_sample.set_rust_fallback(true);
}
eprintln!(
"Failed to find Python implementation; falling back to Rust."
);
rust_main(cmd)
}
res => {
#[cfg(fbcode_build)]
{
telemetry_sample.set_rust_fallback(false);
}
res
}
}
}
}
Expand Down Expand Up @@ -208,7 +231,7 @@ fn main(_fb: FacebookInit) -> Result<()> {
#[cfg(fbcode_build)]
let mut sample = CliUsageSample::build();

let code = match wrapper_main() {
let code = match wrapper_main(&mut sample) {
Ok(code) => Ok(code),
Err(e) => {
#[cfg(fbcode_build)]
Expand Down

0 comments on commit 5f3a66e

Please sign in to comment.