Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better tracing #48

Closed
2 tasks
Hoverbear opened this issue Nov 10, 2022 · 1 comment
Closed
2 tasks

Better tracing #48

Hoverbear opened this issue Nov 10, 2022 · 1 comment

Comments

@Hoverbear
Copy link
Contributor

Right now, unless tracing is enabled, Harmonic is mostly silent during install except during plan prompts.

When tracing is enabled, there is significant boilerplate in each action to enable messaging:

https://github.com/DeterminateSystems/harmonic/blob/673464ede7bac886f5611541eb2706037afbf17f/src/action/common/configure_nix.rs#L102-L127

We should:

  • Have a more standard way for debug/trace messaging like above to happen for actions.
    • Maybe using a const and a wrapper function? We should be careful about not showing the tracing span fields accidentally.
  • Consider and explore how to do good info level logging here.
@Hoverbear
Copy link
Contributor Author

I was experimenting with a pattern like

#[async_trait::async_trait]
#[typetag::serde(tag = "action")]
pub trait Action: Send + Sync + std::fmt::Debug + dyn_clone::DynClone {
    fn execute_description(&self) -> String;
    fn revert_description(&self) -> String;
    async fn execute(&mut self) -> Result<(), Box<dyn std::error::Error + Send + Sync>>;
    async fn revert(&mut self) -> Result<(), Box<dyn std::error::Error + Send + Sync>>;
    fn action_state(&self) -> ActionState;
    fn set_action_state(&mut self, new_state: ActionState);
    fn explanation(&self) -> Vec<String> {
        vec![]
    }

    // They should also have an `async fn plan(args...) -> Result<ActionState<Self>, Box<dyn std::error::Error + Send + Sync>>;`
}


#[async_trait::async_trait]
trait ActionImplementation: Action {
    fn describe_execute(&self) -> Option<String> {
        if self.action_state() == ActionState::Completed {
            return None;
        }
        return Some(self.execute_description());
    }
    fn describe_revert(&self) -> Option<String> {
        if self.action_state() == ActionState::Uncompleted {
            return None;
        }
        return Some(self.revert_description());
    }

    async fn try_execute(&mut self) -> Result<(), Box<dyn std::error::Error + Send + Sync>> {
        if self.action_state() == ActionState::Completed {
            tracing::trace!("Completed: (Already done) {}", self.execute_description());
            return Ok(());
        }
        self.set_action_state(ActionState::Progress);
        tracing::debug!("Executing: {}", self.execute_description());
        self.execute().await?;
        self.set_action_state(ActionState::Completed);
        tracing::debug!("Completed: {}", self.execute_description());
        Ok(())
    }

    async fn try_revert(&mut self) -> Result<(), Box<dyn std::error::Error + Send + Sync>> {
        if self.action_state() == ActionState::Uncompleted {
            tracing::trace!("Reverted: (Already done) {}", self.revert_description());
            return Ok(());
        }
        self.set_action_state(ActionState::Progress);
        tracing::debug!("Reverting: {}", self.revert_description());
        self.revert().await?;
        tracing::debug!("Reverted: {}", self.revert_description());
        self.set_action_state(ActionState::Uncompleted);
        Ok(())
    }
}

But I feel this doesn't do a great job handling meta-actions such as ProvisionNix since it can only output that one...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant