From 14bb7832af9a43f58cc0bf53eb7e4abe9bf44085 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Wed, 2 May 2018 19:54:07 +0200 Subject: [PATCH] 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 | 3 +- pkg/metrics/init.go | 38 ----------------- pkg/metrics/metrics.go | 24 +---------- pkg/metrics/service.go | 71 ++++++++++++++++++++++++++++++++ pkg/metrics/settings.go | 56 ++++++++++--------------- 5 files changed, 95 insertions(+), 97 deletions(-) delete mode 100644 pkg/metrics/init.go create mode 100644 pkg/metrics/service.go diff --git a/pkg/cmd/grafana-server/server.go b/pkg/cmd/grafana-server/server.go index 30bb0b2003a..f20195b563a 100644 --- a/pkg/cmd/grafana-server/server.go +++ b/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() diff --git a/pkg/metrics/init.go b/pkg/metrics/init.go deleted file mode 100644 index 833b148d319..00000000000 --- a/pkg/metrics/init.go +++ /dev/null @@ -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()) - } - } -} diff --git a/pkg/metrics/metrics.go b/pkg/metrics/metrics.go index e3640378f7e..83505826910 100644 --- a/pkg/metrics/metrics.go +++ b/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() { diff --git a/pkg/metrics/service.go b/pkg/metrics/service.go new file mode 100644 index 00000000000..ec38e0acfec --- /dev/null +++ b/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() + } + } +} diff --git a/pkg/metrics/settings.go b/pkg/metrics/settings.go index c21e7279b7e..58b84a7192f 100644 --- a/pkg/metrics/settings.go +++ b/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, - } - - var section, err = file.GetSection("metrics") +func (im *InternalMetricsService) readSettings() error { + var section, err = im.Cfg.Raw.GetSection("metrics") if err != nil { - metricsLogger.Crit("Unable to find metrics config section", "error", err) - return nil + return fmt.Errorf("Unable to find metrics config section %v", err) } - settings.Enabled = section.Key("enabled").MustBool(false) - settings.IntervalSeconds = section.Key("interval_seconds").MustInt64(10) - - if !settings.Enabled { - return settings - } + im.enabled = section.Key("enabled").MustBool(false) + im.intervalSeconds = section.Key("interval_seconds").MustInt64(10) - cfg, err := parseGraphiteSettings(settings, file) - if err != nil { - metricsLogger.Crit("Unable to parse metrics graphite section", "error", err) + if !im.enabled { return nil } - settings.GraphiteBridgeConfig = cfg + if err := im.parseGraphiteSettings(); err != nil { + return fmt.Errorf("Unable to parse metrics graphite section, %v", err) + } - 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 }