Skip to content

Commit

Permalink
[disk] fix system.disk.*_time_pct for Windows
Browse files Browse the repository at this point in the history
And remove them for Linux, as it wasn't in the previous check.
  • Loading branch information
degemer committed Jun 15, 2015
1 parent fe31fad commit bb2250b
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 33 deletions.
29 changes: 10 additions & 19 deletions checks.d/disk.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,11 +85,15 @@ def collect_metrics_psutil(self):

tags = [part.fstype] if self._tag_by_filesystem else []
device_name = part.mountpoint if self._use_mount else part.device
# legacy check names c: vs psutil name C:\\
if Platform.is_win32():
device_name = device_name.strip('\\').lower()
for metric_name, metric_value in self._collect_part_metrics(part).iteritems():
self.gauge(metric_name, metric_value,
tags=tags, device_name=device_name)
# And finally, latency metrics, a legacy gift from the old Windows Check
self.collect_latency_metrics()
if Platform.is_win32():
self.collect_latency_metrics()

def _exclude_disk_psutil(self, part):
# skip cd-rom drives with no disk in it; they may raise
Expand Down Expand Up @@ -136,28 +140,15 @@ def _collect_inodes_metrics(self, mountpoint):

def collect_latency_metrics(self):
for disk_name, disk in psutil.disk_io_counters(True).iteritems():
# disk_name is sda1, _valid_disks contains /dev/sda1
match_disks = [d for d in self._valid_disks
if re.match('^(/dev/)?{0}$'.format(disk_name), d)]
if not match_disks:
continue

device = match_disks[0]
fstype, mountpoint = self._valid_disks[device]
self.log.debug('Passed: {0} -> {1}'.format(disk_name, device))
tags = []
if self._tag_by_filesystem:
tags = [fstype]
device_name = mountpoint if self._use_mount else device

self.log.debug('IO Counters: {0} -> {1}'.format(disk_name, disk))
# x100 to have it as a percentage,
# /1000 as psutil returns the value in ms
read_time_pct = disk.read_time * 100.0 / 1000.0
write_time_pct = disk.write_time * 100.0 / 1000.0
self.gauge(self.METRIC_DISK.format('read_time_pct'),
read_time_pct, device_name=device_name, tags=tags)
self.gauge(self.METRIC_DISK.format('write_time_pct'),
write_time_pct, device_name=device_name, tags=tags)
self.rate(self.METRIC_DISK.format('read_time_pct'),
read_time_pct, device_name=disk_name)
self.rate(self.METRIC_DISK.format('write_time_pct'),
write_time_pct, device_name=disk_name)

# no psutil, let's use df
def collect_metrics_manually(self):
Expand Down
2 changes: 0 additions & 2 deletions tests/checks/integration/test_disk.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@ class TestCheckDisk(AgentCheckTest):
'system.disk.used',
'system.disk.free',
'system.disk.in_use',
'system.disk.read_time_pct',
'system.disk.write_time_pct'
]

INODE_GAUGES = [
Expand Down
39 changes: 27 additions & 12 deletions tests/checks/mock/test_disk.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,36 +83,51 @@ def test_device_exclusion_logic(self):

@mock.patch('psutil.disk_partitions', return_value=[MockPart()])
@mock.patch('psutil.disk_usage', return_value=MockDiskMetrics())
@mock.patch('psutil.disk_io_counters',
return_value={'sda1': MockIoCountersMetrics()})
@mock.patch('os.statvfs', return_value=MockInodesMetrics())
def test_psutil(self, mock_partitions, mock_usage,
mock_io_counters, mock_inodes):
def test_psutil(self, mock_partitions, mock_usage, mock_inodes):
for tag_by in ['yes', 'no']:
self.run_check({'instances': [{'tag_by_filesystem': tag_by}]},
force_reload=True)

# Assert metrics
tags = ['ext4'] if tag_by == 'yes' else []
self.GAUGES_VALUES_PSUTIL.update(self.GAUGES_VALUES)
for metric, value in self.GAUGES_VALUES_PSUTIL.iteritems():
for metric, value in self.GAUGES_VALUES.iteritems():
self.assertMetric(metric, value=value, tags=tags,
device_name=DEFAULT_DEVICE_NAME)

self.coverage_report()

# FIXME: patch the import of Platform to be able to test mocked Windows
# @mock.patch('psutil.disk_partitions',
# return_value=[MockPart(device='C:\\', fstype='ntfs',
# mountpoint='C:\\')])
# @mock.patch('psutil.disk_usage', return_value=MockDiskMetrics())
# @mock.patch('psutil.disk_io_counters',
# return_value={'PhysicalDisk0': MockIoCountersMetrics()})
# @mock.patch('util.Platform', return_value=WindowsPlatform)
# def test_psutil_windows(self, mock_partitions, mock_usage,
# mock_io_counters, mock_platform):
# self.run_check({'instances': [{'use_mount': 'no'}]},
# mocks={'_psutil': lambda: True})
#
# # Assert metrics
# for metric, value in self.GAUGES_VALUES.iteritems():
# self.assertMetric(metric, value=value, tags=[],
# device_name='c:')
# for metric, value in self.GAUGES_VALUES_PSUTIL.iteritems():
# self.assertMetric(metric, value=value, tags=[],
# device_name='PhysicalDisk0')
#
# self.coverage_report()

@mock.patch('psutil.disk_partitions', return_value=[MockPart()])
@mock.patch('psutil.disk_usage', return_value=MockDiskMetrics())
@mock.patch('psutil.disk_io_counters',
return_value={'sda1': MockIoCountersMetrics()})
@mock.patch('os.statvfs', return_value=MockInodesMetrics())
def test_use_mount(self, mock_partitions, mock_usage,
mock_io_counters, mock_inodes):
def test_use_mount(self, mock_partitions, mock_usage, mock_inodes):
self.run_check({'instances': [{'use_mount': 'yes'}]})

# Assert metrics
self.GAUGES_VALUES_PSUTIL.update(self.GAUGES_VALUES)
for metric, value in self.GAUGES_VALUES_PSUTIL.iteritems():
for metric, value in self.GAUGES_VALUES.iteritems():
self.assertMetric(metric, value=value, tags=[],
device_name='/')

Expand Down

0 comments on commit bb2250b

Please sign in to comment.