PMM-9459 Remove usage of DescribeByCollect (#443)

DescribeByCollect method internally uses Collect method. This causes
exporter to retrieve metric data from database twice per scrape. This
patch reduces monitoring load on database server, as well as reduces
CPU and memory usage of exporter.

Changes to Describe and Collect methods has introduced a dependency.
Before a call to Collect method, Describe method  needs to be called.
Tests need to be updated to handle this assumption.

Author: Shashank Sinha

exporter/base_collector.go

@@ -0,0 +1,71 @@
+// mongodb_exporter
+// Copyright (C) 2022 Percona LLC
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU Affero General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU Affero General Public License for more details.
+//
+// You should have received a copy of the GNU Affero General Public License
+// along with this program. If not, see <https://www.gnu.org/licenses/>.
+
+package exporter
+
+import (
+	"sync"
+
+	"github.com/prometheus/client_golang/prometheus"
+	"github.com/sirupsen/logrus"
+	"go.mongodb.org/mongo-driver/mongo"
+)
+
+type baseCollector struct {
+	client *mongo.Client
+	logger *logrus.Logger
+
+	lock         sync.Mutex
+	metricsCache []prometheus.Metric
+}
+
+// newBaseCollector creates a skeletal collector, which is used to create other collectors.
+func newBaseCollector(client *mongo.Client, logger *logrus.Logger) *baseCollector {
+	return &baseCollector{
+		client: client,
+		logger: logger,
+	}
+}
+
+func (d *baseCollector) Describe(ch chan<- *prometheus.Desc, collect func(mCh chan<- prometheus.Metric)) {
+	d.lock.Lock()
+	defer d.lock.Unlock()
+
+	d.metricsCache = make([]prometheus.Metric, 0, defaultCacheSize)
+
+	// This is a copy/paste of prometheus.DescribeByCollect(d, ch) with the aggreated functionality
+	// to populate the metrics cache. Since on each scrape Prometheus will call Describe and inmediatelly
+	// after it will call Collect, it is safe to populate the cache here.
+	metrics := make(chan prometheus.Metric)
+	go func() {
+		collect(metrics)
+		close(metrics)
+	}()
+
+	for m := range metrics {
+		d.metricsCache = append(d.metricsCache, m) // populate the cache
+		ch <- m.Desc()
+	}
+}
+
+func (d *baseCollector) Collect(ch chan<- prometheus.Metric) {
+	d.lock.Lock()
+	defer d.lock.Unlock()
+
+	for _, metric := range d.metricsCache {
+		ch <- metric
+	}
+}

exporter/collstats_collector.go

@@ -27,26 +27,48 @@ import (
 )
 
 type collstatsCollector struct {
-	ctx             context.Context
-	client          *mongo.Client
-	collections     []string
+	ctx  context.Context
+	base *baseCollector
+
 	compatibleMode  bool
 	discoveringMode bool
-	logger          *logrus.Logger
 	topologyInfo    labelsGetter
+
+	collections []string
+}
+
+// newCollectionStatsCollector creates a collector for statistics about collections.
+func newCollectionStatsCollector(ctx context.Context, client *mongo.Client, logger *logrus.Logger, compatible, discovery bool, topology labelsGetter, collections []string) *collstatsCollector {
+	return &collstatsCollector{
+		ctx:  ctx,
+		base: newBaseCollector(client, logger),
+
+		compatibleMode:  compatible,
+		discoveringMode: discovery,
+		topologyInfo:    topology,
+
+		collections: collections,
+	}
 }
 
 func (d *collstatsCollector) Describe(ch chan<- *prometheus.Desc) {
-	prometheus.DescribeByCollect(d, ch)
+	d.base.Describe(ch, d.collect)
 }
 
 func (d *collstatsCollector) Collect(ch chan<- prometheus.Metric) {
+	d.base.Collect(ch)
+}
+
+func (d *collstatsCollector) collect(ch chan<- prometheus.Metric) {
 	collections := d.collections
 
+	client := d.base.client
+	logger := d.base.logger
+
 	if d.discoveringMode {
-		namespaces, err := listAllCollections(d.ctx, d.client, d.collections, systemDBs)
+		namespaces, err := listAllCollections(d.ctx, client, d.collections, systemDBs)
 		if err != nil {
-			d.logger.Errorf("cannot auto discover databases and collections: %s", err.Error())
+			logger.Errorf("cannot auto discover databases and collections: %s", err.Error())
 
 			return
 		}
@@ -81,22 +103,22 @@ func (d *collstatsCollector) Collect(ch chan<- prometheus.Metric) {
 			},
 		}
 
-		cursor, err := d.client.Database(database).Collection(collection).Aggregate(d.ctx, mongo.Pipeline{aggregation, project})
+		cursor, err := client.Database(database).Collection(collection).Aggregate(d.ctx, mongo.Pipeline{aggregation, project})
 		if err != nil {
-			d.logger.Errorf("cannot get $collstats cursor for collection %s.%s: %s", database, collection, err)
+			logger.Errorf("cannot get $collstats cursor for collection %s.%s: %s", database, collection, err)
 
 			continue
 		}
 
 		var stats []bson.M
 		if err = cursor.All(d.ctx, &stats); err != nil {
-			d.logger.Errorf("cannot get $collstats for collection %s.%s: %s", database, collection, err)
+			logger.Errorf("cannot get $collstats for collection %s.%s: %s", database, collection, err)
 
 			continue
 		}
 
-		d.logger.Debugf("$collStats metrics for %s.%s", database, collection)
-		debugResult(d.logger, stats)
+		logger.Debugf("$collStats metrics for %s.%s", database, collection)
+		debugResult(logger, stats)
 
 		// Since all collections will have the same fields, we need to use a metric prefix (db+col)
 		// to differentiate metrics between collection. Labels are being set only to matke it easier

exporter/collstats_collector_test.go

@@ -53,25 +53,20 @@ func TestCollStatsCollector(t *testing.T) {
 
 	ti := labelsGetterMock{}
 
-	c := &collstatsCollector{
-		client:       client,
-		collections:  []string{"testdb.testcol_00", "testdb.testcol_01", "testdb.testcol_02"},
-		logger:       logrus.New(),
-		topologyInfo: ti,
-	}
+	collection := []string{"testdb.testcol_00", "testdb.testcol_01", "testdb.testcol_02"}
+	c := newCollectionStatsCollector(ctx, client, logrus.New(), false, false, ti, collection)
 
 	// The last \n at the end of this string is important
 	expected := strings.NewReader(`
-# HELP mongodb_testdb_testcol_00_latencyStats_commands_latency testdb.testcol_00.latencyStats.commands.
-# TYPE mongodb_testdb_testcol_00_latencyStats_commands_latency untyped
-mongodb_testdb_testcol_00_latencyStats_commands_latency{collection="testcol_00",database="testdb"} 0
-# HELP mongodb_testdb_testcol_01_latencyStats_commands_latency testdb.testcol_01.latencyStats.commands.
-# TYPE mongodb_testdb_testcol_01_latencyStats_commands_latency untyped
-mongodb_testdb_testcol_01_latencyStats_commands_latency{collection="testcol_01",database="testdb"} 0
-# HELP mongodb_testdb_testcol_02_latencyStats_commands_latency testdb.testcol_02.latencyStats.commands.
-# TYPE mongodb_testdb_testcol_02_latencyStats_commands_latency untyped
-mongodb_testdb_testcol_02_latencyStats_commands_latency{collection="testcol_02",database="testdb"} 0` +
-		"\n")
+	# HELP mongodb_testdb_testcol_00_latencyStats_commands_latency testdb.testcol_00.latencyStats.commands.
+	# TYPE mongodb_testdb_testcol_00_latencyStats_commands_latency untyped
+	mongodb_testdb_testcol_00_latencyStats_commands_latency{collection="testcol_00",database="testdb"} 0
+	# HELP mongodb_testdb_testcol_01_latencyStats_commands_latency testdb.testcol_01.latencyStats.commands.
+	# TYPE mongodb_testdb_testcol_01_latencyStats_commands_latency untyped
+	mongodb_testdb_testcol_01_latencyStats_commands_latency{collection="testcol_01",database="testdb"} 0
+	# HELP mongodb_testdb_testcol_02_latencyStats_commands_latency testdb.testcol_02.latencyStats.commands.
+	# TYPE mongodb_testdb_testcol_02_latencyStats_commands_latency untyped
+	mongodb_testdb_testcol_02_latencyStats_commands_latency{collection="testcol_02",database="testdb"} 0` + "\n")
 
 	// Filter metrics for 2 reasons:
 	// 1. The result is huge

exporter/dbstats_collector.go

@@ -26,40 +26,61 @@ import (
 )
 
 type dbstatsCollector struct {
-	ctx            context.Context
-	client         *mongo.Client
+	ctx  context.Context
+	base *baseCollector
+
 	compatibleMode bool
-	logger         *logrus.Logger
 	topologyInfo   labelsGetter
+
+	databaseFilter []string
+}
+
+// newDBStatsCollector creates a collector for statistics on database storage.
+func newDBStatsCollector(ctx context.Context, client *mongo.Client, logger *logrus.Logger, compatible bool, topology labelsGetter, databaseRegex []string) *dbstatsCollector {
+	return &dbstatsCollector{
+		ctx:  ctx,
+		base: newBaseCollector(client, logger),
+
+		compatibleMode: compatible,
+		topologyInfo:   topology,
+
+		databaseFilter: databaseRegex,
+	}
 }
 
 func (d *dbstatsCollector) Describe(ch chan<- *prometheus.Desc) {
-	prometheus.DescribeByCollect(d, ch)
+	d.base.Describe(ch, d.collect)
 }
 
 func (d *dbstatsCollector) Collect(ch chan<- prometheus.Metric) {
-	// List all databases names
-	dbNames, err := d.client.ListDatabaseNames(d.ctx, bson.M{})
+	d.base.Collect(ch)
+}
+
+func (d *dbstatsCollector) collect(ch chan<- prometheus.Metric) {
+	logger := d.base.logger
+	client := d.base.client
+
+	dbNames, err := databases(d.ctx, client, d.databaseFilter, nil)
 	if err != nil {
-		d.logger.Errorf("Failed to get database names: %s", err)
+		logger.Errorf("Failed to get database names: %s", err)
 
 		return
 	}
 
-	d.logger.Debugf("getting stats for databases: %v", dbNames)
+	logger.Debugf("getting stats for databases: %v", dbNames)
 	for _, db := range dbNames {
 		var dbStats bson.M
 		cmd := bson.D{{Key: "dbStats", Value: 1}, {Key: "scale", Value: 1}}
-		r := d.client.Database(db).RunCommand(d.ctx, cmd)
+		r := client.Database(db).RunCommand(d.ctx, cmd)
 		err := r.Decode(&dbStats)
 		if err != nil {
-			d.logger.Errorf("Failed to get $dbstats for database %s: %s", db, err)
+			logger.Errorf("Failed to get $dbstats for database %s: %s", db, err)
 
 			continue
 		}
 
-		d.logger.Debugf("$dbStats metrics for %s", db)
-		debugResult(d.logger, dbStats)
+		logger.Debugf("$dbStats metrics for %s", db)
+		debugResult(logger, dbStats)
 
 		prefix := "dbstats"
 
@@ -69,7 +90,8 @@ func (d *dbstatsCollector) Collect(ch chan<- prometheus.Metric) {
 		// to differentiate metrics between different databases.
 		labels["database"] = db
 
-		for _, metric := range makeMetrics(prefix, dbStats, labels, d.compatibleMode) {
+		newMetrics := makeMetrics(prefix, dbStats, labels, d.compatibleMode)
+		for _, metric := range newMetrics {
 			ch <- metric
 		}
 	}

exporter/dbstats_collector_test.go

@@ -19,25 +19,29 @@ package exporter
 import (
 	"context"
 	"fmt"
-	"sort"
+	"strings"
 	"testing"
 	"time"
 
-	"github.com/percona/exporter_shared/helpers"
+	"github.com/prometheus/client_golang/prometheus/testutil"
 	"github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/assert"
 	"go.mongodb.org/mongo-driver/bson"
 
 	"github.com/percona/mongodb_exporter/internal/tu"
 )
 
+const (
+	dbName = "testdb"
+)
+
 func TestDBStatsCollector(t *testing.T) {
 	ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second)
 	defer cancel()
 
 	client := tu.DefaultTestClient(ctx, t)
 
-	database := client.Database("testdb")
+	database := client.Database(dbName)
 	database.Drop(ctx) //nolint
 
 	defer func() {
@@ -55,47 +59,24 @@ func TestDBStatsCollector(t *testing.T) {
 
 	ti := labelsGetterMock{}
 
-	c := &dbstatsCollector{
-		client:       client,
-		logger:       logrus.New(),
-		topologyInfo: ti,
-	}
-
-	expected := []string{
-		"# HELP mongodb_dbstats_collections dbstats.",
-		"# TYPE mongodb_dbstats_collections untyped",
-		"mongodb_dbstats_collections{database=\"testdb\"} 3",
-		"# HELP mongodb_dbstats_dataSize dbstats.",
-		"# TYPE mongodb_dbstats_dataSize untyped",
-		"mongodb_dbstats_dataSize{database=\"testdb\"} 1200",
-		"# HELP mongodb_dbstats_indexSize dbstats.",
-		"# TYPE mongodb_dbstats_indexSize untyped",
-		"mongodb_dbstats_indexSize{database=\"testdb\"} 12288",
-		"# HELP mongodb_dbstats_indexes dbstats.",
-		"# TYPE mongodb_dbstats_indexes untyped",
-		"mongodb_dbstats_indexes{database=\"testdb\"} 3",
-		"# HELP mongodb_dbstats_objects dbstats.",
-		"# TYPE mongodb_dbstats_objects untyped",
-		"mongodb_dbstats_objects{database=\"testdb\"} 30",
-	}
+	c := newDBStatsCollector(ctx, client, logrus.New(), false, ti, []string{dbName})
+	expected := strings.NewReader(`
+	# HELP mongodb_dbstats_collections dbstats.
+	# TYPE mongodb_dbstats_collections untyped
+	mongodb_dbstats_collections{database="testdb"} 3
+	# HELP mongodb_dbstats_indexes dbstats.
+	# TYPE mongodb_dbstats_indexes untyped
+	mongodb_dbstats_indexes{database="testdb"} 3
+	# HELP mongodb_dbstats_objects dbstats.
+	# TYPE mongodb_dbstats_objects untyped
+	mongodb_dbstats_objects{database="testdb"} 30` + "\n")
 
-	metrics := helpers.CollectMetrics(c)
-	actualMetrics := helpers.ReadMetrics(metrics)
+	// Only look at metrics created by our activity
 	filters := []string{
 		"mongodb_dbstats_collections",
-		"mongodb_dbstats_dataSize",
-		"mongodb_dbstats_indexSize",
 		"mongodb_dbstats_indexes",
 		"mongodb_dbstats_objects",
 	}
-	labels := map[string]string{
-		"database": "testdb",
-	}
-	actualMetrics = filterMetricsWithLabels(actualMetrics,
-		filters,
-		labels)
-	actualLines := helpers.Format(helpers.WriteMetrics(actualMetrics))
-	sort.Strings(actualLines)
-	sort.Strings(expected)
-	assert.Equal(t, expected, actualLines)
+	err := testutil.CollectAndCompare(c, expected, filters...)
+	assert.NoError(t, err)
 }