Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

✨ feat: allow adding new prometheus.Gatherers on top of default registry #2870

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions pkg/manager/internal.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"time"

"github.com/go-logr/logr"
"github.com/prometheus/client_golang/prometheus"
"k8s.io/apimachinery/pkg/api/meta"
"k8s.io/apimachinery/pkg/runtime"
kerrors "k8s.io/apimachinery/pkg/util/errors"
Expand Down Expand Up @@ -197,6 +198,22 @@ func (cm *controllerManager) AddMetricsServerExtraHandler(path string, handler h
return nil
}

// AddMetricsServerExtraGatherer adds an extra prometheus.Gatherer to the metrics server.
func (cm *controllerManager) AddMetricsServerExtraGatherer(gatherer prometheus.Gatherer) error {
cm.Lock()
defer cm.Unlock()
if cm.started {
return fmt.Errorf("unable to add new gatherer because metrics endpoint has already been created")
}
if cm.metricsServer == nil {
cm.GetLogger().Info("warn: metrics server is currently disabled, registering extra gatherer will be ignored")
return nil
}
cm.metricsServer.AddExtraGatherer(gatherer)
cm.logger.V(2).Info("Registering metrics http server extra gatherer")
return nil
}

// AddHealthzCheck allows you to add Healthz checker.
func (cm *controllerManager) AddHealthzCheck(name string, check healthz.Checker) error {
cm.Lock()
Expand Down
5 changes: 5 additions & 0 deletions pkg/manager/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"time"

"github.com/go-logr/logr"
"github.com/prometheus/client_golang/prometheus"
coordinationv1 "k8s.io/api/coordination/v1"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/meta"
Expand Down Expand Up @@ -73,6 +74,10 @@ type Manager interface {
// a new http server/listener should be added as Runnable to the manager via Add method.
AddMetricsServerExtraHandler(path string, handler http.Handler) error

// AddMetricsServerExtraGatherer adds an extra prometheus.Gatherer to the metrics server.
// This can be used to add custom metrics to the metrics server.
AddMetricsServerExtraGatherer(gatherer prometheus.Gatherer) error

// AddHealthzCheck allows you to add Healthz checker
AddHealthzCheck(name string, check healthz.Checker) error

Expand Down
44 changes: 44 additions & 0 deletions pkg/manager/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1303,6 +1303,50 @@ var _ = Describe("manger.Manager", func() {
Expect(ok).To(BeTrue())
})

It("should serve metrics in an extra registry", func() {
extraRegistry := prometheus.NewRegistry()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The prometheus registry itself implements Collector so could instead just do a metrics.Registry.MustRegister(extraRegistry) or not?

one := prometheus.NewCounter(prometheus.CounterOpts{
Name: "test_one",
Help: "test metric for testing",
})
one.Inc()
err := extraRegistry.Register(one)
Expect(err).NotTo(HaveOccurred())

m, err := New(cfg, opts)
Expect(err).NotTo(HaveOccurred())

Expect(m.AddMetricsServerExtraGatherer(extraRegistry)).To(Succeed())
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
go func() {
defer GinkgoRecover()
Expect(m.Start(ctx)).NotTo(HaveOccurred())
}()
<-m.Elected()
// Note: Wait until metrics server has been started. A finished leader election
// doesn't guarantee that the metrics server is up.
Eventually(func() string { return defaultServer.GetBindAddr() }, 10*time.Second).ShouldNot(BeEmpty())

metricsEndpoint := fmt.Sprintf("http://%s/metrics", defaultServer.GetBindAddr())
resp, err := http.Get(metricsEndpoint)
Expect(err).NotTo(HaveOccurred())
defer resp.Body.Close()
Expect(resp.StatusCode).To(Equal(200))

data, err := io.ReadAll(resp.Body)
Expect(err).NotTo(HaveOccurred())
Expect(string(data)).To(ContainSubstring("%s\n%s\n%s\n",
`# HELP test_one test metric for testing`,
`# TYPE test_one counter`,
`test_one 1`,
))

// Unregister will return false if the metric was never registered
ok := extraRegistry.Unregister(one)
Expect(ok).To(BeTrue())
})

It("should serve extra endpoints", func() {
opts.Metrics.ExtraHandlers = map[string]http.Handler{
"/debug": http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
Expand Down
17 changes: 16 additions & 1 deletion pkg/metrics/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"time"

"github.com/go-logr/logr"
"github.com/prometheus/client_golang/prometheus"
"github.com/prometheus/client_golang/prometheus/promhttp"
"k8s.io/client-go/rest"
certutil "k8s.io/client-go/util/cert"
Expand All @@ -49,6 +50,9 @@ type Server interface {
// AddExtraHandler adds extra handler served on path to the http server that serves metrics.
AddExtraHandler(path string, handler http.Handler) error

// AddExtraGatherer adds a gatherer to the metrics server that is used on top of the default via metrics.Registry.
AddExtraGatherer(gatherer prometheus.Gatherer)

// NeedLeaderElection implements the LeaderElectionRunnable interface, which indicates
// the metrics server doesn't need leader election.
NeedLeaderElection() bool
Expand Down Expand Up @@ -76,6 +80,11 @@ type Options struct {
// server/listener should be added as Runnable to the manager via the Add method.
ExtraHandlers map[string]http.Handler

// ExtraGatherers contains a list of gatherers that will be added to the metrics server.
// This might be useful to add custom metrics to the metrics server that are not registerable
// via the metrics.Registry directly, such as prebuilt registries from libraries.
ExtraGatherers []prometheus.Gatherer

// FilterProvider provides a filter which is a func that is added around
// the metrics and the extra handlers on the metrics server.
// This can be e.g. used to enforce authentication and authorization on the handlers
Expand Down Expand Up @@ -202,6 +211,10 @@ func (s *defaultServer) AddExtraHandler(path string, handler http.Handler) error
return nil
}

func (s *defaultServer) AddExtraGatherer(gatherer prometheus.Gatherer) {
s.options.ExtraGatherers = append(s.options.ExtraGatherers, gatherer)
}

// Start runs the server.
// It will install the metrics related resources depend on the server configuration.
func (s *defaultServer) Start(ctx context.Context) error {
Expand All @@ -218,7 +231,9 @@ func (s *defaultServer) Start(ctx context.Context) error {

mux := http.NewServeMux()

handler := promhttp.HandlerFor(metrics.Registry, promhttp.HandlerOpts{
gatherer := prometheus.Gatherers(append([]prometheus.Gatherer{metrics.Registry}, s.options.ExtraGatherers...))

handler := promhttp.HandlerFor(gatherer, promhttp.HandlerOpts{
ErrorHandling: promhttp.HTTPErrorOnError,
})
if s.metricsFilter != nil {
Expand Down