autometrics-dev / am

Autometrics Companion CLI app
Apache License 2.0
16 stars 2 forks source link

Implement `am proxy` command #128

Closed brettimus closed 1 year ago

brettimus commented 1 year ago

Description

This PR adds support for an am proxy command, which would

  1. Start serving explorer, as usual (can still be configured via --listen-address and --explorer-address)
  2. Optionally proxy requests to /prometheus to a user-specified url (configured via --prometheus-url)
  3. NOT download or launch a local Prometheus

So, if we already had prometheus running on http://localhost:8063 for some odd reason, we could run:

am proxy --prometheus-url http://localhost:8063

And am would not download/launch prometheus, but explorer would still be accessible and automagically connected to the Prometheus instance we're interested in connecting to.

For what it's worth, I tested that scenario, and it does indeed work.

Where I need help

I wanted to create a handler factory, but struggled immensely with the type definition, because you can't use impl Trait to describe the return type for a Fn trait (https://github.com/rust-lang/rust/issues/99697).

What I wanted was something that looked like:

if is_proxying_prometheus {
        app = app
            .route("/prometheus/*path", any(prometheus::handler_factory(prometheus_upstream_base)))
            .route("/prometheus",  any(prometheus::handler_factory(prometheus_upstream_base)));
    }

Ultimately, I gave up, and opted to use inline blocks/closures to define the handlers for this functionality.

Additionally, I figured we would need to rewrite the path of any requests to /prometheus/* before forwarding to the external prometheus. There has to be a better/more concise way than I chose.

Basically, this PR still needs some cleanup, api review (is proxy the right command name even?), and probably some tests.

Checklist

hatchan commented 1 year ago

Additionally, I figured we would need to rewrite the path of any requests to /prometheus/* before forwarding to the external prometheus. There has to be a better/more concise way than I chose.

I think this is the correct way of dealing with this :+1:

Basically, this PR still needs some cleanup, api review (is proxy the right command name even?), and probably some tests.

I think proxy is the best name.