diff --git a/pkg/api/http_server.go b/pkg/api/http_server.go index 6a97f789415..194d2f20a6d 100644 --- a/pkg/api/http_server.go +++ b/pkg/api/http_server.go @@ -205,7 +205,6 @@ type HTTPServer struct { authnService authn.Service starApi *starApi.API promRegister prometheus.Registerer - promGatherer prometheus.Gatherer clientConfigProvider grafanaapiserver.DirectRestConfigProvider namespacer request.NamespaceMapper anonService anonymous.Service @@ -249,7 +248,7 @@ func ProvideHTTPServer(opts ServerOptions, cfg *setting.Cfg, routeRegister routi loginAttemptService loginAttempt.Service, orgService org.Service, teamService team.Service, accesscontrolService accesscontrol.Service, navTreeService navtree.Service, annotationRepo annotations.Repository, tagService tag.Service, searchv2HTTPService searchV2.SearchHTTPService, oauthTokenService oauthtoken.OAuthTokenService, - statsService stats.Service, authnService authn.Service, pluginsCDNService *pluginscdn.Service, promGatherer prometheus.Gatherer, + statsService stats.Service, authnService authn.Service, pluginsCDNService *pluginscdn.Service, starApi *starApi.API, promRegister prometheus.Registerer, clientConfigProvider grafanaapiserver.DirectRestConfigProvider, anonService anonymous.Service, ) (*HTTPServer, error) { web.Env = cfg.Env @@ -349,7 +348,6 @@ func ProvideHTTPServer(opts ServerOptions, cfg *setting.Cfg, routeRegister routi pluginsCDNService: pluginsCDNService, starApi: starApi, promRegister: promRegister, - promGatherer: promGatherer, clientConfigProvider: clientConfigProvider, namespacer: request.GetNamespaceMapper(cfg), anonService: anonService, @@ -651,7 +649,7 @@ func (hs *HTTPServer) metricsEndpoint(ctx *web.Context) { } promhttp. - HandlerFor(hs.promGatherer, promhttp.HandlerOpts{EnableOpenMetrics: true}). + HandlerFor(prometheus.DefaultGatherer, promhttp.HandlerOpts{EnableOpenMetrics: true}). ServeHTTP(ctx.Resp, ctx.Req) } diff --git a/pkg/cmd/grafana-server/commands/buildinfo.go b/pkg/cmd/grafana-server/commands/buildinfo.go index d37ef758b55..96bf7185cdd 100644 --- a/pkg/cmd/grafana-server/commands/buildinfo.go +++ b/pkg/cmd/grafana-server/commands/buildinfo.go @@ -4,14 +4,12 @@ import ( "strconv" "time" - "github.com/prometheus/client_golang/prometheus" - "github.com/grafana/grafana/pkg/extensions" "github.com/grafana/grafana/pkg/infra/metrics" "github.com/grafana/grafana/pkg/setting" ) -func setBuildInfo(reg prometheus.Registerer, opts ServerOptions) { +func setBuildInfo(opts ServerOptions) { buildstampInt64, err := strconv.ParseInt(opts.BuildStamp, 10, 64) if err != nil || buildstampInt64 == 0 { buildstampInt64 = time.Now().Unix() @@ -24,5 +22,5 @@ func setBuildInfo(reg prometheus.Registerer, opts ServerOptions) { setting.IsEnterprise = extensions.IsEnterprise setting.Packaging = validPackaging(Packaging) - metrics.SetBuildInformation(reg, opts.Version, opts.Commit, opts.BuildBranch, buildstampInt64) + metrics.SetBuildInformation(opts.Version, opts.Commit, opts.BuildBranch, buildstampInt64) } diff --git a/pkg/cmd/grafana-server/commands/cli.go b/pkg/cmd/grafana-server/commands/cli.go index 5654df74d4d..37ff71f7c76 100644 --- a/pkg/cmd/grafana-server/commands/cli.go +++ b/pkg/cmd/grafana-server/commands/cli.go @@ -16,7 +16,6 @@ import ( "github.com/grafana/grafana/pkg/api" gcli "github.com/grafana/grafana/pkg/cmd/grafana-cli/commands" "github.com/grafana/grafana/pkg/infra/log" - "github.com/grafana/grafana/pkg/infra/metrics" "github.com/grafana/grafana/pkg/infra/process" "github.com/grafana/grafana/pkg/server" _ "github.com/grafana/grafana/pkg/services/alerting/conditions" @@ -98,6 +97,9 @@ func RunServer(opts ServerOptions) error { } }() + setBuildInfo(opts) + checkPrivileges() + configOptions := strings.Split(ConfigOverrides, " ") cfg, err := setting.NewCfgFromArgs(setting.CommandLineArgs{ Config: ConfigFile, @@ -109,9 +111,6 @@ func RunServer(opts ServerOptions) error { return err } - setBuildInfo(metrics.ProvideRegisterer(cfg), opts) - checkPrivileges() - s, err := server.Initialize( cfg, server.Options{ diff --git a/pkg/cmd/grafana-server/commands/target.go b/pkg/cmd/grafana-server/commands/target.go index 3b443b8f526..5843927a4c1 100644 --- a/pkg/cmd/grafana-server/commands/target.go +++ b/pkg/cmd/grafana-server/commands/target.go @@ -11,7 +11,6 @@ import ( "github.com/grafana/grafana/pkg/api" "github.com/grafana/grafana/pkg/infra/log" - "github.com/grafana/grafana/pkg/infra/metrics" "github.com/grafana/grafana/pkg/server" "github.com/grafana/grafana/pkg/setting" ) @@ -75,6 +74,9 @@ func RunTargetServer(opts ServerOptions) error { } }() + setBuildInfo(opts) + checkPrivileges() + configOptions := strings.Split(ConfigOverrides, " ") cfg, err := setting.NewCfgFromArgs(setting.CommandLineArgs{ Config: ConfigFile, @@ -86,9 +88,6 @@ func RunTargetServer(opts ServerOptions) error { return err } - setBuildInfo(metrics.ProvideRegisterer(cfg), opts) - checkPrivileges() - s, err := server.InitializeModuleServer( cfg, server.Options{ diff --git a/pkg/infra/metrics/frontendmetrics.go b/pkg/infra/metrics/frontendmetrics.go index 00e4bff7939..399b084e0a4 100644 --- a/pkg/infra/metrics/frontendmetrics.go +++ b/pkg/infra/metrics/frontendmetrics.go @@ -19,7 +19,7 @@ type FrontendMetricsRecorder func(event FrontendMetricEvent) // FrontendMetrics contains all the valid frontend metrics and a handler function for recording events var FrontendMetrics map[string]FrontendMetricsRecorder = map[string]FrontendMetricsRecorder{} -func registerFrontendHistogram(reg prometheus.Registerer, name string, help string) { +func registerFrontendHistogram(name string, help string) { defBuckets := []float64{.1, .25, .5, 1, 1.5, 2, 5, 10, 20, 40} histogram := prometheus.NewHistogram(prometheus.HistogramOpts{ @@ -33,14 +33,14 @@ func registerFrontendHistogram(reg prometheus.Registerer, name string, help stri histogram.Observe(event.Value) } - reg.MustRegister(histogram) + prometheus.MustRegister(histogram) } -func initFrontendMetrics(r prometheus.Registerer) { - registerFrontendHistogram(r, "frontend_boot_load_time_seconds", "Frontend boot time measurement") - registerFrontendHistogram(r, "frontend_boot_first_paint_time_seconds", "Frontend boot first paint") - registerFrontendHistogram(r, "frontend_boot_first_contentful_paint_time_seconds", "Frontend boot first contentful paint") - registerFrontendHistogram(r, "frontend_boot_js_done_time_seconds", "Frontend boot initial js load") - registerFrontendHistogram(r, "frontend_boot_css_time_seconds", "Frontend boot initial css load") - registerFrontendHistogram(r, "frontend_plugins_preload_ms", "Frontend preload plugin time measurement") +func initFrontendMetrics() { + registerFrontendHistogram("frontend_boot_load_time_seconds", "Frontend boot time measurement") + registerFrontendHistogram("frontend_boot_first_paint_time_seconds", "Frontend boot first paint") + registerFrontendHistogram("frontend_boot_first_contentful_paint_time_seconds", "Frontend boot first contentful paint") + registerFrontendHistogram("frontend_boot_js_done_time_seconds", "Frontend boot initial js load") + registerFrontendHistogram("frontend_boot_css_time_seconds", "Frontend boot initial css load") + registerFrontendHistogram("frontend_plugins_preload_ms", "Frontend preload plugin time measurement") } diff --git a/pkg/infra/metrics/metrics.go b/pkg/infra/metrics/metrics.go index 4ed18149f8b..6c0d1ad706d 100644 --- a/pkg/infra/metrics/metrics.go +++ b/pkg/infra/metrics/metrics.go @@ -613,7 +613,7 @@ func init() { } // SetBuildInformation sets the build information for this binary -func SetBuildInformation(reg prometheus.Registerer, version, revision, branch string, buildTimestamp int64) { +func SetBuildInformation(version, revision, branch string, buildTimestamp int64) { edition := "oss" if setting.IsEnterprise { edition = "enterprise" @@ -631,7 +631,7 @@ func SetBuildInformation(reg prometheus.Registerer, version, revision, branch st Namespace: ExporterName, }, []string{"version", "revision", "branch", "goversion", "edition"}) - reg.MustRegister(grafanaBuildVersion, grafanaBuildTimestamp) + prometheus.MustRegister(grafanaBuildVersion, grafanaBuildTimestamp) grafanaBuildVersion.WithLabelValues(version, revision, branch, runtime.Version(), edition).Set(1) grafanaBuildTimestamp.WithLabelValues(version, revision, branch, runtime.Version(), edition).Set(float64(buildTimestamp)) @@ -639,7 +639,7 @@ func SetBuildInformation(reg prometheus.Registerer, version, revision, branch st // SetEnvironmentInformation exposes environment values provided by the operators as an `_info` metric. // If there are no environment metrics labels configured, this metric will not be exposed. -func SetEnvironmentInformation(reg prometheus.Registerer, labels map[string]string) error { +func SetEnvironmentInformation(labels map[string]string) error { if len(labels) == 0 { return nil } @@ -651,7 +651,7 @@ func SetEnvironmentInformation(reg prometheus.Registerer, labels map[string]stri ConstLabels: labels, }) - reg.MustRegister(grafanaEnvironmentInfo) + prometheus.MustRegister(grafanaEnvironmentInfo) grafanaEnvironmentInfo.Set(1) return nil @@ -661,8 +661,8 @@ func SetPluginBuildInformation(pluginID, pluginType, version, signatureStatus st grafanaPluginBuildInfoDesc.WithLabelValues(pluginID, pluginType, version, signatureStatus).Set(1) } -func initMetricVars(reg prometheus.Registerer) { - reg.MustRegister( +func initMetricVars() { + prometheus.MustRegister( MInstanceStart, MPageStatus, MApiStatus, diff --git a/pkg/infra/metrics/service.go b/pkg/infra/metrics/service.go index 13767508a48..2b24c2a665d 100644 --- a/pkg/infra/metrics/service.go +++ b/pkg/infra/metrics/service.go @@ -2,17 +2,11 @@ package metrics import ( "context" - "errors" - "regexp" - - "github.com/prometheus/client_golang/prometheus" - dto "github.com/prometheus/client_model/go" - "k8s.io/component-base/metrics/legacyregistry" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/metrics/graphitebridge" - "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/setting" + "github.com/prometheus/client_golang/prometheus" ) var metricsLogger log.Logger = log.New("metrics") @@ -25,10 +19,12 @@ func (lw *logWrapper) Println(v ...any) { lw.logger.Info("graphite metric bridge", v...) } -func ProvideService(cfg *setting.Cfg, reg prometheus.Registerer) (*InternalMetricsService, error) { - initMetricVars(reg) - initFrontendMetrics(reg) +func init() { + initMetricVars() + initFrontendMetrics() +} +func ProvideService(cfg *setting.Cfg) (*InternalMetricsService, error) { s := &InternalMetricsService{ Cfg: cfg, } @@ -59,66 +55,5 @@ func (im *InternalMetricsService) Run(ctx context.Context) error { return ctx.Err() } -func ProvideRegisterer(cfg *setting.Cfg) prometheus.Registerer { - if cfg.IsFeatureToggleEnabled(featuremgmt.FlagGrafanaAPIServer) { - return legacyregistry.Registerer() - } - return prometheus.DefaultRegisterer -} - -func ProvideGatherer(cfg *setting.Cfg) prometheus.Gatherer { - if cfg.IsFeatureToggleEnabled(featuremgmt.FlagGrafanaAPIServer) { - return newAddPrefixWrapper(legacyregistry.DefaultGatherer) - } - return prometheus.DefaultGatherer -} - -func ProvideRegistererForTest() prometheus.Registerer { - return prometheus.NewRegistry() -} - -func ProvideGathererForTest(reg prometheus.Registerer) prometheus.Gatherer { - // the registerer provided by ProvideRegistererForTest - // is a *prometheus.Registry, so it also implements prometheus.Gatherer - return reg.(*prometheus.Registry) -} - -var _ prometheus.Gatherer = (*addPrefixWrapper)(nil) - -// addPrefixWrapper wraps a prometheus.Gatherer, and ensures that all metric names are prefixed with `grafana_`. -// metrics with the prefix `grafana_` or `go_` are not modified. -type addPrefixWrapper struct { - orig prometheus.Gatherer - reg *regexp.Regexp -} - -func newAddPrefixWrapper(orig prometheus.Gatherer) *addPrefixWrapper { - return &addPrefixWrapper{ - orig: orig, - reg: regexp.MustCompile("^((?:grafana_|go_).*)"), - } -} - -func (g *addPrefixWrapper) Gather() ([]*dto.MetricFamily, error) { - mf, err := g.orig.Gather() - if err != nil { - return nil, err - } - - names := make(map[string]struct{}) - - for i := 0; i < len(mf); i++ { - m := mf[i] - if m.Name != nil && !g.reg.MatchString(*m.Name) { - *m.Name = "grafana_" + *m.Name - // since we are modifying the name, we need to check for duplicates in the gatherer - if _, exists := names[*m.Name]; exists { - return nil, errors.New("duplicate metric name: " + *m.Name) - } - } - // keep track of names to detect duplicates - names[*m.Name] = struct{}{} - } - - return mf, nil -} +func ProvideRegisterer() prometheus.Registerer { return prometheus.DefaultRegisterer } +func ProvideRegistererForTest() prometheus.Registerer { return prometheus.NewRegistry() } diff --git a/pkg/infra/metrics/service_test.go b/pkg/infra/metrics/service_test.go deleted file mode 100644 index b5c9ea92f50..00000000000 --- a/pkg/infra/metrics/service_test.go +++ /dev/null @@ -1,61 +0,0 @@ -package metrics - -import ( - "testing" - - dto "github.com/prometheus/client_model/go" - "github.com/stretchr/testify/require" -) - -func TestK8sGathererWrapper_Gather(t *testing.T) { - orig := &mockGatherer{} - g := newAddPrefixWrapper(orig) - - t.Run("metrics with grafana and go prefix are not modified", func(t *testing.T) { - originalMF := []*dto.MetricFamily{ - {Name: strptr("grafana_metric1")}, - {Name: strptr("metric2")}, - {Name: strptr("go_metric1")}, - } - - orig.GatherFunc = func() ([]*dto.MetricFamily, error) { - return originalMF, nil - } - - expectedMF := []*dto.MetricFamily{ - {Name: strptr("grafana_metric1")}, - {Name: strptr("grafana_metric2")}, - {Name: strptr("go_metric1")}, - } - - mf, err := g.Gather() - require.NoError(t, err) - require.Equal(t, expectedMF, mf) - }) - - t.Run("duplicate metrics result in an error", func(t *testing.T) { - originalMF := []*dto.MetricFamily{ - {Name: strptr("grafana_metric1")}, - {Name: strptr("metric1")}, - } - - orig.GatherFunc = func() ([]*dto.MetricFamily, error) { - return originalMF, nil - } - - _, err := g.Gather() - require.Error(t, err) - }) -} - -type mockGatherer struct { - GatherFunc func() ([]*dto.MetricFamily, error) -} - -func (m *mockGatherer) Gather() ([]*dto.MetricFamily, error) { - return m.GatherFunc() -} - -func strptr(s string) *string { - return &s -} diff --git a/pkg/infra/metrics/wireset.go b/pkg/infra/metrics/wireset.go deleted file mode 100644 index 489c995badc..00000000000 --- a/pkg/infra/metrics/wireset.go +++ /dev/null @@ -1,17 +0,0 @@ -package metrics - -import ( - "github.com/google/wire" -) - -var WireSet = wire.NewSet( - ProvideService, - ProvideRegisterer, - ProvideGatherer, -) - -var WireSetForTest = wire.NewSet( - ProvideService, - ProvideRegistererForTest, - ProvideGathererForTest, -) diff --git a/pkg/server/server.go b/pkg/server/server.go index ca1263d9e4d..a9de82ef902 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -13,8 +13,6 @@ import ( "golang.org/x/sync/errgroup" - "github.com/prometheus/client_golang/prometheus" - "github.com/grafana/grafana/pkg/api" _ "github.com/grafana/grafana/pkg/extensions" "github.com/grafana/grafana/pkg/infra/log" @@ -40,10 +38,9 @@ type Options struct { func New(opts Options, cfg *setting.Cfg, httpServer *api.HTTPServer, roleRegistry accesscontrol.RoleRegistry, provisioningService provisioning.ProvisioningService, backgroundServiceProvider registry.BackgroundServiceRegistry, usageStatsProvidersRegistry registry.UsageStatsProvidersRegistry, statsCollectorService *statscollector.Service, - promReg prometheus.Registerer, ) (*Server, error) { statsCollectorService.RegisterProviders(usageStatsProvidersRegistry.GetServices()) - s, err := newServer(opts, cfg, httpServer, roleRegistry, provisioningService, backgroundServiceProvider, promReg) + s, err := newServer(opts, cfg, httpServer, roleRegistry, provisioningService, backgroundServiceProvider) if err != nil { return nil, err } @@ -57,13 +54,11 @@ func New(opts Options, cfg *setting.Cfg, httpServer *api.HTTPServer, roleRegistr func newServer(opts Options, cfg *setting.Cfg, httpServer *api.HTTPServer, roleRegistry accesscontrol.RoleRegistry, provisioningService provisioning.ProvisioningService, backgroundServiceProvider registry.BackgroundServiceRegistry, - promReg prometheus.Registerer, ) (*Server, error) { rootCtx, shutdownFn := context.WithCancel(context.Background()) childRoutines, childCtx := errgroup.WithContext(rootCtx) s := &Server{ - promReg: promReg, context: childCtx, childRoutines: childRoutines, HTTPServer: httpServer, @@ -106,7 +101,6 @@ type Server struct { HTTPServer *api.HTTPServer roleRegistry accesscontrol.RoleRegistry provisioningService provisioning.ProvisioningService - promReg prometheus.Registerer } // Init initializes the server and its services. @@ -123,7 +117,7 @@ func (s *Server) Init() error { return err } - if err := metrics.SetEnvironmentInformation(s.promReg, s.cfg.MetricsGrafanaEnvironmentInfo); err != nil { + if err := metrics.SetEnvironmentInformation(s.cfg.MetricsGrafanaEnvironmentInfo); err != nil { return err } diff --git a/pkg/server/server_test.go b/pkg/server/server_test.go index 5e1d57b2fa3..1c45a5a82f2 100644 --- a/pkg/server/server_test.go +++ b/pkg/server/server_test.go @@ -7,7 +7,6 @@ import ( "testing" "time" - "github.com/prometheus/client_golang/prometheus" "github.com/stretchr/testify/require" "github.com/grafana/grafana/pkg/registry" @@ -49,7 +48,7 @@ func (s *testService) IsDisabled() bool { func testServer(t *testing.T, services ...registry.BackgroundService) *Server { t.Helper() - s, err := newServer(Options{}, setting.NewCfg(), nil, &acimpl.Service{}, nil, backgroundsvcs.NewBackgroundServiceRegistry(services...), prometheus.NewRegistry()) + s, err := newServer(Options{}, setting.NewCfg(), nil, &acimpl.Service{}, nil, backgroundsvcs.NewBackgroundServiceRegistry(services...)) require.NoError(t, err) // Required to skip configuration initialization that causes // DI errors in this test. diff --git a/pkg/server/wire.go b/pkg/server/wire.go index 8d395ac879e..6a01d9b6283 100644 --- a/pkg/server/wire.go +++ b/pkg/server/wire.go @@ -253,6 +253,7 @@ var wireBasicSet = wire.NewSet( notifications.ProvideSmtpService, tracing.ProvideService, wire.Bind(new(tracing.Tracer), new(*tracing.TracingService)), + metrics.ProvideService, testdatasource.ProvideService, ldapapi.ProvideService, opentsdb.ProvideService, @@ -385,7 +386,7 @@ var wireBasicSet = wire.NewSet( var wireSet = wire.NewSet( wireBasicSet, - metrics.WireSet, + metrics.ProvideRegisterer, sqlstore.ProvideService, ngmetrics.ProvideService, wire.Bind(new(notifications.Service), new(*notifications.NotificationService)), @@ -400,7 +401,6 @@ var wireSet = wire.NewSet( var wireCLISet = wire.NewSet( NewRunner, wireBasicSet, - metrics.WireSet, sqlstore.ProvideService, ngmetrics.ProvideService, wire.Bind(new(notifications.Service), new(*notifications.NotificationService)), @@ -415,7 +415,7 @@ var wireCLISet = wire.NewSet( var wireTestSet = wire.NewSet( wireBasicSet, ProvideTestEnv, - metrics.WireSetForTest, + metrics.ProvideRegistererForTest, sqlstore.ProvideServiceForTests, ngmetrics.ProvideServiceForTest, notifications.MockNotificationService, diff --git a/pkg/server/wireexts_oss.go b/pkg/server/wireexts_oss.go index 5efd58cd50e..366cc425646 100644 --- a/pkg/server/wireexts_oss.go +++ b/pkg/server/wireexts_oss.go @@ -7,7 +7,6 @@ package server import ( "github.com/google/wire" - "github.com/grafana/grafana/pkg/infra/metrics" "github.com/grafana/grafana/pkg/plugins" "github.com/grafana/grafana/pkg/plugins/manager" "github.com/grafana/grafana/pkg/registry" @@ -124,7 +123,6 @@ var wireExtsTestSet = wire.NewSet( var wireExtsBaseCLISet = wire.NewSet( NewModuleRunner, - metrics.WireSet, featuremgmt.ProvideManagerService, featuremgmt.ProvideToggles, hooks.ProvideService,