Remove report per cpu config option & cleanup tests (#1326)

Author: james-bebbington

receiver/hostmetricsreceiver/config_test.go

@@ -32,6 +32,7 @@ import (
 	"go.opentelemetry.io/collector/receiver/hostmetricsreceiver/internal/scraper/loadscraper"
 	"go.opentelemetry.io/collector/receiver/hostmetricsreceiver/internal/scraper/memoryscraper"
 	"go.opentelemetry.io/collector/receiver/hostmetricsreceiver/internal/scraper/networkscraper"
+	"go.opentelemetry.io/collector/receiver/hostmetricsreceiver/internal/scraper/processesscraper"
 	"go.opentelemetry.io/collector/receiver/hostmetricsreceiver/internal/scraper/processscraper"
 	"go.opentelemetry.io/collector/receiver/hostmetricsreceiver/internal/scraper/swapscraper"
 )
@@ -65,21 +66,20 @@ func TestLoadConfig(t *testing.T) {
 		},
 		CollectionInterval: 30 * time.Second,
 		Scrapers: map[string]internal.Config{
-			cpuscraper.TypeStr: &cpuscraper.Config{
-				ReportPerCPU: true,
-			},
+			cpuscraper.TypeStr:        &cpuscraper.Config{},
 			diskscraper.TypeStr:       &diskscraper.Config{},
 			loadscraper.TypeStr:       &loadscraper.Config{},
 			filesystemscraper.TypeStr: &filesystemscraper.Config{},
 			memoryscraper.TypeStr:     &memoryscraper.Config{},
 			networkscraper.TypeStr:    &networkscraper.Config{},
+			processesscraper.TypeStr:  &processesscraper.Config{},
+			swapscraper.TypeStr:       &swapscraper.Config{},
 			processscraper.TypeStr: &processscraper.Config{
 				Include: processscraper.MatchConfig{
 					Names:  []string{"test1", "test2"},
 					Config: filterset.Config{MatchType: "regexp"},
 				},
 			},
-			swapscraper.TypeStr: &swapscraper.Config{},
 		},
 	}
 

receiver/hostmetricsreceiver/hostmetrics_receiver_test.go

