[MM-57135] [MM-57136] some minor improvements before publishing (#18)

* some minor improvements before publishing

* make load plugin to actually get config from the server

* add tsdb path setting to the plugin setting page

* adjust local retention

* add download icon to the download button

plugin.json

@@ -25,7 +25,14 @@
         "footer": "This plugin is a utility to scrape and store various data points from Mattermost application in openmetrics format, similar to what Prometheus does. It operates by collecting metrics at regular intervals, allowing to be used while troubleshooting issues by including these metrics into a dump file.",
         "settings": [
             {
-                "key": "collect_metrics_from",
+                "key": "DBPath",
+                "display_name": "TSDB Path",
+                "type": "text",
+                "help_text": "The path of time-series database, the database is stored in plugins local storage. To change this setting, you'll need to restart the plugin for the changes to take effect.",
+                "default":"mattermost-plugin-metrics/data"
+            },
+            {
+                "key": "CollectMetricsFrom",
                 "display_name": "Collect metrics from",
                 "type": "dropdown",
                 "help_text": "Select the length of the metrics to be downloaded, consider saving the changes before download.",

server/api.go

@@ -1,15 +1,13 @@
 package main
 
 import (
-	"bytes"
 	"net/http"
+	"os"
 	"path/filepath"
 	"time"
 
 	"github.com/gorilla/mux"
 
-	root "github.com/mattermost/mattermost-plugin-metrics"
-
 	"github.com/mattermost/mattermost/server/public/model"
 	"github.com/mattermost/mattermost/server/v8/platform/shared/web"
 )
@@ -56,14 +54,8 @@ func (h *handler) authorized(next http.Handler) http.Handler {
 }
 
 func (h *handler) downloadDumpHandler(w http.ResponseWriter, r *http.Request) {
-	appCfg := h.plugin.API.GetConfig()
-	metricsFrom, ok := appCfg.PluginSettings.Plugins[root.Manifest.Id]["collect_metrics_from"]
-	if !ok {
-		w.WriteHeader(http.StatusInternalServerError)
-		return
-	}
 	var days int
-	switch metricsFrom {
+	switch *h.plugin.configuration.CollectMetricsFrom {
 	case "yesterday":
 		days = -1
 	case "3_days":
@@ -84,13 +76,21 @@ func (h *handler) downloadDumpHandler(w http.ResponseWriter, r *http.Request) {
 		w.WriteHeader(http.StatusInternalServerError)
 		return
 	}
+	defer func() {
+		fErr := os.RemoveAll(filepath.Dir(fp))
+		if fErr != nil {
+			h.plugin.API.LogError("Unable to remove temp directory for the dump", "error", fErr.Error())
+		}
+	}()
 
-	b, err := h.plugin.fileBackend.ReadFile(fp)
+	f, err := os.Open(fp)
 	if err != nil {
 		h.plugin.API.LogError("Failed to read dump file", "error", err.Error())
 		w.WriteHeader(http.StatusInternalServerError)
 		return
 	}
+	defer f.Close()
 
-	web.WriteFileResponse(filepath.Base(fp), "application/zip", 0, max, *appCfg.ServiceSettings.WebserverMode, bytes.NewReader(b), true, w, r)
+	appCfg := h.plugin.API.GetConfig()
+	web.WriteFileResponse(filepath.Base(fp), "application/zip", 0, max, *appCfg.ServiceSettings.WebserverMode, f, true, w, r)
 }

server/configuration.go

@@ -27,7 +27,7 @@ import (
 // If you add non-reference types to your configuration struct, be sure to rewrite Clone as a deep
 // copy appropriate for your types.
 type configuration struct {
-	DBPath                         *string
+	DBPath                         *string `json:"dbpath"`
 	AllowOverlappingCompaction     *bool
 	EnableMemorySnapshotOnShutdown *bool
 	BodySizeLimitBytes             *int64
@@ -49,6 +49,8 @@ type configuration struct {
 	FileStoreSyncPeriodMinutes *int
 	// FileStoreCleanupPeriodMinutes is the period to run cleanup job in the filestore
 	FileStoreCleanupPeriodMinutes *int
+	// CollectMetricsFrom is the period to collect metrics to create the dump
+	CollectMetricsFrom *string `json:"collectmetricsfrom"`
 }
 
 func (c *configuration) SetDefaults() {
@@ -91,6 +93,9 @@ func (c *configuration) SetDefaults() {
 	if c.FileStoreCleanupPeriodMinutes == nil {
 		c.FileStoreCleanupPeriodMinutes = model.NewInt(120)
 	}
+	if c.CollectMetricsFrom == nil {
+		c.CollectMetricsFrom = model.NewString("3_days")
+	}
 }
 
 func (c *configuration) IsValid() error {

server/const.go

@@ -1,11 +1,14 @@
 package main
 
+import "time"
+
 const (
-	PluginName     = "mattermost-plugin-metrics"
-	tsdbDirName    = "data"
-	metaFileName   = "meta.json"
-	metaVersion1   = 1
-	pluginDataDir  = "plugin-data"
-	zipFileName    = "tsdb_dump.tar.gz"
-	MaxRequestSize = 5 * 1024 * 1024 // 5MB
+	PluginName         = "mattermost-plugin-metrics"
+	tsdbDirName        = "data"
+	metaFileName       = "meta.json"
+	metaVersion1       = 1
+	pluginDataDir      = "plugin-data"
+	zipFileName        = "tsdb_dump.tar.gz"
+	MaxRequestSize     = 5 * 1024 * 1024 // 5MB
+	localRetentionDays = 3 * 24 * time.Hour
 )

server/dump.go

@@ -7,6 +7,7 @@ import (
 	"path/filepath"
 	"time"
 
+	"github.com/mattermost/mattermost/server/public/model"
 	"github.com/prometheus/prometheus/tsdb"
 )
 
@@ -19,8 +20,13 @@ func (p *Plugin) createDump(ctx context.Context, min, max time.Time, remoteStora
 		return "", errors.New("no blocks in the remote sotrage")
 	}
 
-	zipFileNameRemote := filepath.Join(pluginDataDir, PluginName, zipFileName)
-	dumpDir := filepath.Join(PluginName, "dump")
+	// we generate everything under a new directory to avoid conflicts
+	// between simultaneous downloads
+	tempDir := model.NewId()
+
+	dumpDir := filepath.Join(PluginName, "dump", tempDir, "data")
+	tempZipFile := filepath.Join(filepath.Dir(dumpDir), zipFileName)
+
 	for _, b := range blocks {
 		// read block meta from the remote filestore and decide if they are older than the
 		// retention period. If they are within the retention period, copy the data
@@ -67,27 +73,15 @@ func (p *Plugin) createDump(ctx context.Context, min, max time.Time, remoteStora
 		return "", err
 	}
 
-	err = compressDirectory(dumpDir, zipFileName)
+	err = compressDirectory(dumpDir, tempZipFile)
 	if err != nil {
 		return "", err
 	}
-	defer os.Remove(zipFileName)
 
 	err = os.RemoveAll(dumpDir)
 	if err != nil {
 		return "", err
 	}
 
-	f, err := os.Open(zipFileName)
-	if err != nil {
-		return "", err
-	}
-	defer f.Close()
-
-	_, err = p.fileBackend.WriteFile(f, zipFileNameRemote)
-	if err != nil {
-		return "", err
-	}
-
-	return zipFileNameRemote, nil
+	return tempZipFile, nil
 }

server/plugin.go

@@ -61,6 +61,10 @@ type Plugin struct {
 }
 
 func (p *Plugin) OnActivate() error {
+	if err := p.loadConfig(); err != nil {
+		return fmt.Errorf("could not load plugin config: %w", err)
+	}
+
 	p.client = pluginapi.NewClient(p.API, p.Driver)
 	p.logger = &metricsLogger{api: p.API}
 
@@ -111,20 +115,12 @@ func (p *Plugin) OnActivate() error {
 
 	p.closeChan = make(chan bool)
 	p.waitGroup = sync.WaitGroup{}
-	if p.configuration == nil {
-		p.configuration = new(configuration)
-		p.configuration.SetDefaults()
-	}
-
-	if err = p.configuration.IsValid(); err != nil {
-		return fmt.Errorf("could not validate config: %w", err)
-	}
 
 	// initiate local tsdb
 	p.tsdbLock.Lock()
 	defer p.tsdbLock.Unlock()
 	p.db, err = tsdb.Open(*p.configuration.DBPath, p.logger, nil, &tsdb.Options{
-		RetentionDuration:              int64(30 * 24 * time.Hour / time.Millisecond),
+		RetentionDuration:              int64(localRetentionDays / time.Millisecond),
 		AllowOverlappingCompaction:     *p.configuration.AllowOverlappingCompaction,
 		EnableMemorySnapshotOnShutdown: *p.configuration.EnableMemorySnapshotOnShutdown,
 	}, nil)

webapp/src/components/admin_settings/download_dump.tsx

@@ -16,6 +16,7 @@ export const DownloadDump = () => {
                     href={`${Client4.getUrl()}/plugins/${manifest.id}/download`}
                     rel='noopener noreferrer'
                 >
+                    <span className='icon icon-download-outline'/>
                     {'Download Dump'}
                 </a>
             </div>
@@ -27,4 +28,7 @@ const styles = {
     buttonRow: {
         margin: '12px 0',
     },
+    btn: {
+        gap: '0px',
+    },
 };