From 2639e3a9d54278fc5bd826c4bcd4e06caa0db2c0 Mon Sep 17 00:00:00 2001 From: James Bebbington Date: Mon, 6 Jul 2020 17:55:18 +1000 Subject: [PATCH] Added disk merged metric for Linux & updated tests --- .../hostmetrics_receiver_test.go | 2 +- .../scraper/diskscraper/disk_metadata.go | 10 +++ .../scraper/diskscraper/disk_scraper.go | 22 +++--- .../scraper/diskscraper/disk_scraper_linux.go | 40 ++++++++++ .../diskscraper/disk_scraper_others.go | 28 +++++++ .../scraper/diskscraper/disk_scraper_test.go | 74 +++++++++++++------ 6 files changed, 142 insertions(+), 34 deletions(-) create mode 100644 receiver/hostmetricsreceiver/internal/scraper/diskscraper/disk_scraper_linux.go create mode 100644 receiver/hostmetricsreceiver/internal/scraper/diskscraper/disk_scraper_others.go diff --git a/receiver/hostmetricsreceiver/hostmetrics_receiver_test.go b/receiver/hostmetricsreceiver/hostmetrics_receiver_test.go index 5231abd27b3..9c45dd449b7 100644 --- a/receiver/hostmetricsreceiver/hostmetrics_receiver_test.go +++ b/receiver/hostmetricsreceiver/hostmetrics_receiver_test.go @@ -67,7 +67,7 @@ var resourceMetrics = []string{ } var systemSpecificMetrics = map[string][]string{ - "linux": {"system.filesystem.inodes.usage", "system.processes.running", "system.processes.blocked", "system.swap.page_faults"}, + "linux": {"system.disk.merged", "system.filesystem.inodes.usage", "system.processes.running", "system.processes.blocked", "system.swap.page_faults"}, "darwin": {"system.filesystem.inodes.usage", "system.processes.running", "system.processes.blocked", "system.swap.page_faults"}, "freebsd": {"system.filesystem.inodes.usage", "system.processes.running", "system.processes.blocked", "system.swap.page_faults"}, "openbsd": {"system.filesystem.inodes.usage", "system.processes.running", "system.processes.blocked", "system.swap.page_faults"}, diff --git a/receiver/hostmetricsreceiver/internal/scraper/diskscraper/disk_metadata.go b/receiver/hostmetricsreceiver/internal/scraper/diskscraper/disk_metadata.go index f0ea78aaf97..701f8350964 100644 --- a/receiver/hostmetricsreceiver/internal/scraper/diskscraper/disk_metadata.go +++ b/receiver/hostmetricsreceiver/internal/scraper/diskscraper/disk_metadata.go @@ -63,3 +63,13 @@ var diskTimeDescriptor = func() pdata.MetricDescriptor { descriptor.SetType(pdata.MetricTypeMonotonicInt64) return descriptor }() + +var diskMergedDescriptor = func() pdata.MetricDescriptor { + descriptor := pdata.NewMetricDescriptor() + descriptor.InitEmpty() + descriptor.SetName("system.disk.merged") + descriptor.SetDescription("The number of disk reads merged into single physical disk access operations.") + descriptor.SetUnit("1") + descriptor.SetType(pdata.MetricTypeMonotonicInt64) + return descriptor +}() diff --git a/receiver/hostmetricsreceiver/internal/scraper/diskscraper/disk_scraper.go b/receiver/hostmetricsreceiver/internal/scraper/diskscraper/disk_scraper.go index a8c33998c85..59dd48b2ed2 100644 --- a/receiver/hostmetricsreceiver/internal/scraper/diskscraper/disk_scraper.go +++ b/receiver/hostmetricsreceiver/internal/scraper/diskscraper/disk_scraper.go @@ -28,11 +28,14 @@ import ( type scraper struct { config *Config startTime pdata.TimestampUnixNano + + // for mocking gopsutil disk.IOCounters + ioCounters func(names ...string) (map[string]disk.IOCountersStat, error) } // newDiskScraper creates a Disk Scraper func newDiskScraper(_ context.Context, cfg *Config) *scraper { - return &scraper{config: cfg} + return &scraper{config: cfg, ioCounters: disk.IOCounters} } // Initialize @@ -55,19 +58,20 @@ func (s *scraper) Close(_ context.Context) error { func (s *scraper) ScrapeMetrics(_ context.Context) (pdata.MetricSlice, error) { metrics := pdata.NewMetricSlice() - ioCounters, err := disk.IOCounters() + ioCounters, err := s.ioCounters() if err != nil { return metrics, err } - metrics.Resize(3) - initializeDiskIOMetric(metrics.At(0), ioCounters, s.startTime) - initializeDiskOpsMetric(metrics.At(1), ioCounters, s.startTime) - initializeDiskTimeMetric(metrics.At(2), ioCounters, s.startTime) + metrics.Resize(3 + systemSpecificMetricsLen) + initializeDiskIOMetric(metrics.At(0), s.startTime, ioCounters) + initializeDiskOpsMetric(metrics.At(1), s.startTime, ioCounters) + initializeDiskTimeMetric(metrics.At(2), s.startTime, ioCounters) + appendSystemSpecificMetrics(metrics, 3, s.startTime, ioCounters) return metrics, nil } -func initializeDiskIOMetric(metric pdata.Metric, ioCounters map[string]disk.IOCountersStat, startTime pdata.TimestampUnixNano) { +func initializeDiskIOMetric(metric pdata.Metric, startTime pdata.TimestampUnixNano, ioCounters map[string]disk.IOCountersStat) { diskIODescriptor.CopyTo(metric.MetricDescriptor()) idps := metric.Int64DataPoints() @@ -81,7 +85,7 @@ func initializeDiskIOMetric(metric pdata.Metric, ioCounters map[string]disk.IOCo } } -func initializeDiskOpsMetric(metric pdata.Metric, ioCounters map[string]disk.IOCountersStat, startTime pdata.TimestampUnixNano) { +func initializeDiskOpsMetric(metric pdata.Metric, startTime pdata.TimestampUnixNano, ioCounters map[string]disk.IOCountersStat) { diskOpsDescriptor.CopyTo(metric.MetricDescriptor()) idps := metric.Int64DataPoints() @@ -95,7 +99,7 @@ func initializeDiskOpsMetric(metric pdata.Metric, ioCounters map[string]disk.IOC } } -func initializeDiskTimeMetric(metric pdata.Metric, ioCounters map[string]disk.IOCountersStat, startTime pdata.TimestampUnixNano) { +func initializeDiskTimeMetric(metric pdata.Metric, startTime pdata.TimestampUnixNano, ioCounters map[string]disk.IOCountersStat) { diskTimeDescriptor.CopyTo(metric.MetricDescriptor()) idps := metric.Int64DataPoints() diff --git a/receiver/hostmetricsreceiver/internal/scraper/diskscraper/disk_scraper_linux.go b/receiver/hostmetricsreceiver/internal/scraper/diskscraper/disk_scraper_linux.go new file mode 100644 index 00000000000..399cc283d59 --- /dev/null +++ b/receiver/hostmetricsreceiver/internal/scraper/diskscraper/disk_scraper_linux.go @@ -0,0 +1,40 @@ +// Copyright The 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 linux + +package diskscraper + +import ( + "github.com/shirou/gopsutil/disk" + + "go.opentelemetry.io/collector/consumer/pdata" +) + +const systemSpecificMetricsLen = 1 + +func appendSystemSpecificMetrics(metrics pdata.MetricSlice, startIdx int, startTime pdata.TimestampUnixNano, ioCounters map[string]disk.IOCountersStat) { + metric := metrics.At(startIdx) + diskMergedDescriptor.CopyTo(metric.MetricDescriptor()) + + idps := metric.Int64DataPoints() + idps.Resize(2 * len(ioCounters)) + + idx := 0 + for device, ioCounter := range ioCounters { + initializeDataPoint(idps.At(idx+0), startTime, device, readDirectionLabelValue, int64(ioCounter.MergedReadCount)) + initializeDataPoint(idps.At(idx+1), startTime, device, writeDirectionLabelValue, int64(ioCounter.MergedWriteCount)) + idx += 2 + } +} diff --git a/receiver/hostmetricsreceiver/internal/scraper/diskscraper/disk_scraper_others.go b/receiver/hostmetricsreceiver/internal/scraper/diskscraper/disk_scraper_others.go new file mode 100644 index 00000000000..6047da585e2 --- /dev/null +++ b/receiver/hostmetricsreceiver/internal/scraper/diskscraper/disk_scraper_others.go @@ -0,0 +1,28 @@ +// Copyright The 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 !linux + +package diskscraper + +import ( + "github.com/shirou/gopsutil/disk" + + "go.opentelemetry.io/collector/consumer/pdata" +) + +const systemSpecificMetricsLen = 0 + +func appendSystemSpecificMetrics(metrics pdata.MetricSlice, startIdx int, startTime pdata.TimestampUnixNano, ioCounters map[string]disk.IOCountersStat) { +} diff --git a/receiver/hostmetricsreceiver/internal/scraper/diskscraper/disk_scraper_test.go b/receiver/hostmetricsreceiver/internal/scraper/diskscraper/disk_scraper_test.go index 129e33d1a8b..1821a6130cb 100644 --- a/receiver/hostmetricsreceiver/internal/scraper/diskscraper/disk_scraper_test.go +++ b/receiver/hostmetricsreceiver/internal/scraper/diskscraper/disk_scraper_test.go @@ -16,8 +16,11 @@ package diskscraper import ( "context" + "errors" + "runtime" "testing" + "github.com/shirou/gopsutil/disk" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -25,35 +28,58 @@ import ( "go.opentelemetry.io/collector/receiver/hostmetricsreceiver/internal" ) -type validationFn func(*testing.T, pdata.MetricSlice) - func TestScrapeMetrics(t *testing.T) { - createScraperAndValidateScrapedMetrics(t, &Config{}, func(t *testing.T, metrics pdata.MetricSlice) { - // expect 3 metrics - assert.Equal(t, 3, metrics.Len()) - - // for each disk metric, expect a read & write datapoint for at least one drive - assertDiskMetricMatchesDescriptorAndHasReadAndWriteDataPoints(t, metrics.At(0), diskIODescriptor) - assertDiskMetricMatchesDescriptorAndHasReadAndWriteDataPoints(t, metrics.At(1), diskOpsDescriptor) - assertDiskMetricMatchesDescriptorAndHasReadAndWriteDataPoints(t, metrics.At(2), diskTimeDescriptor) - }) + type testCase struct { + name string + ioCountersFunc func(names ...string) (map[string]disk.IOCountersStat, error) + expectedErr string + } + + testCases := []testCase{ + { + name: "Standard", + }, + { + name: "Error", + ioCountersFunc: func(names ...string) (map[string]disk.IOCountersStat, error) { return nil, errors.New("err1") }, + expectedErr: "err1", + }, + } + + for _, test := range testCases { + t.Run(test.name, func(t *testing.T) { + scraper := newDiskScraper(context.Background(), &Config{}) + if test.ioCountersFunc != nil { + scraper.ioCounters = test.ioCountersFunc + } + + err := scraper.Initialize(context.Background()) + require.NoError(t, err, "Failed to initialize disk scraper: %v", err) + defer func() { assert.NoError(t, scraper.Close(context.Background())) }() + + 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) + + assert.GreaterOrEqual(t, metrics.Len(), 3) + + assertDiskMetricValid(t, metrics.At(0), diskIODescriptor) + assertDiskMetricValid(t, metrics.At(1), diskOpsDescriptor) + assertDiskMetricValid(t, metrics.At(2), diskTimeDescriptor) + + if runtime.GOOS == "Linux" { + assertDiskMetricValid(t, metrics.At(3), diskMergedDescriptor) + } + }) + } } -func assertDiskMetricMatchesDescriptorAndHasReadAndWriteDataPoints(t *testing.T, metric pdata.Metric, expectedDescriptor pdata.MetricDescriptor) { +func assertDiskMetricValid(t *testing.T, metric pdata.Metric, expectedDescriptor pdata.MetricDescriptor) { internal.AssertDescriptorEqual(t, expectedDescriptor, metric.MetricDescriptor()) assert.GreaterOrEqual(t, metric.Int64DataPoints().Len(), 2) internal.AssertInt64MetricLabelHasValue(t, metric, 0, directionLabelName, readDirectionLabelValue) internal.AssertInt64MetricLabelHasValue(t, metric, 1, directionLabelName, writeDirectionLabelValue) } - -func createScraperAndValidateScrapedMetrics(t *testing.T, config *Config, assertFn validationFn) { - scraper := newDiskScraper(context.Background(), config) - err := scraper.Initialize(context.Background()) - require.NoError(t, err, "Failed to initialize disk 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) - - assertFn(t, metrics) -}