Skip to content

Commit

Permalink
PMM-10271 Fix meta-metric label for general collector (#515)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
ShashankSinha252 committed Jul 7, 2022
1 parent 1ff9bef commit aa149b4
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 17 deletions.
2 changes: 1 addition & 1 deletion exporter/general_collector.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
24 changes: 16 additions & 8 deletions exporter/general_collector_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand All @@ -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)
}
2 changes: 0 additions & 2 deletions exporter/indexstats_collector_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
}
Expand Down Expand Up @@ -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",
}
Expand Down
11 changes: 5 additions & 6 deletions exporter/replset_status_collector_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}

0 comments on commit aa149b4

Please sign in to comment.