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

proposal: more ergonomic apis for modules #157

Open
banditopazzo opened this issue Feb 11, 2023 · 0 comments
Open

proposal: more ergonomic apis for modules #157

banditopazzo opened this issue Feb 11, 2023 · 0 comments
Assignees
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@banditopazzo
Copy link
Member

I find the modules start task function a bit strange, expecially the:

loop {
    tokio::select! {
    r = shutdown.recv() => return r,
    _ = rx_config.changed() => ...
    msg = receiver.recv() => ...
}

at the current state, this is an obligated pattern modules should do.

Another reason is that for developers is simpler to work with objects as it feels more natural.

My proposal is to have something similar to this for the module trait:

// module name
pub struct Name2(pub String);

// module semver
pub struct Version2(pub i32);

// needed by the upper level to decide if allocate a receiver or not
pub enum OnEvent<'a> {
    Nothing,
    Do(EventHandler<'a>),
}

impl<'a> OnEvent<'a> {
    pub fn execute<F: 'a + FnMut() -> Result<(), Box<dyn std::error::Error>>>(f: F) -> Self {
        Self::Do(EventHandler(Box::new(f)))
    }
}

pub struct EventHandler<'a>(Box<dyn FnMut() -> Result<(), Box<dyn std::error::Error>> + 'a>);

// context similar to the current to send events and errors
pub struct Cx2 {}

// raw module configuration
pub struct ModuleConfig2 {}

// event
#[derive(Debug)]
pub struct Event2 {}

// module trait
pub trait Module2: Sized {
    type Config: for<'a> TryFrom<&'a ModuleConfig2>;

    fn start(config: &Self::Config, cx: &Cx2) -> (Name2, Version2, Self);
    
    // default for producer only modules
    fn on_event<'a>(
        &'a mut self,
        config: &'a Self::Config,
        event: &'a Event2,
        cx: &'a Cx2,
    ) -> OnEvent<'a> {
        OnEvent::Nothing
    }

    fn on_cfg_change(
        &mut self,
        config: &Self::Config,
        cx: &Cx2,
    ) -> Result<(), Box<dyn std::error::Error>> {
        Ok(())
    }

    // default normal drop
    fn graceful_stop(self, cx: &Cx2) {}
}

and move the loop/select block to the upper level into the ModuleManager as it's the same for every module.

The following lines are previews of how modules should look with new traits:

mod producer1 {
    use pdk::*;

    struct MyProducerModuleConfig {
        file_path: String,
    }

    impl TryFrom<&ModuleConfig2> for MyProducerModuleConfig {
        type Error = String;

        fn try_from(value: &ModuleConfig2) -> std::result::Result<Self, Self::Error> {
            todo!()
        }
    }

    struct MyProducerModule {
        ebpf_probe: i32, // in reality Program type
    }

    impl Module2 for MyProducerModule {
        type Config = MyProducerModuleConfig;

        fn start(config: &Self::Config, cx: &Cx2) -> (Name2, Version2, Self) {
            let ebpf_probe = 0; // program_start(...)
            (
                Name2("producer".to_string()),
                Version2(666),
                Self { ebpf_probe },
            )
        }
    }
}

mod consumer1 {
    use pdk::*;

    struct MyConsumerModuleConfig {
        file_path: String,
    }

    impl TryFrom<&ModuleConfig2> for MyConsumerModuleConfig {
        type Error = String;

        fn try_from(value: &ModuleConfig2) -> std::result::Result<Self, Self::Error> {
            todo!()
        }
    }

    struct MyConsumerModule {
        file_writer: i32,
    }

    impl Module2 for MyConsumerModule {
        type Config = MyConsumerModuleConfig;

        fn start(config: &Self::Config, cx: &Cx2) -> (Name2, Version2, Self) {
            let file_writer = 0;
            (
                Name2("consumer".to_string()),
                Version2(666),
                Self { file_writer },
            )
        }

        fn on_event<'a>(
            &'a mut self,
            config: &'a Self::Config,
            event: &'a Event2,
            cx: &'a Cx2,
        ) -> OnEvent<'a> {
            OnEvent::execute(move || {
                // self.file_writer.write(event);
                todo!()
            })
        }

        fn graceful_stop(self, cx: &Cx2) {
            // file_writer.flush()
            // file_writer.close()
        }
    }
}

mod consumer2 {
    use pdk::*;

    struct MyEngineConfig {
        option1: bool,
    }

    impl TryFrom<&ModuleConfig2> for MyEngineConfig {
        type Error = String;

        fn try_from(value: &ModuleConfig2) -> std::result::Result<Self, Self::Error> {
            todo!()
        }
    }

    struct MyEngine {
        engine: i32,
    }

    impl Module2 for MyEngine {
        type Config = MyEngineConfig;

        fn start(config: &Self::Config, cx: &Cx2) -> (Name2, Version2, Self) {
            let engine = 0; //  = PulsarEngine::new(&config.rules_path, cx.get_sender())?;
            (
                Name2("consumer2".to_string()),
                Version2(666),
                Self { engine },
            )
        }

        fn on_cfg_change(&mut self, config: &Self::Config, cx: &Cx2)-> Result<(), Box<dyn std::error::Error>> {
            //  self.engine = PulsarEngine::new(&config.rules_path, ctx.get_sender())?;
            Ok(())
        }

        fn on_event<'a>(
            &'a mut self,
            config: &'a Self::Config,
            event: &'a Event2,
            cx: &'a Cx2,
        ) -> OnEvent<'a> {
            OnEvent::execute(move || {
                // println!("event consumed: {event:?}");
                // engine.process(&event);
                Ok(())
            })
        }
    }
}

Disclaimer: this code is not complete, but it should resemble what we will get in the end, signatures can be polished. it should compile apart from possible errors adapting to this issue.

Note: maybe the type erasure on the handle function is not needed, even the EventHandler shim with the execute function, but it needs a full test.

@banditopazzo banditopazzo added enhancement New feature or request help wanted Extra attention is needed labels Feb 11, 2023
This was referenced May 10, 2024
@banditopazzo banditopazzo self-assigned this May 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant