Refactor metrics registry exporter (#3989)

* Refactoring approach1 * add MetricsConfig struct * nit * lint error * refactored allocator --------- Co-authored-by: Mengye (Max) Gong <[email protected]>
googleforgames · Sep 20, 2024 · 19c1700 · 19c1700
1 parent 5c18bef
commit 19c1700
Show file tree

Hide file tree

Showing 5 changed files with 95 additions and 108 deletions.
diff --git a/cmd/allocator/main.go b/cmd/allocator/main.go
@@ -26,6 +26,16 @@ import (
 	"sync"
 	"time"
 
+	"agones.dev/agones/pkg"
+	"agones.dev/agones/pkg/allocation/converters"
+	pb "agones.dev/agones/pkg/allocation/go"
+	allocationv1 "agones.dev/agones/pkg/apis/allocation/v1"
+	"agones.dev/agones/pkg/client/clientset/versioned"
+	"agones.dev/agones/pkg/client/informers/externalversions"
+	"agones.dev/agones/pkg/gameserverallocations"
+	"agones.dev/agones/pkg/gameservers"
+	"agones.dev/agones/pkg/metrics"
+	"agones.dev/agones/pkg/util/fswatch"
 	"github.com/heptiolabs/healthcheck"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
@@ -45,15 +55,6 @@ import (
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/rest"
 
-	"agones.dev/agones/pkg"
-	"agones.dev/agones/pkg/allocation/converters"
-	pb "agones.dev/agones/pkg/allocation/go"
-	allocationv1 "agones.dev/agones/pkg/apis/allocation/v1"
-	"agones.dev/agones/pkg/client/clientset/versioned"
-	"agones.dev/agones/pkg/client/informers/externalversions"
-	"agones.dev/agones/pkg/gameserverallocations"
-	"agones.dev/agones/pkg/gameservers"
-	"agones.dev/agones/pkg/util/fswatch"
 	"agones.dev/agones/pkg/util/httpserver"
 	"agones.dev/agones/pkg/util/runtime"
 	"agones.dev/agones/pkg/util/signals"
@@ -218,9 +219,19 @@ func main() {
 		logger.WithField("grpc-port", conf.GRPCPort).WithField("http-port", conf.HTTPPort).Fatal("Must specify a valid gRPC port or an HTTP port for the allocator service")
 	}
 	healthserver := &httpserver.Server{Logger: logger}
-	health, closer := setupMetricsRecorder(conf, healthserver)
+	var health healthcheck.Handler
+
+	metricsConf := metrics.Config{
+		Stackdriver:       conf.Stackdriver,
+		PrometheusMetrics: conf.PrometheusMetrics,
+		GCPProjectID:      conf.GCPProjectID,
+		StackdriverLabels: conf.StackdriverLabels,
+	}
+	health, closer := metrics.SetupMetrics(metricsConf, healthserver)
 	defer closer()
 
+	metrics.SetReportingPeriod(conf.PrometheusMetrics, conf.Stackdriver)
+
 	kubeClient, agonesClient, err := getClients(conf)
 	if err != nil {
 		logger.WithError(err).Fatal("could not create clients")

diff --git a/cmd/allocator/metrics.go b/cmd/allocator/metrics.go
@@ -14,10 +14,6 @@
 package main
 
 import (
-	"agones.dev/agones/pkg/metrics"
-	"agones.dev/agones/pkg/util/httpserver"
-	"github.com/heptiolabs/healthcheck"
-	prom "github.com/prometheus/client_golang/prometheus"
 	"go.opencensus.io/plugin/ocgrpc"
 	"go.opencensus.io/stats/view"
 )
@@ -31,32 +27,3 @@ func registerMetricViews() {
 		logger.WithError(err).Error("could not register view")
 	}
 }
-
-func setupMetricsRecorder(conf config, healthserver *httpserver.Server) (health healthcheck.Handler, closer func()) {
-	health = healthcheck.NewHandler()
-	closer = func() {}
-
-	// Stackdriver metrics
-	if conf.Stackdriver {
-		sd, err := metrics.RegisterStackdriverExporter(conf.GCPProjectID, conf.StackdriverLabels)
-		if err != nil {
-			logger.WithError(err).Fatal("Could not register stackdriver exporter")
-		}
-		// It is imperative to invoke flush before your main function exits
-		closer = func() { sd.Flush() }
-	}
-
-	// Prometheus metrics
-	if conf.PrometheusMetrics {
-		registry := prom.NewRegistry()
-		metricHandler, err := metrics.RegisterPrometheusExporter(registry)
-		if err != nil {
-			logger.WithError(err).Fatal("Could not register prometheus exporter")
-		}
-		healthserver.Handle("/metrics", metricHandler)
-		health = healthcheck.NewMetricsHandler(registry, "agones")
-	}
-
-	metrics.SetReportingPeriod(conf.PrometheusMetrics, conf.Stackdriver)
-	return
-}
diff --git a/cmd/controller/main.go b/cmd/controller/main.go
@@ -25,12 +25,23 @@ import (
 	"strings"
 	"time"
 
+	"agones.dev/agones/pkg"
 	agonesv1 "agones.dev/agones/pkg/apis/agones/v1"
+	"agones.dev/agones/pkg/client/clientset/versioned"
+	"agones.dev/agones/pkg/client/informers/externalversions"
+	"agones.dev/agones/pkg/cloudproduct"
+	"agones.dev/agones/pkg/fleetautoscalers"
+	"agones.dev/agones/pkg/fleets"
+	"agones.dev/agones/pkg/gameservers"
+	"agones.dev/agones/pkg/gameserversets"
+	"agones.dev/agones/pkg/metrics"
 	"agones.dev/agones/pkg/portallocator"
+	"agones.dev/agones/pkg/util/httpserver"
+	"agones.dev/agones/pkg/util/runtime"
+	"agones.dev/agones/pkg/util/signals"
 	"github.com/google/uuid"
 	"github.com/heptiolabs/healthcheck"
 	"github.com/pkg/errors"
-	prom "github.com/prometheus/client_golang/prometheus"
 	"github.com/sirupsen/logrus"
 	"github.com/spf13/pflag"
 	"github.com/spf13/viper"
@@ -43,19 +54,6 @@ import (
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/tools/leaderelection"
 	"k8s.io/client-go/tools/leaderelection/resourcelock"
-
-	"agones.dev/agones/pkg"
-	"agones.dev/agones/pkg/client/clientset/versioned"
-	"agones.dev/agones/pkg/client/informers/externalversions"
-	"agones.dev/agones/pkg/cloudproduct"
-	"agones.dev/agones/pkg/fleetautoscalers"
-	"agones.dev/agones/pkg/fleets"
-	"agones.dev/agones/pkg/gameservers"
-	"agones.dev/agones/pkg/gameserversets"
-	"agones.dev/agones/pkg/metrics"
-	"agones.dev/agones/pkg/util/httpserver"
-	"agones.dev/agones/pkg/util/runtime"
-	"agones.dev/agones/pkg/util/signals"
 )
 
 const (
@@ -180,28 +178,15 @@ func main() {
 	var rs []runner
 	var health healthcheck.Handler
 
-	// Stackdriver metrics
-	if ctlConf.Stackdriver {
-		sd, err := metrics.RegisterStackdriverExporter(ctlConf.GCPProjectID, ctlConf.StackdriverLabels)
-		if err != nil {
-			logger.WithError(err).Fatal("Could not register stackdriver exporter")
-		}
-		// It is imperative to invoke flush before your main function exits
-		defer sd.Flush()
+	metricsConf := metrics.Config{
+		Stackdriver:       ctlConf.Stackdriver,
+		PrometheusMetrics: ctlConf.PrometheusMetrics,
+		GCPProjectID:      ctlConf.GCPProjectID,
+		StackdriverLabels: ctlConf.StackdriverLabels,
 	}
 
-	// Prometheus metrics
-	if ctlConf.PrometheusMetrics {
-		registry := prom.NewRegistry()
-		metricHandler, err := metrics.RegisterPrometheusExporter(registry)
-		if err != nil {
-			logger.WithError(err).Fatal("Could not register prometheus exporter")
-		}
-		server.Handle("/metrics", metricHandler)
-		health = healthcheck.NewMetricsHandler(registry, "agones")
-	} else {
-		health = healthcheck.NewHandler()
-	}
+	health, closer := metrics.SetupMetrics(metricsConf, server)
+	defer closer()
 
 	// If we are using Prometheus only exporter we can make reporting more often,
 	// every 1 seconds, if we are using Stackdriver we would use 60 seconds reporting period,

diff --git a/cmd/extensions/main.go b/cmd/extensions/main.go
@@ -23,16 +23,6 @@ import (
 	"strings"
 	"time"
 
-	"github.com/heptiolabs/healthcheck"
-	"github.com/pkg/errors"
-	prom "github.com/prometheus/client_golang/prometheus"
-	"github.com/sirupsen/logrus"
-	"github.com/spf13/pflag"
-	"github.com/spf13/viper"
-	"gopkg.in/natefinch/lumberjack.v2"
-	"k8s.io/client-go/informers"
-	"k8s.io/client-go/kubernetes"
-
 	"agones.dev/agones/pkg"
 	"agones.dev/agones/pkg/client/clientset/versioned"
 	"agones.dev/agones/pkg/client/informers/externalversions"
@@ -49,6 +39,14 @@ import (
 	"agones.dev/agones/pkg/util/runtime"
 	"agones.dev/agones/pkg/util/signals"
 	"agones.dev/agones/pkg/util/webhooks"
+	"github.com/heptiolabs/healthcheck"
+	"github.com/pkg/errors"
+	"github.com/sirupsen/logrus"
+	"github.com/spf13/pflag"
+	"github.com/spf13/viper"
+	"gopkg.in/natefinch/lumberjack.v2"
+	"k8s.io/client-go/informers"
+	"k8s.io/client-go/kubernetes"
 )
 
 const (
@@ -158,29 +156,15 @@ func main() {
 	}
 	var health healthcheck.Handler
 
-	// Stackdriver metrics
-	if ctlConf.Stackdriver {
-		sd, err := metrics.RegisterStackdriverExporter(ctlConf.GCPProjectID, ctlConf.StackdriverLabels)
-		if err != nil {
-			logger.WithError(err).Fatal("Could not register stackdriver exporter")
-		}
-		// It is imperative to invoke flush before your main function exits
-		defer sd.Flush()
+	metricsConf := metrics.Config{
+		Stackdriver:       ctlConf.Stackdriver,
+		PrometheusMetrics: ctlConf.PrometheusMetrics,
+		GCPProjectID:      ctlConf.GCPProjectID,
+		StackdriverLabels: ctlConf.StackdriverLabels,
 	}
 
-	// Prometheus metrics
-	if ctlConf.PrometheusMetrics {
-		registry := prom.NewRegistry()
-		metricHandler, err := metrics.RegisterPrometheusExporter(registry)
-		if err != nil {
-			logger.WithError(err).Fatal("Could not register prometheus exporter in extensions")
-		}
-		server.Handle("/metrics", metricHandler)
-		health = healthcheck.NewMetricsHandler(registry, "agones")
-	} else {
-		logger.Info("Not registaring prometheus metrics")
-		health = healthcheck.NewHandler()
-	}
+	health, closer := metrics.SetupMetrics(metricsConf, server)
+	defer closer()
 
 	podReady = true
 	health.AddReadinessCheck("agones-extensions", func() error {

diff --git a/pkg/metrics/exporter.go b/pkg/metrics/exporter.go
@@ -19,16 +19,26 @@ import (
 	"os"
 	"time"
 
+	"agones.dev/agones/pkg/util/httpserver"
 	"cloud.google.com/go/compute/metadata"
 	"contrib.go.opencensus.io/exporter/prometheus"
 	"contrib.go.opencensus.io/exporter/stackdriver"
+	"github.com/heptiolabs/healthcheck"
 	"github.com/pkg/errors"
 	prom "github.com/prometheus/client_golang/prometheus"
 	"github.com/prometheus/client_golang/prometheus/collectors"
 	"go.opencensus.io/stats/view"
 	"google.golang.org/genproto/googleapis/api/monitoredres"
 )
 
+// Config holds configuration for metrics reporting
+type Config struct {
+	GCPProjectID      string
+	StackdriverLabels string
+	Stackdriver       bool
+	PrometheusMetrics bool
+}
+
 // RegisterPrometheusExporter register a prometheus exporter to OpenCensus with a given prometheus metric registry.
 // It will automatically add go runtime and process metrics using default prometheus collectors.
 // The function return an http.handler that you can use to expose the prometheus endpoint.
@@ -119,3 +129,33 @@ func getMonitoredResource(projectID string) (*monitoredres.MonitoredResource, er
 		},
 	}, nil
 }
+
+// SetupMetrics initializes metrics reporting with the provided configuration
+func SetupMetrics(conf Config, server *httpserver.Server) (healthcheck.Handler, func()) {
+	var health healthcheck.Handler
+	var closer = func() {}
+
+	// Stackriver Metrics
+	if conf.Stackdriver {
+		sd, err := RegisterStackdriverExporter(conf.GCPProjectID, conf.StackdriverLabels)
+		if err != nil {
+			logger.WithError(err).Fatal("Could not register Stackdriver exporter")
+		}
+		closer = func() { sd.Flush() }
+	}
+
+	// Prometheus Metrics
+	if conf.PrometheusMetrics {
+		registry := prom.NewRegistry()
+		metricHandler, err := RegisterPrometheusExporter(registry)
+		if err != nil {
+			logger.WithError(err).Fatal("Could not register Prometheus exporter")
+		}
+		server.Handle("/metrics", metricHandler)
+		health = healthcheck.NewMetricsHandler(registry, "agones")
+	} else {
+		health = healthcheck.NewHandler()
+	}
+
+	return health, closer
+}