From cab15612ebe1e7452a7f1e0911f72e732093a4ef Mon Sep 17 00:00:00 2001 From: James Bebbington Date: Tue, 14 Jul 2020 00:58:25 +1000 Subject: [PATCH] Disable process scraper for any non Linux/Windows OS (#1328) --- .../hostmetricsreceiver/example_config.yaml | 1 + .../hostmetrics_receiver.go | 5 +- .../hostmetrics_receiver_test.go | 97 ++++++++++++++----- .../scraper/processscraper/factory.go | 6 ++ .../scraper/processscraper/factory_test.go | 10 +- .../processscraper/process_scraper_linux.go | 30 ++++++ .../process_scraper_notwindows.go | 47 --------- .../processscraper/process_scraper_others.go | 12 ++- .../processscraper/process_scraper_test.go | 5 +- 9 files changed, 134 insertions(+), 79 deletions(-) delete mode 100644 receiver/hostmetricsreceiver/internal/scraper/processscraper/process_scraper_notwindows.go diff --git a/receiver/hostmetricsreceiver/example_config.yaml b/receiver/hostmetricsreceiver/example_config.yaml index 5086ac98151..2ee28b7bfd6 100644 --- a/receiver/hostmetricsreceiver/example_config.yaml +++ b/receiver/hostmetricsreceiver/example_config.yaml @@ -12,6 +12,7 @@ receivers: disk: filesystem: network: + processes: swap: exporters: diff --git a/receiver/hostmetricsreceiver/hostmetrics_receiver.go b/receiver/hostmetricsreceiver/hostmetrics_receiver.go index e9336955937..15bd6975019 100644 --- a/receiver/hostmetricsreceiver/hostmetrics_receiver.go +++ b/receiver/hostmetricsreceiver/hostmetrics_receiver.go @@ -19,7 +19,6 @@ import ( "fmt" "time" - "github.com/pkg/errors" "go.opencensus.io/trace" "go.uber.org/zap" @@ -58,7 +57,7 @@ func newHostMetricsReceiver( for key, cfg := range config.Scrapers { hostMetricsScraper, ok, err := createHostMetricsScraper(ctx, logger, key, cfg, factories) if err != nil { - errors.Wrapf(err, "failed to create scraper for key %q", key) + return nil, fmt.Errorf("failed to create scraper for key %q: %w", key, err) } if ok { @@ -68,7 +67,7 @@ func newHostMetricsReceiver( resourceMetricsScraper, ok, err := createResourceMetricsScraper(ctx, logger, key, cfg, resourceFactories) if err != nil { - errors.Wrapf(err, "failed to create resource scraper for key %q", key) + return nil, fmt.Errorf("failed to create resource scraper for key %q: %w", key, err) } if ok { diff --git a/receiver/hostmetricsreceiver/hostmetrics_receiver_test.go b/receiver/hostmetricsreceiver/hostmetrics_receiver_test.go index 7b591c487ee..da781056f78 100644 --- a/receiver/hostmetricsreceiver/hostmetrics_receiver_test.go +++ b/receiver/hostmetricsreceiver/hostmetrics_receiver_test.go @@ -22,6 +22,7 @@ import ( "time" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" "go.uber.org/zap" @@ -103,10 +104,13 @@ func TestGatherMetrics_EndToEnd(t *testing.T) { networkscraper.TypeStr: &networkscraper.Config{}, processesscraper.TypeStr: &processesscraper.Config{}, swapscraper.TypeStr: &swapscraper.Config{}, - processscraper.TypeStr: &processscraper.Config{}, }, } + if runtime.GOOS == "linux" || runtime.GOOS == "windows" { + config.Scrapers[processscraper.TypeStr] = &processscraper.Config{} + } + receiver, err := newHostMetricsReceiver(context.Background(), zap.NewNop(), config, factories, resourceFactories, sink) require.NoError(t, err, "Failed to create metrics receiver: %v", err) @@ -120,7 +124,7 @@ func TestGatherMetrics_EndToEnd(t *testing.T) { cancelFn() const tick = 50 * time.Millisecond - const waitFor = 5 * time.Second + const waitFor = time.Second require.Eventuallyf(t, func() bool { got := sink.AllMetrics() if len(got) == 0 { @@ -154,6 +158,10 @@ func assertIncludesStandardMetrics(t *testing.T, got pdata.Metrics) { } func assertIncludesResourceMetrics(t *testing.T, got pdata.Metrics) { + if runtime.GOOS != "linux" && runtime.GOOS != "windows" { + return + } + md := pdatautil.MetricsToInternalMetrics(got) // get the superset of metrics returned by all resource metrics (excluding the first) @@ -197,31 +205,33 @@ const mockResourceTypeStr = "mockresource" type mockConfig struct{} -type mockFactory struct{} -type mockScraper struct{} +type mockFactory struct{ mock.Mock } +type mockScraper struct{ mock.Mock } -func (*mockFactory) CreateDefaultConfig() internal.Config { return &mockConfig{} } -func (*mockFactory) CreateMetricsScraper(ctx context.Context, logger *zap.Logger, cfg internal.Config) (internal.Scraper, error) { - return &mockScraper{}, nil +func (m *mockFactory) CreateDefaultConfig() internal.Config { return &mockConfig{} } +func (m *mockFactory) CreateMetricsScraper(ctx context.Context, logger *zap.Logger, cfg internal.Config) (internal.Scraper, error) { + args := m.MethodCalled("CreateMetricsScraper") + return args.Get(0).(internal.Scraper), args.Error(1) } -func (*mockScraper) Initialize(ctx context.Context) error { return nil } -func (*mockScraper) Close(ctx context.Context) error { return nil } -func (*mockScraper) ScrapeMetrics(ctx context.Context) (pdata.MetricSlice, error) { +func (m *mockScraper) Initialize(ctx context.Context) error { return nil } +func (m *mockScraper) Close(ctx context.Context) error { return nil } +func (m *mockScraper) ScrapeMetrics(ctx context.Context) (pdata.MetricSlice, error) { return pdata.NewMetricSlice(), errors.New("err1") } -type mockResourceFactory struct{} -type mockResourceScraper struct{} +type mockResourceFactory struct{ mock.Mock } +type mockResourceScraper struct{ mock.Mock } -func (*mockResourceFactory) CreateDefaultConfig() internal.Config { return &mockConfig{} } -func (*mockResourceFactory) CreateMetricsScraper(ctx context.Context, logger *zap.Logger, cfg internal.Config) (internal.ResourceScraper, error) { - return &mockResourceScraper{}, nil +func (m *mockResourceFactory) CreateDefaultConfig() internal.Config { return &mockConfig{} } +func (m *mockResourceFactory) CreateMetricsScraper(ctx context.Context, logger *zap.Logger, cfg internal.Config) (internal.ResourceScraper, error) { + args := m.MethodCalled("CreateMetricsScraper") + return args.Get(0).(internal.ResourceScraper), args.Error(1) } -func (*mockResourceScraper) Initialize(ctx context.Context) error { return nil } -func (*mockResourceScraper) Close(ctx context.Context) error { return nil } -func (*mockResourceScraper) ScrapeMetrics(ctx context.Context) (pdata.ResourceMetricsSlice, error) { +func (m *mockResourceScraper) Initialize(ctx context.Context) error { return nil } +func (m *mockResourceScraper) Close(ctx context.Context) error { return nil } +func (m *mockResourceScraper) ScrapeMetrics(ctx context.Context) (pdata.ResourceMetricsSlice, error) { return pdata.NewResourceMetricsSlice(), errors.New("err2") } @@ -236,9 +246,38 @@ func TestGatherMetrics_ScraperKeyConfigError(t *testing.T) { require.Error(t, err) } +func TestGatherMetrics_CreateMetricsScraperError(t *testing.T) { + mFactory := &mockFactory{} + mFactory.On("CreateMetricsScraper").Return(&mockScraper{}, errors.New("err1")) + var mockFactories = map[string]internal.ScraperFactory{mockTypeStr: mFactory} + var mockResourceFactories = map[string]internal.ResourceScraperFactory{} + + sink := &exportertest.SinkMetricsExporter{} + config := &Config{Scrapers: map[string]internal.Config{mockTypeStr: &mockConfig{}}} + _, err := newHostMetricsReceiver(context.Background(), zap.NewNop(), config, mockFactories, mockResourceFactories, sink) + require.Error(t, err) +} + +func TestGatherMetrics_CreateMetricsResourceScraperError(t *testing.T) { + mResourceFactory := &mockResourceFactory{} + mResourceFactory.On("CreateMetricsScraper").Return(&mockResourceScraper{}, errors.New("err1")) + var mockFactories = map[string]internal.ScraperFactory{} + var mockResourceFactories = map[string]internal.ResourceScraperFactory{mockTypeStr: mResourceFactory} + + sink := &exportertest.SinkMetricsExporter{} + config := &Config{Scrapers: map[string]internal.Config{mockTypeStr: &mockConfig{}}} + _, err := newHostMetricsReceiver(context.Background(), zap.NewNop(), config, mockFactories, mockResourceFactories, sink) + require.Error(t, err) +} + func TestGatherMetrics_Error(t *testing.T) { - var mockFactories = map[string]internal.ScraperFactory{mockTypeStr: &mockFactory{}} - var mockResourceFactories = map[string]internal.ResourceScraperFactory{mockResourceTypeStr: &mockResourceFactory{}} + mFactory := &mockFactory{} + mFactory.On("CreateMetricsScraper").Return(&mockScraper{}, nil) + mResourceFactory := &mockResourceFactory{} + mResourceFactory.On("CreateMetricsScraper").Return(&mockResourceScraper{}, nil) + + var mockFactories = map[string]internal.ScraperFactory{mockTypeStr: mFactory} + var mockResourceFactories = map[string]internal.ResourceScraperFactory{mockResourceTypeStr: mResourceFactory} sink := &exportertest.SinkMetricsExporter{} @@ -313,8 +352,8 @@ func Benchmark_ScrapeNetworkMetrics(b *testing.B) { benchmarkScrapeMetrics(b, cfg) } -func Benchmark_ScrapeProcessMetrics(b *testing.B) { - cfg := &Config{Scrapers: map[string]internal.Config{processscraper.TypeStr: (&processscraper.Factory{}).CreateDefaultConfig()}} +func Benchmark_ScrapeProcessesMetrics(b *testing.B) { + cfg := &Config{Scrapers: map[string]internal.Config{processesscraper.TypeStr: (&processesscraper.Factory{}).CreateDefaultConfig()}} benchmarkScrapeMetrics(b, cfg) } @@ -323,6 +362,15 @@ func Benchmark_ScrapeSwapMetrics(b *testing.B) { benchmarkScrapeMetrics(b, cfg) } +func Benchmark_ScrapeProcessMetrics(b *testing.B) { + if runtime.GOOS != "linux" && runtime.GOOS != "windows" { + b.Skip("skipping test on non linux/windows") + } + + cfg := &Config{Scrapers: map[string]internal.Config{processscraper.TypeStr: (&processscraper.Factory{}).CreateDefaultConfig()}} + benchmarkScrapeMetrics(b, cfg) +} + func Benchmark_ScrapeSystemMetrics(b *testing.B) { cfg := &Config{ Scrapers: map[string]internal.Config{ @@ -351,9 +399,12 @@ func Benchmark_ScrapeSystemAndProcessMetrics(b *testing.B) { networkscraper.TypeStr: &networkscraper.Config{}, processesscraper.TypeStr: &processesscraper.Config{}, swapscraper.TypeStr: &swapscraper.Config{}, - processscraper.TypeStr: &processscraper.Config{}, }, } + if runtime.GOOS == "linux" || runtime.GOOS == "windows" { + cfg.Scrapers[processscraper.TypeStr] = &processscraper.Config{} + } + benchmarkScrapeMetrics(b, cfg) } diff --git a/receiver/hostmetricsreceiver/internal/scraper/processscraper/factory.go b/receiver/hostmetricsreceiver/internal/scraper/processscraper/factory.go index 343a079f209..4a96706c526 100644 --- a/receiver/hostmetricsreceiver/internal/scraper/processscraper/factory.go +++ b/receiver/hostmetricsreceiver/internal/scraper/processscraper/factory.go @@ -16,6 +16,8 @@ package processscraper import ( "context" + "errors" + "runtime" "go.uber.org/zap" @@ -45,6 +47,10 @@ func (f *Factory) CreateMetricsScraper( _ *zap.Logger, config internal.Config, ) (internal.ResourceScraper, error) { + if runtime.GOOS != "linux" && runtime.GOOS != "windows" { + return nil, errors.New("process scraper only available on Linux or Windows") + } + cfg := config.(*Config) ps, err := newProcessScraper(cfg) if err != nil { diff --git a/receiver/hostmetricsreceiver/internal/scraper/processscraper/factory_test.go b/receiver/hostmetricsreceiver/internal/scraper/processscraper/factory_test.go index 3b8c438d990..d05e1bf8cfe 100644 --- a/receiver/hostmetricsreceiver/internal/scraper/processscraper/factory_test.go +++ b/receiver/hostmetricsreceiver/internal/scraper/processscraper/factory_test.go @@ -16,6 +16,7 @@ package processscraper import ( "context" + "runtime" "testing" "github.com/stretchr/testify/assert" @@ -34,6 +35,11 @@ func TestCreateMetricsScraper(t *testing.T) { scraper, err := factory.CreateMetricsScraper(context.Background(), zap.NewNop(), cfg) - assert.NoError(t, err) - assert.NotNil(t, scraper) + if runtime.GOOS == "linux" || runtime.GOOS == "windows" { + assert.NoError(t, err) + assert.NotNil(t, scraper) + } else { + assert.Error(t, err) + assert.Nil(t, scraper) + } } diff --git a/receiver/hostmetricsreceiver/internal/scraper/processscraper/process_scraper_linux.go b/receiver/hostmetricsreceiver/internal/scraper/processscraper/process_scraper_linux.go index 4b990010de7..c2bc6be62d0 100644 --- a/receiver/hostmetricsreceiver/internal/scraper/processscraper/process_scraper_linux.go +++ b/receiver/hostmetricsreceiver/internal/scraper/processscraper/process_scraper_linux.go @@ -29,3 +29,33 @@ func appendCPUTimeStateDataPoints(ddps pdata.DoubleDataPointSlice, startTime pda initializeCPUTimeDataPoint(ddps.At(1), startTime, cpuTime.System, systemStateLabelValue) initializeCPUTimeDataPoint(ddps.At(2), startTime, cpuTime.Iowait, waitStateLabelValue) } + +func getProcessExecutable(proc processHandle) (*executableMetadata, error) { + name, err := proc.Name() + if err != nil { + return nil, err + } + + exe, err := proc.Exe() + if err != nil { + return nil, err + } + + executable := &executableMetadata{name: name, path: exe} + return executable, nil +} + +func getProcessCommand(proc processHandle) (*commandMetadata, error) { + cmdline, err := proc.CmdlineSlice() + if err != nil { + return nil, err + } + + var cmd string + if len(cmdline) > 0 { + cmd = cmdline[0] + } + + command := &commandMetadata{command: cmd, commandLineSlice: cmdline} + return command, nil +} diff --git a/receiver/hostmetricsreceiver/internal/scraper/processscraper/process_scraper_notwindows.go b/receiver/hostmetricsreceiver/internal/scraper/processscraper/process_scraper_notwindows.go deleted file mode 100644 index 02faf9f6ec3..00000000000 --- a/receiver/hostmetricsreceiver/internal/scraper/processscraper/process_scraper_notwindows.go +++ /dev/null @@ -1,47 +0,0 @@ -// Copyright 2020, OpenTelemetry Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -// +build !windows - -package processscraper - -func getProcessExecutable(proc processHandle) (*executableMetadata, error) { - name, err := proc.Name() - if err != nil { - return nil, err - } - - exe, err := proc.Exe() - if err != nil { - return nil, err - } - - executable := &executableMetadata{name: name, path: exe} - return executable, nil -} - -func getProcessCommand(proc processHandle) (*commandMetadata, error) { - cmdline, err := proc.CmdlineSlice() - if err != nil { - return nil, err - } - - var cmd string - if len(cmdline) > 0 { - cmd = cmdline[0] - } - - command := &commandMetadata{command: cmd, commandLineSlice: cmdline} - return command, nil -} diff --git a/receiver/hostmetricsreceiver/internal/scraper/processscraper/process_scraper_others.go b/receiver/hostmetricsreceiver/internal/scraper/processscraper/process_scraper_others.go index d9bc2deca24..46eedad5325 100644 --- a/receiver/hostmetricsreceiver/internal/scraper/processscraper/process_scraper_others.go +++ b/receiver/hostmetricsreceiver/internal/scraper/processscraper/process_scraper_others.go @@ -22,9 +22,15 @@ import ( "go.opentelemetry.io/collector/consumer/pdata" ) -const cpuStatesLen = 2 +const cpuStatesLen = 0 func appendCPUTimeStateDataPoints(ddps pdata.DoubleDataPointSlice, startTime pdata.TimestampUnixNano, cpuTime *cpu.TimesStat) { - initializeCPUTimeDataPoint(ddps.At(0), startTime, cpuTime.User, userStateLabelValue) - initializeCPUTimeDataPoint(ddps.At(1), startTime, cpuTime.System, systemStateLabelValue) +} + +func getProcessExecutable(proc processHandle) (*executableMetadata, error) { + return nil, nil +} + +func getProcessCommand(proc processHandle) (*commandMetadata, error) { + return nil, nil } diff --git a/receiver/hostmetricsreceiver/internal/scraper/processscraper/process_scraper_test.go b/receiver/hostmetricsreceiver/internal/scraper/processscraper/process_scraper_test.go index 6e7fc9f32fe..4c9699f5ccd 100644 --- a/receiver/hostmetricsreceiver/internal/scraper/processscraper/process_scraper_test.go +++ b/receiver/hostmetricsreceiver/internal/scraper/processscraper/process_scraper_test.go @@ -12,11 +12,14 @@ // See the License for the specific language governing permissions and // limitations under the License. +//+build linux windows + package processscraper import ( "context" "errors" + "fmt" "runtime" "strings" "testing" @@ -107,7 +110,7 @@ func getMetric(t *testing.T, descriptor pdata.MetricDescriptor, rms pdata.Resour } } - require.Failf(t, "no metric with name %s was returned", descriptor.Name()) + require.Fail(t, fmt.Sprintf("no metric with name %s was returned", descriptor.Name())) return pdata.NewMetric() }