From 854550bbc82e2fe451a7d87131172f83ce4189cd Mon Sep 17 00:00:00 2001 From: Dmitrii Anoshin Date: Tue, 5 Sep 2023 21:29:28 -0700 Subject: [PATCH] [chore] [receiver/hostmetrics] Don't collect data for disabled metric (#26474) If `system.network.connections` metric is disabled, don't collect the information from the host to not waste CPU cycles Fixes https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/25815 --- ...ont-collect-disabled-connections-data.yaml | 22 ++++ .../scraper/networkscraper/network_scraper.go | 4 + .../networkscraper/network_scraper_test.go | 110 +++++++++++------- 3 files changed, 94 insertions(+), 42 deletions(-) create mode 100644 .chloggen/hostmetrics-dont-collect-disabled-connections-data.yaml diff --git a/.chloggen/hostmetrics-dont-collect-disabled-connections-data.yaml b/.chloggen/hostmetrics-dont-collect-disabled-connections-data.yaml new file mode 100644 index 000000000000..92fa4cb7cde9 --- /dev/null +++ b/.chloggen/hostmetrics-dont-collect-disabled-connections-data.yaml @@ -0,0 +1,22 @@ +# Use this changelog template to create an entry for release notes. + +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: enhancement + +# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver) +component: receiver/hostmetrics + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Don't collect connections data from the host if system.network.connections metric is disabled to not waste CPU cycles. + +# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists. +issues: [25815] + +# If your change doesn't affect end users or the exported elements of any package, +# you should instead start your pull request title with [chore] or use the "Skip Changelog" label. +# Optional: The change log or logs in which this entry should be included. +# e.g. '[user]' or '[user, api]' +# Include 'user' if the change is relevant to end users. +# Include 'api' if there is a change to a library API. +# Default: '[user]' +change_logs: [user] diff --git a/receiver/hostmetricsreceiver/internal/scraper/networkscraper/network_scraper.go b/receiver/hostmetricsreceiver/internal/scraper/networkscraper/network_scraper.go index c014c4278aac..10eed34cb73b 100644 --- a/receiver/hostmetricsreceiver/internal/scraper/networkscraper/network_scraper.go +++ b/receiver/hostmetricsreceiver/internal/scraper/networkscraper/network_scraper.go @@ -157,6 +157,10 @@ func (s *scraper) recordNetworkIOMetric(now pcommon.Timestamp, ioCountersSlice [ } func (s *scraper) recordNetworkConnectionsMetrics() error { + if !s.config.Metrics.SystemNetworkConnections.Enabled { + return nil + } + ctx := context.WithValue(context.Background(), common.EnvKey, s.config.EnvMap) now := pcommon.NewTimestampFromTime(time.Now()) diff --git a/receiver/hostmetricsreceiver/internal/scraper/networkscraper/network_scraper_test.go b/receiver/hostmetricsreceiver/internal/scraper/networkscraper/network_scraper_test.go index 12ddcc75f90d..3be1aa5edd6f 100644 --- a/receiver/hostmetricsreceiver/internal/scraper/networkscraper/network_scraper_test.go +++ b/receiver/hostmetricsreceiver/internal/scraper/networkscraper/network_scraper_test.go @@ -24,19 +24,20 @@ import ( func TestScrape(t *testing.T) { type testCase struct { - name string - config Config - bootTimeFunc func(context.Context) (uint64, error) - ioCountersFunc func(context.Context, bool) ([]net.IOCountersStat, error) - connectionsFunc func(context.Context, string) ([]net.ConnectionStat, error) - conntrackFunc func(context.Context) ([]net.FilterStat, error) - expectNetworkMetrics bool - expectedStartTime pcommon.Timestamp - newErrRegex string - initializationErr string - expectedErr string - expectedErrCount int - mutateScraper func(*scraper) + name string + config Config + bootTimeFunc func(context.Context) (uint64, error) + ioCountersFunc func(context.Context, bool) ([]net.IOCountersStat, error) + connectionsFunc func(context.Context, string) ([]net.ConnectionStat, error) + conntrackFunc func(context.Context) ([]net.FilterStat, error) + expectConntrakMetrics bool + expectConnectionsMetric bool + expectedStartTime pcommon.Timestamp + newErrRegex string + initializationErr string + expectedErr string + expectedErrCount int + mutateScraper func(*scraper) } testCases := []testCase{ @@ -45,23 +46,26 @@ func TestScrape(t *testing.T) { config: Config{ MetricsBuilderConfig: metadata.DefaultMetricsBuilderConfig(), }, - expectNetworkMetrics: true, + expectConntrakMetrics: true, + expectConnectionsMetric: true, }, { name: "Standard with direction removed", config: Config{ MetricsBuilderConfig: metadata.DefaultMetricsBuilderConfig(), }, - expectNetworkMetrics: true, + expectConntrakMetrics: true, + expectConnectionsMetric: true, }, { name: "Validate Start Time", config: Config{ MetricsBuilderConfig: metadata.DefaultMetricsBuilderConfig(), }, - bootTimeFunc: func(context.Context) (uint64, error) { return 100, nil }, - expectNetworkMetrics: true, - expectedStartTime: 100 * 1e9, + bootTimeFunc: func(context.Context) (uint64, error) { return 100, nil }, + expectConntrakMetrics: true, + expectConnectionsMetric: true, + expectedStartTime: 100 * 1e9, }, { name: "Include Filter that matches nothing", @@ -69,7 +73,8 @@ func TestScrape(t *testing.T) { MetricsBuilderConfig: metadata.DefaultMetricsBuilderConfig(), Include: MatchConfig{filterset.Config{MatchType: "strict"}, []string{"@*^#&*$^#)"}}, }, - expectNetworkMetrics: false, + expectConntrakMetrics: false, + expectConnectionsMetric: true, }, { name: "Invalid Include Filter", @@ -77,7 +82,8 @@ func TestScrape(t *testing.T) { MetricsBuilderConfig: metadata.DefaultMetricsBuilderConfig(), Include: MatchConfig{Interfaces: []string{"test"}}, }, - newErrRegex: "^error creating network interface include filters:", + newErrRegex: "^error creating network interface include filters:", + expectConnectionsMetric: true, }, { name: "Invalid Exclude Filter", @@ -85,21 +91,25 @@ func TestScrape(t *testing.T) { MetricsBuilderConfig: metadata.DefaultMetricsBuilderConfig(), Exclude: MatchConfig{Interfaces: []string{"test"}}, }, - newErrRegex: "^error creating network interface exclude filters:", + newErrRegex: "^error creating network interface exclude filters:", + expectConnectionsMetric: true, }, { - name: "Boot Time Error", - bootTimeFunc: func(context.Context) (uint64, error) { return 0, errors.New("err1") }, - initializationErr: "err1", + name: "Boot Time Error", + bootTimeFunc: func(context.Context) (uint64, error) { return 0, errors.New("err1") }, + initializationErr: "err1", + expectConnectionsMetric: true, }, { - name: "IOCounters Error", - ioCountersFunc: func(context.Context, bool) ([]net.IOCountersStat, error) { return nil, errors.New("err2") }, - expectedErr: "failed to read network IO stats: err2", - expectedErrCount: networkMetricsLen, + name: "IOCounters Error", + ioCountersFunc: func(context.Context, bool) ([]net.IOCountersStat, error) { return nil, errors.New("err2") }, + expectedErr: "failed to read network IO stats: err2", + expectedErrCount: networkMetricsLen, + expectConnectionsMetric: true, }, { name: "Connections Error", + config: Config{MetricsBuilderConfig: metadata.DefaultMetricsBuilderConfig()}, connectionsFunc: func(context.Context, string) ([]net.ConnectionStat, error) { return nil, errors.New("err3") }, expectedErr: "failed to read TCP connections: err3", expectedErrCount: connectionsMetricsLen, @@ -109,8 +119,21 @@ func TestScrape(t *testing.T) { config: Config{ MetricsBuilderConfig: metadata.DefaultMetricsBuilderConfig(), // conntrack metrics are disabled by default }, - conntrackFunc: func(ctx context.Context) ([]net.FilterStat, error) { return nil, errors.New("conntrack failed") }, - expectNetworkMetrics: true, + conntrackFunc: func(ctx context.Context) ([]net.FilterStat, error) { return nil, errors.New("conntrack failed") }, + expectConntrakMetrics: true, + expectConnectionsMetric: true, + }, + { + name: "Connections metrics is disabled", + config: func() Config { + cfg := Config{MetricsBuilderConfig: metadata.DefaultMetricsBuilderConfig()} + cfg.MetricsBuilderConfig.Metrics.SystemNetworkConnections.Enabled = false + return cfg + }(), + connectionsFunc: func(ctx context.Context, s string) ([]net.ConnectionStat, error) { + panic("should not be called") + }, + expectConntrakMetrics: true, }, } @@ -163,27 +186,30 @@ func TestScrape(t *testing.T) { } require.NoError(t, err, "Failed to scrape metrics: %v", err) - expectedMetricCount := 1 - if test.expectNetworkMetrics { + expectedMetricCount := 0 + if test.expectConntrakMetrics { expectedMetricCount += 4 } + if test.expectConnectionsMetric { + expectedMetricCount++ + } assert.Equal(t, expectedMetricCount, md.MetricCount()) metrics := md.ResourceMetrics().At(0).ScopeMetrics().At(0).Metrics() idx := 0 - assertNetworkConnectionsMetricValid(t, metrics.At(idx)) - if test.expectNetworkMetrics { - assertNetworkIOMetricValid(t, metrics.At(idx+1), "system.network.dropped", + if test.expectConnectionsMetric { + assertNetworkConnectionsMetricValid(t, metrics.At(idx)) + idx++ + } + if test.expectConntrakMetrics { + assertNetworkIOMetricValid(t, metrics.At(idx), "system.network.dropped", test.expectedStartTime) - assertNetworkIOMetricValid(t, metrics.At(idx+2), "system.network.errors", test.expectedStartTime) - assertNetworkIOMetricValid(t, metrics.At(idx+3), "system.network.io", test.expectedStartTime) - assertNetworkIOMetricValid(t, metrics.At(idx+4), "system.network.packets", + assertNetworkIOMetricValid(t, metrics.At(idx+1), "system.network.errors", test.expectedStartTime) + assertNetworkIOMetricValid(t, metrics.At(idx+2), "system.network.io", test.expectedStartTime) + assertNetworkIOMetricValid(t, metrics.At(idx+3), "system.network.packets", test.expectedStartTime) - internal.AssertSameTimeStampForMetrics(t, metrics, 1, 5) - idx += 4 + internal.AssertSameTimeStampForMetrics(t, metrics, idx, idx+4) } - - internal.AssertSameTimeStampForMetrics(t, metrics, idx, idx+1) }) } }