From aa149b47949bb890de7807a9e10b7c4dc984e07f Mon Sep 17 00:00:00 2001 From: Shashank Sinha Date: Thu, 7 Jul 2022 16:10:37 +0530 Subject: [PATCH] PMM-10271 Fix meta-metric label for general collector (#515) * PMM-10271 Fix meta-metric label for general collector Meta-metric generated for general collector were incorrectly labeled as dbstats. When dbstats and general collector were executed for a scrape request, two metric were generated with the same name and labels. This duplicate metric would trigger a panic in the metric generation library. This change updates the label for general collector and fixes the issue. * Fix tests for general collector * Fix test for replication status collector --- exporter/general_collector.go | 2 +- exporter/general_collector_test.go | 24 +++++++++++++++-------- exporter/indexstats_collector_test.go | 2 -- exporter/replset_status_collector_test.go | 11 +++++------ 4 files changed, 22 insertions(+), 17 deletions(-) diff --git a/exporter/general_collector.go b/exporter/general_collector.go index 7658238bc..1e95b5918 100644 --- a/exporter/general_collector.go +++ b/exporter/general_collector.go @@ -49,7 +49,7 @@ func (d *generalCollector) Collect(ch chan<- prometheus.Metric) { } func (d *generalCollector) collect(ch chan<- prometheus.Metric) { - defer prometheus.MeasureCollectTime(ch, "mongodb", "dbstats")() + defer prometheus.MeasureCollectTime(ch, "mongodb", "general")() ch <- mongodbUpMetric(d.ctx, d.base.client, d.base.logger) } diff --git a/exporter/general_collector_test.go b/exporter/general_collector_test.go index 8521de6d9..f7924befc 100644 --- a/exporter/general_collector_test.go +++ b/exporter/general_collector_test.go @@ -37,15 +37,22 @@ func TestGeneralCollector(t *testing.T) { client := tu.DefaultTestClient(ctx, t) c := newGeneralCollector(ctx, client, logrus.New()) + filter := []string{ + "collector_scrape_time_ms", + } + count := testutil.CollectAndCount(c, filter...) + assert.Equal(t, len(filter), count, "Meta-metric for collector is missing") + // The last \n at the end of this string is important expected := strings.NewReader(` # HELP mongodb_up Whether MongoDB is up. # TYPE mongodb_up gauge mongodb_up 1 - # HELP collector_scrape_time_ms Time taken for scrape by collector - # TYPE collector_scrape_time_ms gauge - collector_scrape_time_ms{collector="dbstats",exporter="mongodb"} 0` + "\n") - err := testutil.CollectAndCompare(c, expected) + ` + "\n") + filter = []string{ + "mongodb_up", + } + err := testutil.CollectAndCompare(c, expected, filter...) require.NoError(t, err) assert.NoError(t, client.Disconnect(ctx)) @@ -54,9 +61,10 @@ func TestGeneralCollector(t *testing.T) { # HELP mongodb_up Whether MongoDB is up. # TYPE mongodb_up gauge mongodb_up 0 - # HELP collector_scrape_time_ms Time taken for scrape by collector - # TYPE collector_scrape_time_ms gauge - collector_scrape_time_ms{collector="dbstats",exporter="mongodb"} 0` + "\n") - err = testutil.CollectAndCompare(c, expected) + ` + "\n") + filter = []string{ + "mongodb_up", + } + err = testutil.CollectAndCompare(c, expected, filter...) require.NoError(t, err) } diff --git a/exporter/indexstats_collector_test.go b/exporter/indexstats_collector_test.go index f3c7505d0..af87212a0 100644 --- a/exporter/indexstats_collector_test.go +++ b/exporter/indexstats_collector_test.go @@ -79,7 +79,6 @@ mongodb_indexstats_accesses_ops{collection="testcol_02",database="testdb",key_na mongodb_indexstats_accesses_ops{collection="testcol_02",database="testdb",key_name="idx_01"} 0` + "\n") - // on race run we will have collector_scrape_time_ms > 0, in normal run it will be fast and will 0 filter := []string{ "mongodb_indexstats_accesses_ops", } @@ -132,7 +131,6 @@ func TestDescendingIndexOverride(t *testing.T) { mongodb_indexstats_accesses_ops{collection="testcol_02",database="testdb",key_name="f1_1"} 0 mongodb_indexstats_accesses_ops{collection="testcol_02",database="testdb",key_name="f1_DESC"} 0` + "\n") - // on race run we will have collector_scrape_time_ms > 0, in normal run it will be fast and will 0 filter := []string{ "mongodb_indexstats_accesses_ops", } diff --git a/exporter/replset_status_collector_test.go b/exporter/replset_status_collector_test.go index 73426cf0c..1362ba538 100644 --- a/exporter/replset_status_collector_test.go +++ b/exporter/replset_status_collector_test.go @@ -69,10 +69,9 @@ func TestReplsetStatusCollectorNoSharding(t *testing.T) { c := newReplicationSetStatusCollector(ctx, client, logrus.New(), false, ti) - expected := strings.NewReader(` - # HELP collector_scrape_time_ms Time taken for scrape by collector - # TYPE collector_scrape_time_ms gauge - collector_scrape_time_ms{collector="replset_status",exporter="mongodb"} 0` + "\n") - err := testutil.CollectAndCompare(c, expected) - assert.NoError(t, err) + // Replication set metrics should not be generated for unsharded server + count := testutil.CollectAndCount(c) + + metaMetricCount := 1 + assert.Equal(t, metaMetricCount, count, "Mismatch in metric count for collector run on unsharded server") }