yabeda-rb / yabeda-prometheus

Adapter to expose metrics collected by Yabeda plugins to Prometheus
MIT License
112 stars 18 forks source link

Allow to mount on Rails #2

Closed elct9620 closed 5 years ago

elct9620 commented 5 years ago

I try to use this gem with yabed-rails but it seems not works well for me.

But I think we can add a small change to allow it mountable on Rails

# config/routes.rb

mount Yabed::Prometheus::Exporter.server => "/"

If you think this patch is ok, I will send a PR to this reop.

The inspect method is to prevent rails routes print a plain class with a lot unused information

diff --git a/lib/yabeda/prometheus/exporter.rb b/lib/yabeda/prometheus/exporter.rb
index cdbd7c3..dc9b048 100644
--- a/lib/yabeda/prometheus/exporter.rb
+++ b/lib/yabeda/prometheus/exporter.rb
@@ -10,7 +10,7 @@ module Yabeda
           Thread.new do
             default_port = ENV.fetch("PORT", 9394)
             Rack::Handler::WEBrick.run(
-              rack_app,
+              server,
               Host: ENV["PROMETHEUS_EXPORTER_BIND"] || "0.0.0.0",
               Port: ENV.fetch("PROMETHEUS_EXPORTER_PORT", default_port),
               AccessLog: [],
@@ -18,10 +18,11 @@ module Yabeda
           end
         end

-        protected
-
-        def rack_app(exporter = self)
+        def server(exporter = self)
           Rack::Builder.new do
+            def inspect
+              "Yabed::Prometheus v#{VERSION}"
+            end
             use Rack::CommonLogger
             use Rack::ShowExceptions
             use exporter, registry: Yabeda::Prometheus.registry
(
Envek commented 5 years ago

Hello and thank you for interest in Yabeda!

Whether recommended in README method through config.ru file doesn't work for you? Why?

# config.ru
use Yabeda::Prometheus::Exporter
require ::File.expand_path("../config/environment", __FILE__)
Rails.application.eager_load!
run Rails.application

I will look on how mounting to config/routes.rb approach can be implemented too.


In general I don't recommend to expose metrics through primary web interface. They may contain sensitive data about your app performance which you probably do not want to be available publicly. Running another web server in separate thread dedicated for serving metrics may be more desirable.

elct9620 commented 5 years ago

Hi,

Put it inside config.ru is works for me, but in a Rails project I think it should not put inside config.ru. I think it should use middleware API to add it.

But this way is not make sense for people want to attach another Rack application, so I think the best way is to allow it mountable. And we also be allowed to configure it and not depend on Rails.


I know the security problem, but I think it not alwasy better than same port with application. It still has risk when we running it on the internet. In my case, I plan to mount on Rails and config the web server (or reverse proxy) to limit the access for it.

Ex.

# nginx config

location /metrics {
  allow 10.0.0.1/24;
  deny all;
}

I think add some basic auth middleware or SSL is necessary if we run it on production, so mount it on Rails and config web server to limit access is more flexible in my use case.

Envek commented 5 years ago

Upgrade to v0.1.3 and mount it to config/routes.rb:

mount Yabeda::Prometheus::Exporter => "/metrics"

Yes, restricting access on reverse proxy is absolutely fine too.