Metrics package now follows new service interface & registration (#11787)

* refactoring: metrics package now follows new service interface & registration

* fix: minor fix, make sure metrics service is imported, by grafana-server

pkg/cmd/grafana-server/server.go

@@ -24,7 +24,6 @@ import (
 	"github.com/grafana/grafana/pkg/api"
 	"github.com/grafana/grafana/pkg/log"
 	"github.com/grafana/grafana/pkg/login"
-	"github.com/grafana/grafana/pkg/metrics"
 	"github.com/grafana/grafana/pkg/services/sqlstore"
 	"github.com/grafana/grafana/pkg/setting"
 
@@ -33,6 +32,7 @@ import (
 
 	// self registering services
 	_ "github.com/grafana/grafana/pkg/extensions"
+	_ "github.com/grafana/grafana/pkg/metrics"
 	_ "github.com/grafana/grafana/pkg/plugins"
 	_ "github.com/grafana/grafana/pkg/services/alerting"
 	_ "github.com/grafana/grafana/pkg/services/cleanup"
@@ -72,7 +72,6 @@ func (g *GrafanaServerImpl) Start() error {
 	sqlstore.NewEngine() // TODO: this should return an error
 	sqlstore.EnsureAdminUser()
 
-	metrics.Init(g.cfg.Raw)
 	login.Init()
 	social.NewOAuthService()
 

pkg/metrics/init.go

@@ -1,38 +0,0 @@
-package metrics
-
-import (
-	"context"
-
-	ini "gopkg.in/ini.v1"
-
-	"github.com/grafana/grafana/pkg/log"
-	"github.com/grafana/grafana/pkg/metrics/graphitebridge"
-)
-
-var metricsLogger log.Logger = log.New("metrics")
-
-type logWrapper struct {
-	logger log.Logger
-}
-
-func (lw *logWrapper) Println(v ...interface{}) {
-	lw.logger.Info("graphite metric bridge", v...)
-}
-
-func Init(file *ini.File) {
-	cfg := ReadSettings(file)
-	internalInit(cfg)
-}
-
-func internalInit(settings *MetricSettings) {
-	initMetricVars(settings)
-
-	if settings.GraphiteBridgeConfig != nil {
-		bridge, err := graphitebridge.NewBridge(settings.GraphiteBridgeConfig)
-		if err != nil {
-			metricsLogger.Error("failed to create graphite bridge", "error", err)
-		} else {
-			go bridge.Run(context.Background())
-		}
-	}
-}

pkg/metrics/metrics.go

@@ -279,7 +279,7 @@ func init() {
 	}, []string{"version"})
 }
 
-func initMetricVars(settings *MetricSettings) {
+func initMetricVars() {
 	prometheus.MustRegister(
 		M_Instance_Start,
 		M_Page_Status,
@@ -316,28 +316,6 @@ func initMetricVars(settings *MetricSettings) {
 		M_StatTotal_Playlists,
 		M_Grafana_Version)
 
-	go instrumentationLoop(settings)
-}
-
-func instrumentationLoop(settings *MetricSettings) chan struct{} {
-	M_Instance_Start.Inc()
-
-	// set the total stats gauges before we publishing metrics
-	updateTotalStats()
-
-	onceEveryDayTick := time.NewTicker(time.Hour * 24)
-	everyMinuteTicker := time.NewTicker(time.Minute)
-	defer onceEveryDayTick.Stop()
-	defer everyMinuteTicker.Stop()
-
-	for {
-		select {
-		case <-onceEveryDayTick.C:
-			sendUsageStats()
-		case <-everyMinuteTicker.C:
-			updateTotalStats()
-		}
-	}
 }
 
 func updateTotalStats() {

pkg/metrics/service.go

@@ -0,0 +1,71 @@
+package metrics
+
+import (
+	"context"
+	"time"
+
+	"github.com/grafana/grafana/pkg/log"
+	"github.com/grafana/grafana/pkg/metrics/graphitebridge"
+	"github.com/grafana/grafana/pkg/registry"
+	"github.com/grafana/grafana/pkg/setting"
+)
+
+var metricsLogger log.Logger = log.New("metrics")
+
+type logWrapper struct {
+	logger log.Logger
+}
+
+func (lw *logWrapper) Println(v ...interface{}) {
+	lw.logger.Info("graphite metric bridge", v...)
+}
+
+func init() {
+	registry.RegisterService(&InternalMetricsService{})
+	initMetricVars()
+}
+
+type InternalMetricsService struct {
+	Cfg *setting.Cfg `inject:""`
+
+	enabled         bool
+	intervalSeconds int64
+	graphiteCfg     *graphitebridge.Config
+}
+
+func (im *InternalMetricsService) Init() error {
+	return im.readSettings()
+}
+
+func (im *InternalMetricsService) Run(ctx context.Context) error {
+	// Start Graphite Bridge
+	if im.graphiteCfg != nil {
+		bridge, err := graphitebridge.NewBridge(im.graphiteCfg)
+		if err != nil {
+			metricsLogger.Error("failed to create graphite bridge", "error", err)
+		} else {
+			go bridge.Run(ctx)
+		}
+	}
+
+	M_Instance_Start.Inc()
+
+	// set the total stats gauges before we publishing metrics
+	updateTotalStats()
+
+	onceEveryDayTick := time.NewTicker(time.Hour * 24)
+	everyMinuteTicker := time.NewTicker(time.Minute)
+	defer onceEveryDayTick.Stop()
+	defer everyMinuteTicker.Stop()
+
+	for {
+		select {
+		case <-onceEveryDayTick.C:
+			sendUsageStats()
+		case <-everyMinuteTicker.C:
+			updateTotalStats()
+		case <-ctx.Done():
+			return ctx.Err()
+		}
+	}
+}

pkg/metrics/settings.go

@@ -1,67 +1,53 @@
 package metrics
 
 import (
+	"fmt"
 	"strings"
 	"time"
 
 	"github.com/grafana/grafana/pkg/metrics/graphitebridge"
 	"github.com/grafana/grafana/pkg/setting"
 	"github.com/prometheus/client_golang/prometheus"
-	ini "gopkg.in/ini.v1"
 )
 
-type MetricSettings struct {
-	Enabled              bool
-	IntervalSeconds      int64
-	GraphiteBridgeConfig *graphitebridge.Config
-}
-
-func ReadSettings(file *ini.File) *MetricSettings {
-	var settings = &MetricSettings{
-		Enabled: false,
+func (im *InternalMetricsService) readSettings() error {
+	var section, err = im.Cfg.Raw.GetSection("metrics")
+	if err != nil {
+		return fmt.Errorf("Unable to find metrics config section %v", err)
 	}
 
-	var section, err = file.GetSection("metrics")
-	if err != nil {
-		metricsLogger.Crit("Unable to find metrics config section", "error", err)
+	im.enabled = section.Key("enabled").MustBool(false)
+	im.intervalSeconds = section.Key("interval_seconds").MustInt64(10)
+
+	if !im.enabled {
 		return nil
 	}
 
-	settings.Enabled = section.Key("enabled").MustBool(false)
-	settings.IntervalSeconds = section.Key("interval_seconds").MustInt64(10)
-
-	if !settings.Enabled {
-		return settings
+	if err := im.parseGraphiteSettings(); err != nil {
+		return fmt.Errorf("Unable to parse metrics graphite section, %v", err)
 	}
 
-	cfg, err := parseGraphiteSettings(settings, file)
-	if err != nil {
-		metricsLogger.Crit("Unable to parse metrics graphite section", "error", err)
-		return nil
-	}
-
-	settings.GraphiteBridgeConfig = cfg
-
-	return settings
+	return nil
 }
 
-func parseGraphiteSettings(settings *MetricSettings, file *ini.File) (*graphitebridge.Config, error) {
-	graphiteSection, err := setting.Raw.GetSection("metrics.graphite")
+func (im *InternalMetricsService) parseGraphiteSettings() error {
+	graphiteSection, err := im.Cfg.Raw.GetSection("metrics.graphite")
+
 	if err != nil {
-		return nil, nil
+		return nil
 	}
 
 	address := graphiteSection.Key("address").String()
 	if address == "" {
-		return nil, nil
+		return nil
 	}
 
-	cfg := &graphitebridge.Config{
+	bridgeCfg := &graphitebridge.Config{
 		URL:             address,
 		Prefix:          graphiteSection.Key("prefix").MustString("prod.grafana.%(instance_name)s"),
 		CountersAsDelta: true,
 		Gatherer:        prometheus.DefaultGatherer,
-		Interval:        time.Duration(settings.IntervalSeconds) * time.Second,
+		Interval:        time.Duration(im.intervalSeconds) * time.Second,
 		Timeout:         10 * time.Second,
 		Logger:          &logWrapper{logger: metricsLogger},
 		ErrorHandling:   graphitebridge.ContinueOnError,
@@ -74,6 +60,8 @@ func parseGraphiteSettings(settings *MetricSettings, file *ini.File) (*graphiteb
 		prefix = "prod.grafana.%(instance_name)s."
 	}
 
-	cfg.Prefix = strings.Replace(prefix, "%(instance_name)s", safeInstanceName, -1)
-	return cfg, nil
+	bridgeCfg.Prefix = strings.Replace(prefix, "%(instance_name)s", safeInstanceName, -1)
+
+	im.graphiteCfg = bridgeCfg
+	return nil
 }