only expose metrics via ServeMetrics (#491)

* remove avatar handling

* drop support for MySQL

Simplify unit tests (more idiomatic now that there's one driver!) and
speed up test container initialization.

* Drop job status tracking

This is redundant given the way
`[cluster.job](https://github.com/mattermost/mattermost/blob/a57be19da4cc97c11d3f1237d25bf9bf4113bf80/server/public/pluginapi/cluster/job.go#L91-L96)`
works.

* drop TestStart, it tested nothing

* only expose metrics via ServeMetrics

server/metrics/server.go

@@ -2,7 +2,6 @@ package metrics
 
 import (
 	"net/http"
-	"time"
 
 	"github.com/pkg/errors"
 	"github.com/prometheus/client_golang/prometheus/promhttp"
@@ -21,17 +20,11 @@ func (el *ErrorLoggerWrapper) Println(v ...interface{}) {
 	logrus.Warn("metric server error", v)
 }
 
-// NewMetricsServer factory method to create a new prometheus server.
-func NewMetricsServer(address string, metricsService Metrics) *Server {
-	return &Server{
-		&http.Server{
-			ReadTimeout: 30 * time.Second,
-			Addr:        address,
-			Handler: promhttp.HandlerFor(metricsService.GetRegistry(), promhttp.HandlerOpts{
-				ErrorLog: &ErrorLoggerWrapper{},
-			}),
-		},
-	}
+// NewMetricsHandler creates an HTTP handler to expose metrics.
+func NewMetricsHandler(metricsService Metrics) http.Handler {
+	return promhttp.HandlerFor(metricsService.GetRegistry(), promhttp.HandlerOpts{
+		ErrorLog: &ErrorLoggerWrapper{},
+	})
 }
 
 // Run will start the prometheus server.

server/plugin.go

@@ -45,9 +45,9 @@ const (
 	whitelistClusterMutexKey     = "whitelist_cluster_mutex"
 	msteamsUserTypeGuest         = "Guest"
 	syncUsersJobName             = "sync_users"
+	metricsJobName               = "metrics"
 	checkCredentialsJobName      = "check_credentials" //#nosec G101 -- This is a false positive
 
-	metricsExposePort          = ":9094"
 	updateMetricsTaskFrequency = 15 * time.Minute
 )
 
@@ -83,15 +83,16 @@ type Plugin struct {
 
 	clientBuilderWithToken func(string, string, string, string, *oauth2.Token, *pluginapi.LogService) msteams.Client
 	metricsService         metrics.Metrics
-	metricsServer          *metrics.Server
+	metricsHandler         http.Handler
+	metricsJob             *cluster.Job
 }
 
 func (p *Plugin) ServeHTTP(_ *plugin.Context, w http.ResponseWriter, r *http.Request) {
 	p.apiHandler.ServeHTTP(w, r)
 }
 
 func (p *Plugin) ServeMetrics(_ *plugin.Context, w http.ResponseWriter, r *http.Request) {
-	p.metricsServer.Handler.ServeHTTP(w, r)
+	p.metricsHandler.ServeHTTP(w, r)
 }
 
 func (p *Plugin) GetAPI() plugin.API {
@@ -241,23 +242,28 @@ func (p *Plugin) connectTeamsAppClient() error {
 }
 
 func (p *Plugin) start(isRestart bool) {
-	enableMetrics := p.API.GetConfig().MetricsSettings.Enable
-
-	if enableMetrics != nil && *enableMetrics {
-		// run metrics server to expose data
-		p.runMetricsServer()
-		// run metrics updater recurring task
-		p.runMetricsUpdaterTask(p.store, updateMetricsTaskFrequency)
+	var err error
 
-		p.metricsService.ObserveWhitelistLimit(p.configuration.ConnectedUsersAllowed)
+	if !isRestart {
+		p.metricsJob, err = cluster.Schedule(
+			p.API,
+			metricsJobName,
+			cluster.MakeWaitForRoundedInterval(updateMetricsTaskFrequency),
+			p.updateMetrics,
+		)
+		if err != nil {
+			p.API.LogError("failed to start metrics job", "error", err)
+		}
 	}
 
+	p.metricsService.ObserveWhitelistLimit(p.configuration.ConnectedUsersAllowed)
+
 	// We don't restart the activity handler since it's stateless.
 	if !isRestart {
 		p.activityHandler.Start()
 	}
 
-	err := p.connectTeamsAppClient()
+	err = p.connectTeamsAppClient()
 	if err != nil {
 		return
 	}
@@ -362,6 +368,12 @@ func (p *Plugin) stop(isRestart bool) {
 	}
 
 	p.stopSyncUsersJob()
+
+	if !isRestart && p.metricsJob != nil {
+		if err := p.metricsJob.Close(); err != nil {
+			p.API.LogError("failed to close metrics job", "error", err)
+		}
+	}
 }
 
 func (p *Plugin) restart() {
@@ -420,7 +432,7 @@ func (p *Plugin) OnActivate() error {
 		InstallationID: os.Getenv("MM_CLOUD_INSTALLATION_ID"),
 		PluginVersion:  manifest.Version,
 	})
-	p.metricsServer = metrics.NewMetricsServer(metricsExposePort, p.GetMetrics())
+	p.metricsHandler = metrics.NewMetricsHandler(p.GetMetrics())
 
 	p.apiClient = pluginapi.NewClient(p.API, p.Driver)
 
@@ -506,10 +518,6 @@ func (p *Plugin) OnActivate() error {
 }
 
 func (p *Plugin) OnDeactivate() error {
-	if err := p.metricsServer.Shutdown(); err != nil {
-		p.API.LogWarn("Error shutting down metrics server", "error", err)
-	}
-
 	p.stop(false)
 	return nil
 }
@@ -761,36 +769,12 @@ func isRemoteUser(user *model.User) bool {
 	return user.RemoteId != nil && *user.RemoteId != "" && strings.HasPrefix(user.Username, "msteams_")
 }
 
-func (p *Plugin) runMetricsServer() {
-	p.API.LogInfo("Starting metrics server", "port", metricsExposePort)
-
-	// Run server to expose metrics
-	go func() {
-		defer func() {
-			if r := recover(); r != nil {
-				p.GetMetrics().ObserveGoroutineFailure()
-				p.API.LogError("Recovering from panic", "panic", r, "stack", string(debug.Stack()))
-			}
-		}()
-
-		err := p.metricsServer.Run()
-		if err != nil && !errors.Is(err, http.ErrServerClosed) {
-			p.API.LogError("Metrics server could not be started", "error", err)
-		}
-	}()
-}
-
-func (p *Plugin) runMetricsUpdaterTask(store store.Store, updateMetricsTaskFrequency time.Duration) {
-	metricsUpdater := func() {
-		stats, err := store.GetStats()
-		if err != nil {
-			p.API.LogWarn("failed to update computed metrics", "error", err)
-		}
-		p.GetMetrics().ObserveConnectedUsers(stats.ConnectedUsers)
-		p.GetMetrics().ObserveSyntheticUsers(stats.SyntheticUsers)
-		p.GetMetrics().ObserveLinkedChannels(stats.LinkedChannels)
+func (p *Plugin) updateMetrics() {
+	stats, err := p.store.GetStats()
+	if err != nil {
+		p.API.LogWarn("failed to update computed metrics", "error", err)
 	}
-
-	metricsUpdater()
-	model.CreateRecurringTask("metricsUpdater", metricsUpdater, updateMetricsTaskFrequency)
+	p.GetMetrics().ObserveConnectedUsers(stats.ConnectedUsers)
+	p.GetMetrics().ObserveSyntheticUsers(stats.SyntheticUsers)
+	p.GetMetrics().ObserveLinkedChannels(stats.LinkedChannels)
 }