Make disk-io montior default on Linux (#1142)

- Removing plugin_instance dimension from Linux part of the monitor - Make the monitor default in all distributed configs This monitor was beta so the breaking changes are acceptable in the 4.x release line.
signalfx · Jan 15, 2020 · fb7640e · fb7640e
1 parent 014e25c
commit fb7640e
Show file tree

Hide file tree

Showing 19 changed files with 49 additions and 27 deletions.
diff --git a/deployments/ansible/README.md b/deployments/ansible/README.md
@@ -36,7 +36,7 @@ This role sources the following variables:
         - type: collectd/cpu
         - type: collectd/cpufreq
         - type: collectd/df
-        - type: collectd/disk
+        - type: disk
         - type: collectd/interface
         - type: collectd/load
         - type: collectd/memory

diff --git a/deployments/ansible/example-config.yml b/deployments/ansible/example-config.yml
@@ -5,7 +5,7 @@ sfx_agent_config:
     - type: collectd/cpu
     - type: collectd/cpufreq
     - type: collectd/df
-    - type: collectd/disk
+    - type: disk
     - type: collectd/interface
     - type: collectd/load
     - type: collectd/memory

diff --git a/deployments/chef/README.md b/deployments/chef/README.md
@@ -51,7 +51,7 @@ node['signalfx_agent']['conf'] = {
     {type: "collectd/cpu"},
     {type: "collectd/cpufreq"},
     {type: "collectd/df"},
-    {type: "collectd/disk"},
+    {type: "disk"},
     {type: "collectd/interface"},
     {type: "collectd/load"},
     {type: "collectd/memory"},

diff --git a/deployments/chef/example_attrs.json b/deployments/chef/example_attrs.json
@@ -10,7 +10,7 @@
       {"type": "collectd/cpu"},
       {"type": "collectd/cpufreq"},
       {"type": "collectd/df"},
-      {"type": "collectd/disk"},
+      {"type": "disk"},
       {"type": "collectd/interface"},
       {"type": "collectd/load"},
       {"type": "collectd/memory"},

diff --git a/deployments/docker/agent.yaml b/deployments/docker/agent.yaml
@@ -25,7 +25,7 @@ monitors:
   - type: collectd/cpufreq
   - type: collectd/df
     hostFSPath: /hostfs
-  - type: collectd/disk
+  - type: disk
   - type: collectd/interface
   - type: collectd/load
   - type: collectd/memory

diff --git a/deployments/ecs/agent.yaml b/deployments/ecs/agent.yaml
@@ -39,7 +39,7 @@ monitors:
   - type: collectd/cpufreq
   - type: collectd/df
     hostFSPath: /hostfs
-  - type: collectd/disk
+  - type: disk
   - type: collectd/interface
   - type: collectd/load
   - type: collectd/memory

diff --git a/deployments/fargate/agent.yaml b/deployments/fargate/agent.yaml
@@ -19,7 +19,7 @@ observers:
 
 monitors:
   - type: collectd/cpu
-  - type: collectd/disk
+  - type: disk
   - type: collectd/interface
   - type: collectd/load
   - type: collectd/memory

diff --git a/deployments/k8s/configmap.yaml b/deployments/k8s/configmap.yaml
@@ -35,7 +35,7 @@ data:
     - type: collectd/cpufreq
     - type: collectd/df
       hostFSPath: /hostfs
-    - type: collectd/disk
+    - type: disk
     - type: collectd/interface
     - type: collectd/load
     - type: collectd/memory

diff --git a/deployments/k8s/helm/signalfx-agent/templates/configmap.yaml b/deployments/k8s/helm/signalfx-agent/templates/configmap.yaml
@@ -59,7 +59,7 @@ data:
     - type: collectd/cpufreq
     - type: collectd/df
       hostFSPath: {{ .Values.hostFSPath }}
-    - type: collectd/disk
+    - type: disk
     - type: collectd/interface
     - type: collectd/load
     - type: collectd/memory

diff --git a/deployments/puppet/README.md b/deployments/puppet/README.md
@@ -20,7 +20,7 @@ class accepts the following parameters:
         {type: "collectd/cpu"},
         {type: "collectd/cpufreq"},
         {type: "collectd/df"},
-        {type: "collectd/disk"},
+        {type: "disk"},
         {type: "collectd/interface"},
         {type: "collectd/load"},
         {type: "collectd/memory"},

diff --git a/deployments/salt/README.md b/deployments/salt/README.md
@@ -47,7 +47,7 @@ signalfx-agent:
       - type: collectd/cpu
       - type: collectd/cpufreq
       - type: collectd/df
-      - type: collectd/disk
+      - type: disk
       - type: collectd/interface
       - type: collectd/load
       - type: collectd/memory

diff --git a/deployments/salt/pillar.example b/deployments/salt/pillar.example
@@ -9,7 +9,7 @@ signalfx-agent:
       - type: collectd/cpu
       - type: collectd/cpufreq
       - type: collectd/df
-      - type: collectd/disk
+      - type: disk
       - type: collectd/interface
       - type: collectd/load
       - type: collectd/memory

diff --git a/docs/monitors/collectd-disk.md b/docs/monitors/collectd-disk.md
@@ -15,6 +15,12 @@ physical disks and logical disks (partitions).
 
 See https://collectd.org/wiki/index.php/Plugin:Disk.
 
+**This monitor has been deprecated in favor of the `disk-io` monitor.
+Please migrate to that monitor as this collectd-based monitor will be
+removed in a future release of the agent.**  Note that the `disk-io`
+monitor has a different dimension (`disk` instead of `plugin_instance`) to
+specify the disk.
+
 
 ## Configuration
 

diff --git a/docs/monitors/disk-io.md b/docs/monitors/disk-io.md
@@ -70,5 +70,14 @@ This monitor emits all metrics by default; however, **none are categorized as
  - ***`disk_time.write`*** (*cumulative*)<br>    (Linux Only) The average amount of time it took to do a write operation.
 The agent does not do any built-in filtering of metrics coming out of this
 monitor.
+## Dimensions
+
+The following dimensions may occur on metrics emitted by this monitor.  Some
+dimensions may be specific to certain metrics.
+
+| Name | Description |
+| ---  | ---         |
+| `disk` | The name of the disk that the metric describes |
+
 
 
diff --git a/packaging/etc/agent.yaml b/packaging/etc/agent.yaml
@@ -22,7 +22,7 @@ monitors:
   - type: collectd/cpu
   - type: collectd/cpufreq
   - type: collectd/df
-  - type: collectd/disk
+  - type: disk
   - type: collectd/interface
   - type: collectd/load
   - type: collectd/memory

diff --git a/pkg/monitors/collectd/disk/metadata.yaml b/pkg/monitors/collectd/disk/metadata.yaml
@@ -5,6 +5,12 @@ monitors:
     physical disks and logical disks (partitions).
 
     See https://collectd.org/wiki/index.php/Plugin:Disk.
+
+    **This monitor has been deprecated in favor of the `disk-io` monitor.
+    Please migrate to that monitor as this collectd-based monitor will be
+    removed in a future release of the agent.**  Note that the `disk-io`
+    monitor has a different dimension (`disk` instead of `plugin_instance`) to
+    specify the disk.
   metrics:
     disk_merged.read:
       description: The number of disk reads merged into single physical disk access

diff --git a/pkg/monitors/diskio/diskio_others.go b/pkg/monitors/diskio/diskio_others.go
@@ -7,14 +7,14 @@ import (
 	"strings"
 	"time"
 
-	gopsutil "github.com/shirou/gopsutil/disk"
+	"github.com/shirou/gopsutil/disk"
 	"github.com/signalfx/golib/v3/datapoint"
 	"github.com/signalfx/signalfx-agent/pkg/monitors/types"
 	"github.com/signalfx/signalfx-agent/pkg/utils"
 	"github.com/signalfx/signalfx-agent/pkg/utils/filter"
 )
 
-var iOCounters = gopsutil.IOCounters
+var iOCounters = disk.IOCounters
 
 // Monitor for Utilization
 type Monitor struct {
@@ -24,7 +24,7 @@ type Monitor struct {
 	filter *filter.OverridableStringFilter
 }
 
-func (m *Monitor) makeLinuxDatapoints(disk gopsutil.IOCountersStat, dimensions map[string]string) []*datapoint.Datapoint {
+func (m *Monitor) makeLinuxDatapoints(disk disk.IOCountersStat, dimensions map[string]string) []*datapoint.Datapoint {
 	return []*datapoint.Datapoint{
 		datapoint.New("disk_ops.read", dimensions, datapoint.NewIntValue(int64(disk.ReadCount)), datapoint.Counter, time.Time{}),
 		datapoint.New("disk_ops.write", dimensions, datapoint.NewIntValue(int64(disk.WriteCount)), datapoint.Counter, time.Time{}),
@@ -41,11 +41,8 @@ func (m *Monitor) makeLinuxDatapoints(disk gopsutil.IOCountersStat, dimensions m
 func (m *Monitor) emitDatapoints() {
 	iocounts, err := iOCounters()
 	if err != nil {
-		if err == context.DeadlineExceeded {
-			logger.WithField("debug", err).Debugf("failed to load io counters. if this message repeats frequently there may be a problem")
-		} else {
-			logger.WithError(err).Errorf("failed to load io counters. if this message repeats frequently there may be a problem")
-		}
+		logger.WithError(err).Errorf("Failed to load disk io counters")
+		return
 	}
 	// var total uint64
 	for key, disk := range iocounts {
@@ -55,17 +52,15 @@ func (m *Monitor) emitDatapoints() {
 			continue
 		}
 
-		pluginInstance := strings.Replace(key, " ", "_", -1)
+		diskName := strings.Replace(key, " ", "_", -1)
 
-		m.Output.SendDatapoints(m.makeLinuxDatapoints(disk, map[string]string{"plugin": monitorType, "plugin_instance": pluginInstance, "disk": pluginInstance})...)
+		m.Output.SendDatapoints(m.makeLinuxDatapoints(disk, map[string]string{"disk": diskName})...)
 	}
 }
 
 // Configure is the main function of the monitor, it will report host metadata
 // on a varied interval
 func (m *Monitor) Configure(conf *Config) error {
-	logger.Warningf("'%s' monitor is in beta on this platform.  For production environments please use 'collectd/%s'.", monitorType, monitorType)
-
 	// create contexts for managing the the plugin loop
 	var ctx context.Context
 	ctx, m.cancel = context.WithCancel(context.Background())

diff --git a/pkg/monitors/diskio/metadata.yaml b/pkg/monitors/diskio/metadata.yaml
@@ -1,5 +1,7 @@
 monitors:
 - dimensions:
+    disk:
+      description: The name of the disk that the metric describes
   doc: |
     This monitor reports I/O metrics about disks.
 

diff --git a/selfdescribe.json b/selfdescribe.json
@@ -3953,7 +3953,7 @@
       "monitorType": "collectd/disk",
       "sendAll": false,
       "dimensions": null,
-      "doc": "This monitor collects information about the usage of\nphysical disks and logical disks (partitions).\n\nSee https://collectd.org/wiki/index.php/Plugin:Disk.\n",
+      "doc": "This monitor collects information about the usage of\nphysical disks and logical disks (partitions).\n\nSee https://collectd.org/wiki/index.php/Plugin:Disk.\n\n**This monitor has been deprecated in favor of the `disk-io` monitor.\nPlease migrate to that monitor as this collectd-based monitor will be\nremoved in a future release of the agent.**  Note that the `disk-io`\nmonitor has a different dimension (`disk` instead of `plugin_instance`) to\nspecify the disk.\n",
       "groups": {
         "": {
           "description": "",
@@ -19008,7 +19008,11 @@
     {
       "monitorType": "disk-io",
       "sendAll": true,
-      "dimensions": null,
+      "dimensions": {
+        "disk": {
+          "description": "The name of the disk that the metric describes"
+        }
+      },
       "doc": "This monitor reports I/O metrics about disks.\n\nOn Linux hosts, this monitor relies on the `/proc` filesystem.\nIf the underlying host's `/proc` file system is mounted somewhere other than\n/proc please specify the path using the top level configuration `procPath`.\n\n```yaml\nprocPath: /proc\nmonitors:\n - type: disk-io\n```\n",
       "groups": {
         "": {