@@ -95,7 +95,7 @@ func TestGatherMetrics_EndToEnd(t *testing.T) {
 	config := &Config{
 		CollectionInterval: 100 * time.Millisecond,
 		Scrapers: map[string]internal.Config{
-			cpuscraper.TypeStr:        &cpuscraper.Config{ReportPerCPU: true},
+			cpuscraper.TypeStr:        &cpuscraper.Config{},
 			diskscraper.TypeStr:       &diskscraper.Config{},
 			filesystemscraper.TypeStr: &filesystemscraper.Config{},
 			loadscraper.TypeStr:       &loadscraper.Config{},
@@ -323,7 +323,7 @@ func Benchmark_ScrapeSwapMetrics(b *testing.B) {
 	benchmarkScrapeMetrics(b, cfg)
 }
 
-func Benchmark_ScrapeDefaultMetrics(b *testing.B) {
+func Benchmark_ScrapeSystemMetrics(b *testing.B) {
 	cfg := &Config{
 		Scrapers: map[string]internal.Config{
 			cpuscraper.TypeStr:        (&cpuscraper.Factory{}).CreateDefaultConfig(),
@@ -340,10 +340,10 @@ func Benchmark_ScrapeDefaultMetrics(b *testing.B) {
 	benchmarkScrapeMetrics(b, cfg)
 }
 
-func Benchmark_ScrapeAllMetrics(b *testing.B) {
+func Benchmark_ScrapeSystemAndProcessMetrics(b *testing.B) {
 	cfg := &Config{
 		Scrapers: map[string]internal.Config{
-			cpuscraper.TypeStr:        &cpuscraper.Config{ReportPerCPU: true},
+			cpuscraper.TypeStr:        &cpuscraper.Config{},
 			diskscraper.TypeStr:       &diskscraper.Config{},
 			filesystemscraper.TypeStr: &filesystemscraper.Config{},
 			loadscraper.TypeStr:       &loadscraper.Config{},

receiver/hostmetricsreceiver/internal/scraper/cpuscraper/config.go

@@ -19,10 +19,4 @@ import "go.opentelemetry.io/collector/receiver/hostmetricsreceiver/internal"
 // Config relating to CPU Metric Scraper.
 type Config struct {
 	internal.ConfigSettings `mapstructure:",squash"` // squash ensures fields are correctly decoded in embedded struct
-
-	// If `true`, stats will be generated for the system as a whole _as well
-	// as_ for each individual CPU/core in the system and will be distinguished
-	// by the `cpu` dimension.  If `false`, stats will only be generated for
-	// the system as a whole that will not include a `cpu` dimension.
-	ReportPerCPU bool `mapstructure:"report_per_cpu"`
 }

receiver/hostmetricsreceiver/internal/scraper/cpuscraper/cpu_scraper.go

@@ -28,11 +28,14 @@ import (
 type scraper struct {
 	config    *Config
 	startTime pdata.TimestampUnixNano
+
+	// for mocking gopsutil cpu.Times
+	times func(bool) ([]cpu.TimesStat, error)
 }
 
 // newCPUScraper creates a set of CPU related metrics
 func newCPUScraper(_ context.Context, cfg *Config) *scraper {
-	return &scraper{config: cfg}
+	return &scraper{config: cfg, times: cpu.Times}
 }
 
 // Initialize
@@ -55,7 +58,7 @@ func (s *scraper) Close(_ context.Context) error {
 func (s *scraper) ScrapeMetrics(_ context.Context) (pdata.MetricSlice, error) {
 	metrics := pdata.NewMetricSlice()
 
-	cpuTimes, err := cpu.Times(s.config.ReportPerCPU)
+	cpuTimes, err := s.times( /*percpu=*/ true)
 	if err != nil {
 		return metrics, err
 	}

receiver/hostmetricsreceiver/internal/scraper/cpuscraper/cpu_scraper_test.go

@@ -16,86 +16,78 @@ package cpuscraper
 
 import (
 	"context"
+	"errors"
 	"runtime"
 	"testing"
 
+	"github.com/shirou/gopsutil/cpu"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
 	"go.opentelemetry.io/collector/consumer/pdata"
 	"go.opentelemetry.io/collector/receiver/hostmetricsreceiver/internal"
 )
 
-type validationFn func(*testing.T, pdata.MetricSlice)
+func TestScrapeMetrics(t *testing.T) {
+	type testCase struct {
+		name        string
+		timesFunc   func(bool) ([]cpu.TimesStat, error)
+		expectedErr string
+	}
 
-func TestScrapeMetrics_MinimalData(t *testing.T) {
-	createScraperAndValidateScrapedMetrics(t, &Config{}, func(t *testing.T, metrics pdata.MetricSlice) {
-		// expect 1 metric
-		assert.Equal(t, 1, metrics.Len())
+	testCases := []testCase{
+		{
+			name: "Standard",
+		},
+		{
+			name:        "Error",
+			timesFunc:   func(bool) ([]cpu.TimesStat, error) { return nil, errors.New("err1") },
+			expectedErr: "err1",
+		},
+	}
 
-		// for cpu seconds metric, expect a datapoint for each state label, including at least 4 standard states
-		cpuTimeMetric := metrics.At(0)
-		internal.AssertDescriptorEqual(t, cpuTimeDescriptor, cpuTimeMetric.MetricDescriptor())
-		assert.GreaterOrEqual(t, cpuTimeMetric.DoubleDataPoints().Len(), 4)
-		internal.AssertDoubleMetricLabelDoesNotExist(t, cpuTimeMetric, 0, cpuLabelName)
-		internal.AssertDoubleMetricLabelHasValue(t, cpuTimeMetric, 0, stateLabelName, userStateLabelValue)
-		internal.AssertDoubleMetricLabelHasValue(t, cpuTimeMetric, 1, stateLabelName, systemStateLabelValue)
-		internal.AssertDoubleMetricLabelHasValue(t, cpuTimeMetric, 2, stateLabelName, idleStateLabelValue)
-		internal.AssertDoubleMetricLabelHasValue(t, cpuTimeMetric, 3, stateLabelName, interruptStateLabelValue)
-	})
-}
+	for _, test := range testCases {
+		t.Run(test.name, func(t *testing.T) {
+			scraper := newCPUScraper(context.Background(), &Config{})
+			if test.timesFunc != nil {
+				scraper.times = test.timesFunc
+			}
 
-func TestScrapeMetrics_AllData(t *testing.T) {
-	config := &Config{
-		ReportPerCPU: true,
-	}
+			err := scraper.Initialize(context.Background())
+			require.NoError(t, err, "Failed to initialize cpu scraper: %v", err)
+			defer func() { assert.NoError(t, scraper.Close(context.Background())) }()
 
-	createScraperAndValidateScrapedMetrics(t, config, func(t *testing.T, metrics pdata.MetricSlice) {
-		// expect 1 metric
-		assert.Equal(t, 1, metrics.Len())
+			metrics, err := scraper.ScrapeMetrics(context.Background())
+			if test.expectedErr != "" {
+				assert.EqualError(t, err, test.expectedErr)
+				return
+			}
+			require.NoError(t, err, "Failed to scrape metrics: %v", err)
 
-		// for cpu seconds metric, expect a datapoint for each state label & core combination with at least 4 standard states
-		cpuTimeMetric := metrics.At(0)
-		internal.AssertDescriptorEqual(t, cpuTimeDescriptor, cpuTimeMetric.MetricDescriptor())
-		assert.GreaterOrEqual(t, cpuTimeMetric.DoubleDataPoints().Len(), runtime.NumCPU()*4)
-		internal.AssertDoubleMetricLabelExists(t, cpuTimeMetric, 0, cpuLabelName)
-		internal.AssertDoubleMetricLabelHasValue(t, cpuTimeMetric, 0, stateLabelName, userStateLabelValue)
-		internal.AssertDoubleMetricLabelHasValue(t, cpuTimeMetric, 1, stateLabelName, systemStateLabelValue)
-		internal.AssertDoubleMetricLabelHasValue(t, cpuTimeMetric, 2, stateLabelName, idleStateLabelValue)
-		internal.AssertDoubleMetricLabelHasValue(t, cpuTimeMetric, 3, stateLabelName, interruptStateLabelValue)
-	})
-}
+			assert.Equal(t, 1, metrics.Len())
 
-func TestScrapeMetrics_Linux(t *testing.T) {
-	if runtime.GOOS != "linux" {
-		return
-	}
+			assertCPUMetricValid(t, metrics.At(0), cpuTimeDescriptor)
 
-	createScraperAndValidateScrapedMetrics(t, &Config{}, func(t *testing.T, metrics pdata.MetricSlice) {
-		// for cpu seconds metric, expect a datapoint for all 8 state labels
-		cpuTimeMetric := metrics.At(0)
-		internal.AssertDescriptorEqual(t, cpuTimeDescriptor, cpuTimeMetric.MetricDescriptor())
-		assert.Equal(t, 8, cpuTimeMetric.DoubleDataPoints().Len())
-		internal.AssertDoubleMetricLabelDoesNotExist(t, cpuTimeMetric, 0, cpuLabelName)
-		internal.AssertDoubleMetricLabelHasValue(t, cpuTimeMetric, 0, stateLabelName, userStateLabelValue)
-		internal.AssertDoubleMetricLabelHasValue(t, cpuTimeMetric, 1, stateLabelName, systemStateLabelValue)
-		internal.AssertDoubleMetricLabelHasValue(t, cpuTimeMetric, 2, stateLabelName, idleStateLabelValue)
-		internal.AssertDoubleMetricLabelHasValue(t, cpuTimeMetric, 3, stateLabelName, interruptStateLabelValue)
-		internal.AssertDoubleMetricLabelHasValue(t, cpuTimeMetric, 4, stateLabelName, niceStateLabelValue)
-		internal.AssertDoubleMetricLabelHasValue(t, cpuTimeMetric, 5, stateLabelName, softIRQStateLabelValue)
-		internal.AssertDoubleMetricLabelHasValue(t, cpuTimeMetric, 6, stateLabelName, stealStateLabelValue)
-		internal.AssertDoubleMetricLabelHasValue(t, cpuTimeMetric, 7, stateLabelName, waitStateLabelValue)
-	})
+			if runtime.GOOS == "linux" {
+				assertCPUMetricHasLinuxSpecificStateLabels(t, metrics.At(0))
+			}
+		})
+	}
 }
 
-func createScraperAndValidateScrapedMetrics(t *testing.T, config *Config, assertFn validationFn) {
-	scraper := newCPUScraper(context.Background(), config)
-	err := scraper.Initialize(context.Background())
-	require.NoError(t, err, "Failed to initialize cpu scraper: %v", err)
-	defer func() { assert.NoError(t, scraper.Close(context.Background())) }()
-
-	metrics, err := scraper.ScrapeMetrics(context.Background())
-	require.NoError(t, err, "Failed to scrape metrics: %v", err)
+func assertCPUMetricValid(t *testing.T, metric pdata.Metric, descriptor pdata.MetricDescriptor) {
+	internal.AssertDescriptorEqual(t, descriptor, metric.MetricDescriptor())
+	assert.GreaterOrEqual(t, metric.DoubleDataPoints().Len(), 4*runtime.NumCPU())
+	internal.AssertDoubleMetricLabelExists(t, metric, 0, cpuLabelName)
+	internal.AssertDoubleMetricLabelHasValue(t, metric, 0, stateLabelName, userStateLabelValue)
+	internal.AssertDoubleMetricLabelHasValue(t, metric, 1, stateLabelName, systemStateLabelValue)
+	internal.AssertDoubleMetricLabelHasValue(t, metric, 2, stateLabelName, idleStateLabelValue)
+	internal.AssertDoubleMetricLabelHasValue(t, metric, 3, stateLabelName, interruptStateLabelValue)
+}
 
-	assertFn(t, metrics)
+func assertCPUMetricHasLinuxSpecificStateLabels(t *testing.T, metric pdata.Metric) {
+	internal.AssertDoubleMetricLabelHasValue(t, metric, 4, stateLabelName, niceStateLabelValue)
+	internal.AssertDoubleMetricLabelHasValue(t, metric, 5, stateLabelName, softIRQStateLabelValue)
+	internal.AssertDoubleMetricLabelHasValue(t, metric, 6, stateLabelName, stealStateLabelValue)
+	internal.AssertDoubleMetricLabelHasValue(t, metric, 7, stateLabelName, waitStateLabelValue)
 }

receiver/hostmetricsreceiver/internal/scraper/cpuscraper/factory_test.go

@@ -26,7 +26,6 @@ func TestCreateDefaultConfig(t *testing.T) {
 	factory := &Factory{}
 	cfg := factory.CreateDefaultConfig()
 	assert.IsType(t, &Config{}, cfg)
-	assert.Equal(t, false, cfg.(*Config).ReportPerCPU)
 }
 
 func TestCreateMetricsScraper(t *testing.T) {

receiver/hostmetricsreceiver/internal/testutils.go

@@ -55,13 +55,3 @@ func AssertDoubleMetricLabelExists(t *testing.T, metric pdata.Metric, index int,
 	_, ok := metric.DoubleDataPoints().At(index).LabelsMap().Get(labelName)
 	assert.Truef(t, ok, "Missing label %q in metric %q", labelName, metric.MetricDescriptor().Name())
 }
-
-func AssertInt64MetricLabelDoesNotExist(t *testing.T, metric pdata.Metric, index int, labelName string) {
-	_, ok := metric.Int64DataPoints().At(index).LabelsMap().Get(labelName)
-	assert.Falsef(t, ok, "Unexpected label %q in metric %q", labelName, metric.MetricDescriptor().Name())
-}
-
-func AssertDoubleMetricLabelDoesNotExist(t *testing.T, metric pdata.Metric, index int, labelName string) {
-	_, ok := metric.DoubleDataPoints().At(index).LabelsMap().Get(labelName)
-	assert.Falsef(t, ok, "Unexpected label %q in metric %q", labelName, metric.MetricDescriptor().Name())
-}

receiver/hostmetricsreceiver/testdata/config.yaml

@@ -6,12 +6,12 @@ receivers:
     collection_interval: 30s
     scrapers:
       cpu:
-        report_per_cpu: true
       disk:
       load:
       filesystem:
       memory:
       network:
+      processes:
       swap:
       process:
         include: