patternrubyMinor
Printing some metrics from a Windows machine
Viewed 0 times
printingmetricssomewindowsmachinefrom
Problem
New to Ruby here. I just made a script that print some metrics from a Windows machine about its disk. The script works fine and as I wanted it to. Now, since I'm new to the ruby and programming world, I'd like to ask you if anyone could help me to make to code look better or if there's other more efficient way I could have wrote it.
``
logical_disk = []
bytes_to_gbytes = 1073741824
#Convert data from bytes to gigabytes and find the used space (size-free)
tmp.each do |disk|
disk = disk.split(" ")
if disk[1].to_i == 3
disk[2] = (disk[2].to_f/bytes_to_gbytes).round(2)
disk[3] = (disk[3].to_f/bytes_to_gbytes).round(2)
disk.push((disk[3].to_f - disk[2].to_f).round(2))
logical_disk.push(disk)
end
end
return logical_disk
end
#Add the disks usage in %
def get_logical_disk_usage_percentage(logical_disk)
logical_disk.each do |disk|
disk.push(((disk[2].to_f/disk[3].to_f)*100).round(2))
disk.push(100.00-disk[5])
end
return logical_disk
end
def run
timestamp = Time.now.utc.to_i
logical_disk = get_logical_disk_metrics()
logical_disk = get_logical_disk_usage_percentage(logical_disk)
logical_disk.each do |disk|
metrics = {
:"logical_disk_#{disk[0]}" => {
:"size(Gb)" => disk[3],
:"free_space(Gb)" => disk[2],
:"free_space(%)" => disk[5],
:"used_space(Gb)" => disk[4],
:"used_space(%)" => disk[6]
}
}
metrics.each do |parent, children|
children.each do |child, value|
output [config[:scheme], parent, child].join("."), val
``
require 'rubygems' if RUBY_VERSION "Metric naming scheme, text to prepend to .$parent.$child",
:long => "--scheme SCHEME",
:default => "#{Socket.gethostname}"
#Run a wmic command to get some stats about the disks usage
def get_logical_disk_metrics
tmp = wmic logicaldisk get caption, drivetype, freespace, size`.split("\n")logical_disk = []
bytes_to_gbytes = 1073741824
#Convert data from bytes to gigabytes and find the used space (size-free)
tmp.each do |disk|
disk = disk.split(" ")
if disk[1].to_i == 3
disk[2] = (disk[2].to_f/bytes_to_gbytes).round(2)
disk[3] = (disk[3].to_f/bytes_to_gbytes).round(2)
disk.push((disk[3].to_f - disk[2].to_f).round(2))
logical_disk.push(disk)
end
end
return logical_disk
end
#Add the disks usage in %
def get_logical_disk_usage_percentage(logical_disk)
logical_disk.each do |disk|
disk.push(((disk[2].to_f/disk[3].to_f)*100).round(2))
disk.push(100.00-disk[5])
end
return logical_disk
end
def run
timestamp = Time.now.utc.to_i
logical_disk = get_logical_disk_metrics()
logical_disk = get_logical_disk_usage_percentage(logical_disk)
logical_disk.each do |disk|
metrics = {
:"logical_disk_#{disk[0]}" => {
:"size(Gb)" => disk[3],
:"free_space(Gb)" => disk[2],
:"free_space(%)" => disk[5],
:"used_space(Gb)" => disk[4],
:"used_space(%)" => disk[6]
}
}
metrics.each do |parent, children|
children.each do |child, value|
output [config[:scheme], parent, child].join("."), val
Solution
Some general pointers:
Refrain from using
Working with arrays as object replacement (
This code returns an array of hashes, one for each disk with drivetype '3'. The variables (
Refrain from using
tmp as variable names. raw_output will do the job better.Working with arrays as object replacement (
x[0] is the caption, x[1] is the type...) is cumbersome, unreadable, and difficult to maintain. I suggest moving to Hash:tmp.map(&:split).select { |_, drivetype, _, _| drivetype.to_i == 3 }.map do |caption, drivetype, freespace, size|
{
caption: caption,
drivetype: drivetype,
freespace: (freespace.to_f/bytes_to_gbytes).round(2),
size: (size.to_f/bytes_to_gbytes).round(2),
usedspace: ((size.to_f - freespace.to_f)/bytes_to_gbytes).round(2)
}
endThis code returns an array of hashes, one for each disk with drivetype '3'. The variables (
caption, drivetype, etc...) are created when ruby translates the array to the list of variables declared, so actually the tmp.map(&:split).map { |caption, drivetype, freespace, size| ... } is like saying:tmp.map do |disk|
disk = disk.split
caption = data[0]
drivetype = data[1]
freespace = data[2]
size = data[3]
...
endCode Snippets
tmp.map(&:split).select { |_, drivetype, _, _| drivetype.to_i == 3 }.map do |caption, drivetype, freespace, size|
{
caption: caption,
drivetype: drivetype,
freespace: (freespace.to_f/bytes_to_gbytes).round(2),
size: (size.to_f/bytes_to_gbytes).round(2),
usedspace: ((size.to_f - freespace.to_f)/bytes_to_gbytes).round(2)
}
endtmp.map do |disk|
disk = disk.split
caption = data[0]
drivetype = data[1]
freespace = data[2]
size = data[3]
...
endContext
StackExchange Code Review Q#39824, answer score: 2
Revisions (0)
No revisions yet.