Revert "move metrics server into platform service (#20683)" (#20726)

This reverts commit b18a42313b.

Co-authored-by: Mattermod <mattermod@users.noreply.github.com>

api4/apitestlib.go

@@ -322,8 +322,8 @@ func SetupWithStoreMock(tb testing.TB) *TestHelper {
 	return th
 }
 
-func SetupEnterpriseWithStoreMock(tb testing.TB, options ...app.Option) *TestHelper {
+func SetupEnterpriseWithStoreMock(tb testing.TB) *TestHelper {
-	th := setupTestHelper(testlib.GetMockStoreForSetupFunctions(), nil, true, false, nil, options)
+	th := setupTestHelper(testlib.GetMockStoreForSetupFunctions(), nil, true, false, nil, nil)
 	statusMock := mocks.StatusStore{}
 	statusMock.On("UpdateExpiredDNDStatuses").Return([]*model.Status{}, nil)
 	statusMock.On("Get", "user1").Return(&model.Status{UserId: "user1", Status: model.StatusOnline}, nil)

app/config.go

@@ -82,9 +82,9 @@ func (w *configWrapper) SaveConfig(newCfg *model.Config, sendConfigChangeCluster
 		if w.srv.Metrics != nil {
 			w.srv.Metrics.Register()
 		}
-		w.srv.platformService.RestartMetrics() // TODO: remove when this moved to the platform service
+		w.srv.SetupMetricsServer()
 	} else {
-		w.srv.platformService.ShutdownMetrics() // TODO: remove when this moved to the platform service
+		w.srv.StopMetricsServer()
 	}
 
 	if w.srv.Cluster != nil {

app/platform/config.go

@@ -3,28 +3,14 @@
 
 package platform
 
-import (
-	"errors"
-
-	"github.com/mattermost/mattermost-server/v6/config"
-	"github.com/mattermost/mattermost-server/v6/einterfaces"
-)
-
 // ServiceConfig is used to initialize the PlatformService.
 // The mandatory fields will be checked during the initialization of the service.
 type ServiceConfig struct {
 	// Mandatory fields
-	ConfigStore  *config.Store
-	StartMetrics bool // TODO: find an elegant way to start/stop metrics server by default
 	// Optional fields
-	Metrics einterfaces.MetricsInterface
-	Cluster einterfaces.ClusterInterface
 }
 
 func (c *ServiceConfig) validate() error {
 	// Mandatory fields need to be checked here
-	if c.ConfigStore == nil {
-		return errors.New("ConfigStore is required")
-	}
 	return nil
 }

app/platform/metrics.go

@@ -1,162 +0,0 @@
-// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
-// See LICENSE.txt for license information.
-
-package platform
-
-import (
-	"context"
-	"net"
-	"net/http"
-	"net/http/pprof"
-	"runtime"
-	"sync"
-	"text/template"
-	"time"
-
-	"github.com/gorilla/handlers"
-	"github.com/gorilla/mux"
-	"github.com/mattermost/mattermost-server/v6/einterfaces"
-	"github.com/mattermost/mattermost-server/v6/model"
-	"github.com/mattermost/mattermost-server/v6/shared/mlog"
-	"github.com/pkg/errors"
-)
-
-const TimeToWaitForConnectionsToCloseOnServerShutdown = time.Second
-
-type platformMetrics struct {
-	server *http.Server
-	router *mux.Router
-	lock   sync.Mutex
-
-	metricsImpl einterfaces.MetricsInterface
-
-	cfgFn func() *model.Config
-}
-
-func newPlatformMetrics(metricsImpl einterfaces.MetricsInterface, cfgFn func() *model.Config) *platformMetrics {
-	if !*cfgFn().MetricsSettings.Enable {
-		return nil
-	}
-
-	pm := &platformMetrics{
-		cfgFn: cfgFn,
-	}
-
-	pm.stopMetricsServer()
-
-	if err := pm.initMetricsRouter(); err != nil {
-		mlog.Error("Error initiating metrics router.", mlog.Err(err))
-	}
-
-	if metricsImpl != nil {
-		metricsImpl.Register()
-	}
-
-	pm.startMetricsServer()
-
-	return pm
-}
-
-func (pm *platformMetrics) stopMetricsServer() {
-	pm.lock.Lock()
-	defer pm.lock.Unlock()
-
-	if pm.server != nil {
-		ctx, cancel := context.WithTimeout(context.Background(), TimeToWaitForConnectionsToCloseOnServerShutdown)
-		defer cancel()
-
-		pm.server.Shutdown(ctx)
-		mlog.Info("Metrics and profiling server is stopping")
-	}
-}
-
-func (pm *platformMetrics) startMetricsServer() {
-	var notify chan struct{}
-	pm.lock.Lock()
-	defer func() {
-		if notify != nil {
-			<-notify
-		}
-		pm.lock.Unlock()
-	}()
-
-	l, err := net.Listen("tcp", *pm.cfgFn().MetricsSettings.ListenAddress)
-	if err != nil {
-		mlog.Error(err.Error())
-		return
-	}
-
-	notify = make(chan struct{})
-	pm.server = &http.Server{
-		Handler:      handlers.RecoveryHandler(handlers.PrintRecoveryStack(true))(pm.router),
-		ReadTimeout:  time.Duration(*pm.cfgFn().ServiceSettings.ReadTimeout) * time.Second,
-		WriteTimeout: time.Duration(*pm.cfgFn().ServiceSettings.WriteTimeout) * time.Second,
-	}
-
-	go func() {
-		close(notify)
-		if err := pm.server.Serve(l); err != nil && err != http.ErrServerClosed {
-			mlog.Critical(err.Error())
-		}
-	}()
-
-	mlog.Info("Metrics and profiling server is started", mlog.String("address", l.Addr().String()))
-}
-
-func (pm *platformMetrics) initMetricsRouter() error {
-	pm.router = mux.NewRouter()
-	runtime.SetBlockProfileRate(*pm.cfgFn().MetricsSettings.BlockProfileRate)
-
-	metricsPage := `
-			<html>
-				<body>{{if .}}
-					<div><a href="/metrics">Metrics</a></div>{{end}}
-					<div><a href="/debug/pprof/">Profiling Root</a></div>
-					<div><a href="/debug/pprof/cmdline">Profiling Command Line</a></div>
-					<div><a href="/debug/pprof/symbol">Profiling Symbols</a></div>
-					<div><a href="/debug/pprof/goroutine">Profiling Goroutines</a></div>
-					<div><a href="/debug/pprof/heap">Profiling Heap</a></div>
-					<div><a href="/debug/pprof/threadcreate">Profiling Threads</a></div>
-					<div><a href="/debug/pprof/block">Profiling Blocking</a></div>
-					<div><a href="/debug/pprof/trace">Profiling Execution Trace</a></div>
-					<div><a href="/debug/pprof/profile">Profiling CPU</a></div>
-				</body>
-			</html>
-		`
-	metricsPageTmpl, err := template.New("page").Parse(metricsPage)
-	if err != nil {
-		return errors.Wrap(err, "failed to create template")
-	}
-
-	rootHandler := func(w http.ResponseWriter, r *http.Request) {
-		metricsPageTmpl.Execute(w, pm.metricsImpl != nil)
-	}
-
-	pm.router.HandleFunc("/", rootHandler)
-	pm.router.StrictSlash(true)
-
-	pm.router.Handle("/debug", http.RedirectHandler("/", http.StatusMovedPermanently))
-	pm.router.HandleFunc("/debug/pprof/", pprof.Index)
-	pm.router.HandleFunc("/debug/pprof/cmdline", pprof.Cmdline)
-	pm.router.HandleFunc("/debug/pprof/profile", pprof.Profile)
-	pm.router.HandleFunc("/debug/pprof/symbol", pprof.Symbol)
-	pm.router.HandleFunc("/debug/pprof/trace", pprof.Trace)
-
-	// Manually add support for paths linked to by index page at /debug/pprof/
-	pm.router.Handle("/debug/pprof/goroutine", pprof.Handler("goroutine"))
-	pm.router.Handle("/debug/pprof/heap", pprof.Handler("heap"))
-	pm.router.Handle("/debug/pprof/threadcreate", pprof.Handler("threadcreate"))
-	pm.router.Handle("/debug/pprof/block", pprof.Handler("block"))
-
-	return nil
-}
-
-func (ps *PlatformService) HandleMetrics(route string, h http.Handler) {
-	if ps.metrics.router != nil {
-		ps.metrics.router.Handle(route, h)
-	}
-}
-
-func (ps *PlatformService) RestartMetrics() {
-	ps.metrics = newPlatformMetrics(ps.serviceConfig.Metrics, ps.serviceConfig.ConfigStore.Get)
-}

app/platform/service.go

@@ -3,42 +3,17 @@
 
 package platform
 
-import (
-	"github.com/mattermost/mattermost-server/v6/config"
-	"github.com/mattermost/mattermost-server/v6/einterfaces"
-)
-
 // PlatformService is the service for the platform related tasks. It is
 // responsible for non-entity related functionalities that are required
 // by a product such as database access, configuration access, licensing etc.
 type PlatformService struct {
-	serviceConfig ServiceConfig
-	configStore   *config.Store
-
-	metrics *platformMetrics
-
-	cluster einterfaces.ClusterInterface
 }
 
 // New creates a new PlatformService.
-func New(sc ServiceConfig) (*PlatformService, error) {
+func New(c ServiceConfig) (*PlatformService, error) {
-	if err := sc.validate(); err != nil {
+	if err := c.validate(); err != nil {
 		return nil, err
 	}
 
-	ps := &PlatformService{
+	return &PlatformService{}, nil
-		serviceConfig: sc,
-		configStore:   sc.ConfigStore,
-		cluster:       sc.Cluster,
-	}
-
-	ps.metrics = newPlatformMetrics(sc.Metrics, ps.configStore.Get)
-
-	return ps, nil
-}
-
-func (ps *PlatformService) ShutdownMetrics() {
-	if ps.metrics != nil {
-		ps.metrics.stopMetricsServer()
-	}
 }

app/server.go

@@ -9,8 +9,10 @@ import (
 	"crypto/tls"
 	"fmt"
 	"hash/maphash"
+	"html/template"
 	"net"
 	"net/http"
+	"net/http/pprof"
 	"net/url"
 	"os"
 	"os/exec"
@@ -25,6 +27,7 @@ import (
 
 	"github.com/getsentry/sentry-go"
 	sentryhttp "github.com/getsentry/sentry-go/http"
+	"github.com/gorilla/handlers"
 	"github.com/gorilla/mux"
 	"github.com/pkg/errors"
 	"github.com/rs/cors"
@@ -32,7 +35,6 @@ import (
 
 	"github.com/mattermost/mattermost-server/v6/app/email"
 	"github.com/mattermost/mattermost-server/v6/app/featureflag"
-	"github.com/mattermost/mattermost-server/v6/app/platform"
 	"github.com/mattermost/mattermost-server/v6/app/request"
 	"github.com/mattermost/mattermost-server/v6/app/teams"
 	"github.com/mattermost/mattermost-server/v6/app/users"
@@ -129,6 +131,10 @@ type Server struct {
 
 	localModeServer *http.Server
 
+	metricsServer *http.Server
+	metricsRouter *mux.Router
+	metricsLock   sync.Mutex
+
 	didFinishListen chan struct{}
 
 	goroutineCount      int32
@@ -171,7 +177,6 @@ type Server struct {
 	configStore             *configWrapper
 	filestore               filestore.FileBackend
 
-	platformService  *platform.PlatformService
 	telemetryService *telemetry.TelemetryService
 	userService      *users.UserService
 	teamService      *teams.TeamService
@@ -251,17 +256,6 @@ func NewServer(options ...Option) (*Server, error) {
 		s.configStore = &configWrapper{srv: s, Store: configStore}
 	}
 
-	ps, sErr := platform.New(platform.ServiceConfig{
-		ConfigStore:  s.configStore.Store,
-		StartMetrics: s.startMetrics,
-		Metrics:      s.Metrics,
-		Cluster:      s.Cluster,
-	})
-	if sErr != nil {
-		return nil, errors.Wrap(sErr, "failed to initialize platform")
-	}
-	s.platformService = ps
-
 	// Step 2: Logging
 	if err := s.initLogging(); err != nil {
 		mlog.Error("Could not initiate logging", mlog.Err(err))
@@ -625,6 +619,10 @@ func NewServer(options ...Option) (*Server, error) {
 		s.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableDeveloper = true })
 	}
 
+	if s.startMetrics {
+		s.SetupMetricsServer()
+	}
+
 	s.AddLicenseListener(func(oldLicense, newLicense *model.License) {
 		if (oldLicense == nil && newLicense == nil) || !s.startMetrics {
 			return
@@ -634,7 +632,7 @@ func NewServer(options ...Option) (*Server, error) {
 			return
 		}
 
-		s.platformService.RestartMetrics() // TODO: remove when this moved to the platform service
+		s.SetupMetricsServer()
 	})
 
 	s.SearchEngine.UpdateConfig(s.Config())
@@ -703,6 +701,24 @@ func NewServer(options ...Option) (*Server, error) {
 	return s, nil
 }
 
+func (s *Server) SetupMetricsServer() {
+	if !*s.Config().MetricsSettings.Enable {
+		return
+	}
+
+	s.StopMetricsServer()
+
+	if err := s.InitMetricsRouter(); err != nil {
+		mlog.Error("Error initiating metrics router.", mlog.Err(err))
+	}
+
+	if s.Metrics != nil {
+		s.Metrics.Register()
+	}
+
+	s.startMetricsServer()
+}
+
 func maxInt(a, b int) int {
 	if a > b {
 		return a
@@ -1030,7 +1046,7 @@ func (s *Server) Shutdown() {
 		s.Cluster.StopInterNodeCommunication()
 	}
 
-	s.platformService.ShutdownMetrics()
+	s.StopMetricsServer()
 
 	// This must be done after the cluster is stopped.
 	if s.Jobs != nil {
@@ -1614,9 +1630,104 @@ func doConfigCleanup(s *Server) {
 	}
 }
 
-// TODO: remove this method when we switch to using platform service.
+func (s *Server) StopMetricsServer() {
+	s.metricsLock.Lock()
+	defer s.metricsLock.Unlock()
+
+	if s.metricsServer != nil {
+		ctx, cancel := context.WithTimeout(context.Background(), TimeToWaitForConnectionsToCloseOnServerShutdown)
+		defer cancel()
+
+		s.metricsServer.Shutdown(ctx)
+		s.Log.Info("Metrics and profiling server is stopping")
+	}
+}
+
 func (s *Server) HandleMetrics(route string, h http.Handler) {
-	s.platformService.HandleMetrics(route, h)
+	if s.metricsRouter != nil {
+		s.metricsRouter.Handle(route, h)
+	}
+}
+
+func (s *Server) InitMetricsRouter() error {
+	s.metricsRouter = mux.NewRouter()
+	runtime.SetBlockProfileRate(*s.Config().MetricsSettings.BlockProfileRate)
+
+	metricsPage := `
+			<html>
+				<body>{{if .}}
+					<div><a href="/metrics">Metrics</a></div>{{end}}
+					<div><a href="/debug/pprof/">Profiling Root</a></div>
+					<div><a href="/debug/pprof/cmdline">Profiling Command Line</a></div>
+					<div><a href="/debug/pprof/symbol">Profiling Symbols</a></div>
+					<div><a href="/debug/pprof/goroutine">Profiling Goroutines</a></div>
+					<div><a href="/debug/pprof/heap">Profiling Heap</a></div>
+					<div><a href="/debug/pprof/threadcreate">Profiling Threads</a></div>
+					<div><a href="/debug/pprof/block">Profiling Blocking</a></div>
+					<div><a href="/debug/pprof/trace">Profiling Execution Trace</a></div>
+					<div><a href="/debug/pprof/profile">Profiling CPU</a></div>
+				</body>
+			</html>
+		`
+	metricsPageTmpl, err := template.New("page").Parse(metricsPage)
+	if err != nil {
+		return errors.Wrap(err, "failed to create template")
+	}
+
+	rootHandler := func(w http.ResponseWriter, r *http.Request) {
+		metricsPageTmpl.Execute(w, s.Metrics != nil)
+	}
+
+	s.metricsRouter.HandleFunc("/", rootHandler)
+	s.metricsRouter.StrictSlash(true)
+
+	s.metricsRouter.Handle("/debug", http.RedirectHandler("/", http.StatusMovedPermanently))
+	s.metricsRouter.HandleFunc("/debug/pprof/", pprof.Index)
+	s.metricsRouter.HandleFunc("/debug/pprof/cmdline", pprof.Cmdline)
+	s.metricsRouter.HandleFunc("/debug/pprof/profile", pprof.Profile)
+	s.metricsRouter.HandleFunc("/debug/pprof/symbol", pprof.Symbol)
+	s.metricsRouter.HandleFunc("/debug/pprof/trace", pprof.Trace)
+
+	// Manually add support for paths linked to by index page at /debug/pprof/
+	s.metricsRouter.Handle("/debug/pprof/goroutine", pprof.Handler("goroutine"))
+	s.metricsRouter.Handle("/debug/pprof/heap", pprof.Handler("heap"))
+	s.metricsRouter.Handle("/debug/pprof/threadcreate", pprof.Handler("threadcreate"))
+	s.metricsRouter.Handle("/debug/pprof/block", pprof.Handler("block"))
+
+	return nil
+}
+
+func (s *Server) startMetricsServer() {
+	var notify chan struct{}
+	s.metricsLock.Lock()
+	defer func() {
+		if notify != nil {
+			<-notify
+		}
+		s.metricsLock.Unlock()
+	}()
+
+	l, err := net.Listen("tcp", *s.Config().MetricsSettings.ListenAddress)
+	if err != nil {
+		mlog.Error(err.Error())
+		return
+	}
+
+	notify = make(chan struct{})
+	s.metricsServer = &http.Server{
+		Handler:      handlers.RecoveryHandler(handlers.PrintRecoveryStack(true))(s.metricsRouter),
+		ReadTimeout:  time.Duration(*s.Config().ServiceSettings.ReadTimeout) * time.Second,
+		WriteTimeout: time.Duration(*s.Config().ServiceSettings.WriteTimeout) * time.Second,
+	}
+
+	go func() {
+		close(notify)
+		if err := s.metricsServer.Serve(l); err != nil && err != http.ErrServerClosed {
+			mlog.Critical(err.Error())
+		}
+	}()
+
+	s.Log.Info("Metrics and profiling server is started", mlog.String("address", l.Addr().String()))
 }
 
 func (s *Server) sendLicenseUpForRenewalEmail(users map[string]*model.User, license *model.License) *model.AppError {