code review - make sure in tests that the metric is not captured if i…

…t's not enabled
open-telemetry · Aug 18, 2022 · 1bde002 · 1bde002
1 parent 55d3a17
commit 1bde002
Show file tree

Hide file tree

Showing 3 changed files with 40 additions and 2 deletions.
diff --git a/receiver/hostmetricsreceiver/internal/scraper/processscraper/metadata.yaml b/receiver/hostmetricsreceiver/internal/scraper/processscraper/metadata.yaml
@@ -114,4 +114,4 @@ metrics:
     sum:
       value_type: int
       aggregation: cumulative
-      monotonic: false
+      monotonic: false
diff --git a/receiver/hostmetricsreceiver/internal/scraper/processscraper/process_scraper.go b/receiver/hostmetricsreceiver/internal/scraper/processscraper/process_scraper.go
@@ -246,6 +246,9 @@ func (s *scraper) scrapeAndAppendDiskIOMetric(now pcommon.Timestamp, handle proc
 }
 
 func (s *scraper) scrapeAndAppendThreadsMetrics(now pcommon.Timestamp, handle processHandle) error {
+	if !s.config.Metrics.ProcessThreads.Enabled {
+		return nil
+	}
 	threads, err := handle.NumThreads()
 	if err != nil {
 		return err

diff --git a/receiver/hostmetricsreceiver/internal/scraper/processscraper/process_scraper_test.go b/receiver/hostmetricsreceiver/internal/scraper/processscraper/process_scraper_test.go
@@ -50,6 +50,7 @@ func TestScrape(t *testing.T) {
 		name                                   string
 		expectMetricsWithDirectionAttribute    bool
 		expectMetricsWithoutDirectionAttribute bool
+		expectThreadsCount                     bool
 		mutateScraper                          func(*scraper)
 	}
 	testCases := []testCase{
@@ -76,14 +77,22 @@ func TestScrape(t *testing.T) {
 				s.emitMetricsWithoutDirectionAttribute = true
 			},
 		},
+		{
+			name:               "With threads count",
+			expectThreadsCount: true,
+		},
 	}
 
 	const bootTime = 100
 	const expectedStartTime = 100 * 1e9
 
 	for _, test := range testCases {
 		t.Run(test.name, func(t *testing.T) {
-			scraper, err := newProcessScraper(componenttest.NewNopReceiverCreateSettings(), &Config{Metrics: metadata.DefaultMetricsSettings()})
+			metricsConfig := metadata.DefaultMetricsSettings()
+			if test.expectThreadsCount {
+				metricsConfig.ProcessThreads.Enabled = true
+			}
+			scraper, err := newProcessScraper(componenttest.NewNopReceiverCreateSettings(), &Config{Metrics: metricsConfig})
 			if test.mutateScraper != nil {
 				test.mutateScraper(scraper)
 			}
@@ -119,6 +128,11 @@ func TestScrape(t *testing.T) {
 			if test.expectMetricsWithoutDirectionAttribute {
 				assertNewDiskIOMetricValid(t, md.ResourceMetrics(), expectedStartTime)
 			}
+			if test.expectThreadsCount {
+				assertThreadsCountValid(t, md.ResourceMetrics(), expectedStartTime)
+			} else {
+				assertMetricMissing(t, md.ResourceMetrics(), "process.threads")
+			}
 			assertSameTimeStampForAllMetricsWithinResource(t, md.ResourceMetrics())
 		})
 	}
@@ -175,6 +189,27 @@ func assertNewDiskIOMetricValid(t *testing.T, resourceMetrics pmetric.ResourceMe
 	}
 }
 
+func assertThreadsCountValid(t *testing.T, resourceMetrics pmetric.ResourceMetricsSlice, startTime pcommon.Timestamp) {
+	for _, metricName := range []string{"process.threads"} {
+		threadsMetric := getMetric(t, metricName, resourceMetrics)
+		if startTime != 0 {
+			internal.AssertSumMetricStartTimeEquals(t, threadsMetric, startTime)
+		}
+	}
+}
+
+func assertMetricMissing(t *testing.T, resourceMetrics pmetric.ResourceMetricsSlice, expectedMetricName string) {
+	for i := 0; i < resourceMetrics.Len(); i++ {
+		metrics := getMetricSlice(t, resourceMetrics.At(i))
+		for j := 0; j < metrics.Len(); j++ {
+			metric := metrics.At(j)
+			if metric.Name() == expectedMetricName {
+				require.Fail(t, fmt.Sprintf("metric with name %s should not be present", expectedMetricName))
+			}
+		}
+	}
+}
+
 func assertOldDiskIOMetricValid(t *testing.T, resourceMetrics pmetric.ResourceMetricsSlice,
 	startTime pcommon.Timestamp) {
 	diskIOMetric := getMetric(t, "process.disk.io", resourceMetrics)