metrics: Use custom registry to avoid metric conflicts (#188)

* metrics: Use custom registry to avoid metric conflicts

* api: Create a small test for the metrics

* Revert "metrics: Use custom registry to avoid metric conflicts"

This reverts commit fcd215b.

* api: Add some failing tests

* Revert "Revert "metrics: Use custom registry to avoid metric conflicts""

This reverts commit 0016b52.

Author: victorges

api/handler.go

@@ -11,7 +11,6 @@ import (
 	"github.com/livepeer/task-runner/clients"
 	"github.com/livepeer/task-runner/metrics"
 	"github.com/livepeer/task-runner/task"
-	"github.com/prometheus/client_golang/prometheus/promhttp"
 )
 
 type APIHandlerOptions struct {
@@ -32,7 +31,7 @@ func NewHandler(serverCtx context.Context, opts APIHandlerOptions, runner task.R
 
 	router.HandlerFunc("GET", "/_healthz", router.healthcheck)
 	if opts.Prometheus {
-		router.Handler("GET", "/metrics", promhttp.Handler())
+		router.Handler("GET", "/metrics", metrics.ScrapeHandler())
 	}
 
 	hookHandler := metrics.ObservedHandlerFunc("catalyst_hook", router.catalystHook)

api/handler_test.go

@@ -0,0 +1,42 @@
+package api
+
+import (
+	"context"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/livepeer/task-runner/clients"
+	"github.com/stretchr/testify/require"
+)
+
+func TestMetricsEndpoint(t *testing.T) {
+	require := require.New(t)
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
+	handler := NewHandler(ctx, APIHandlerOptions{
+		Catalyst:   &clients.CatalystOptions{Secret: "🤫"},
+		Prometheus: true,
+	}, nil)
+
+	req, err := http.NewRequest("GET", "http://localhost/metrics", nil)
+	require.NoError(err)
+
+	rw := httptest.NewRecorder()
+	handler.ServeHTTP(rw, req)
+
+	res := rw.Result()
+	require.Equal(http.StatusOK, res.StatusCode)
+	require.Contains(res.Header.Get("Content-Type"), "text/plain")
+
+	body := rw.Body.String()
+	require.NotEmpty(body)
+
+	// check that it contains some random metric
+	require.Contains(body, "# TYPE livepeer_task_runner_http_requests_in_flight gauge\n")
+	// check that it does NOT contain metrics from some libs we use
+	require.NotContains(body, "catalyst-api")
+	require.NotContains(body, "transcode_segment_duration_seconds")
+	require.NotContains(body, "livepeer_analyzer")
+}

metrics/factory.go

@@ -1,16 +1,28 @@
 package metrics
 
 import (
+	"net/http"
+
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/prometheus/client_golang/prometheus/promauto"
+	"github.com/prometheus/client_golang/prometheus/promhttp"
 )
 
 var (
 	Namespace = "livepeer"
 	Subsystem = "task_runner"
-	Factory   = promauto.With(prometheus.DefaultRegisterer)
+	// We use catalyst-api as a library which uses prometheus lib and default
+	// registry. Avoid metric conflicts by using a custom registry.
+	Registry = prometheus.NewRegistry()
+	Factory  = promauto.With(Registry)
 )
 
 func FQName(name string) string {
 	return prometheus.BuildFQName(Namespace, Subsystem, name)
 }
+
+func ScrapeHandler() http.Handler {
+	handler := promhttp.HandlerFor(Registry, promhttp.HandlerOpts{})
+	handler = promhttp.InstrumentMetricHandler(Registry, handler)
+	return handler
+}