akoutmos / prom_ex

An Elixir Prometheus metrics collection library built on top of Telemetry with accompanying Grafana dashboards
MIT License
594 stars 104 forks source link

Customize Plugins like PlugRouter to pass in custom distribution buckets #107

Open pauljamescleary opened 2 years ago

pauljamescleary commented 2 years ago

Is your feature request related to a problem? Please describe. For defining SLI metrics / dashboards, we need the ability to align latency buckets with our latency thresholds for HTTP request.

Describe the solution you would like to see We use your library extensively (thank you), but would like the ability to pass in additional configuration options (specifically the latency buckets).

Ideally, we could pass in as config options (like metric prefix today) and specify latency buckets (reporter_options for buckets).

Here is the relevant code (it is similar for Phoneix and Plug Router) - https://github.com/akoutmos/prom_ex/blob/ca899bdc92d7372609b46685127faba7f02d8102/lib/prom_ex/plugins/plug_router.ex#L173

How would you expect this feature to work User can define latency buckets when the plugin is setup...


      @impl true
      def plugins do
        [
          ...
          {Plugins.PlugRouter,
            event_prefix: [:webapp, :router], 
            metric_prefix: [:prom_ex, :router], 
            routers: [WebApp.Router],
            latency_buckets: [1, 5, 10, 20, 50, 100, 500, 1000, 2000, 5000]
          }
        ]
      end
akoutmos commented 2 years ago

Hey Paul and sorry for the delay. Been a little afk during the holiday season :).

This has been on my mind for a while as well, I just haven't come up with a good solution for it. The reason for this being that most plugins have multiple distribution metric types, so you couldn't define 1 bucket list and pass that to the plugin as the plugin wouldn't know what data point the buckets apply to. The other issue is that some of the bucket intervals are referenced in the Grafana dashboards and so those would also need to be slightly configurable.

At the moment, I am thinking something along the lines of what you have in your example, but instead of latency_buckets, it would be something like so:

@impl true
      def plugins do
        [
          ...
          {Plugins.PlugRouter,
            event_prefix: [:webapp, :router], 
            metric_prefix: [:prom_ex, :router], 
            routers: [WebApp.Router],
            distribution_buckets: %{
              [:http, :request, :duration, :milliseconds] => [1, 5, 10, 20, 50, 100, 500, 1000, 2000, 5000]
            }
          }
        ]
      end

This way you can override the specific datapoint that you want, and leave the other datapoints as the defaults that I provide.

Thoughts?

romul commented 2 years ago

@akoutmos Any news for this one? Would be great to have the opportunity to customize buckets for all plugins.