cloudflare / pingora

A library for building fast, reliable and evolvable network services.
Apache License 2.0
20.21k stars 1.1k forks source link

How to add third party module when use pingora as dependency? #294

Closed taikulawo closed 1 day ago

taikulawo commented 1 week ago

I want to add my module like

let mut proxy = HttpProxy::new(inner, conf.clone());
    // Add disabled downstream compression module by default
    proxy
        .downstream_modules
        .add_module(ResponseCompressionBuilder::enable(0));
    Service::new(name.to_string(), proxy)
}

But HttpProxy::new is private, I can't call it outside

https://github.com/cloudflare/pingora/blob/a432c2da9b71a2cf1e5169cabc1205cc272c4c9c/pingora-proxy/src/lib.rs#L102

JosiahParry commented 1 week ago

Would implementing the ProxyHttp trait get you far enough?

eaufavor commented 1 week ago

(This is all on the master branch but not released yet) The pingora_core::services::listening::Service has a app_logic_mut() method that points to the HttpProxy inside it. From there you can access downstream_modules.

We probably need a few examples and maybe more intuitive APIs to add 3rd party modules.

taikulawo commented 1 week ago

http_proxy_service_with_name will add a default ResponseCompressionBuilder, I don't want it.

taikulawo commented 1 week ago

also type Module isn't public to outside type Module = Box<dyn HttpModule + 'static + Send + Sync>;

struct ChangeOntheFlyBuilder {}
impl HttpModuleBuilder for ChangeOntheFlyBuilder {
    fn init(&self) -> Module {
        Box::new(ChangeOnTheFly {})
    }

    fn order(&self) -> i16 {
        0
    }
}

cannot find type Module in this scope not found in this scope

eaufavor commented 1 week ago

http_proxy_service_with_name will add a default ResponseCompressionBuilder, I don't want it.

We might add an API to remove modules. The compression module is inert by default.

Beside you can remove it by assigning a fresh HttpModules to it.

proxy.app_logic_mut().unwrap().downstream_modules = pingora::modules::http::HttpModules::new();
eaufavor commented 1 week ago

also type Module isn't public to outside type Module = Box<dyn HttpModule + 'static + Send + Sync>;

It is just an alias. You can use Box<dyn HttpModule + 'static + Send + Sync>. Meanwhile, we will also make it public to make it easier to use.

taikulawo commented 1 week ago

thankyou, but why not just export HttpProxy::new

eaufavor commented 1 week ago

why not just export HttpProxy::new

It is not the best UX if you have to call HttpProxy::new, add modules, and then put it into a Service all manually. We are working on bringing in https://github.com/cloudflare/pingora/pull/284 so that users can add modules more ergonomically.

taikulawo commented 6 days ago

Also, I can call Service::handle_event(pingora_stream, self.inner.clone(), self.close_recv.clone()).await; outside because or handle_event is public, but this require we create a HttpProxy instance.

How we can call handle_event without create service first?

eaufavor commented 1 day ago

I don't think that is supported. This question is a bit off topic. So if it is needed, create a standalone issue for it please.