HiveBrain v1.2.0
Get Started
← Back to all entries
patternrubyMinor

Printing some metrics from a Windows machine

Submitted by: @import:stackexchange-codereview··
0
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.

``
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 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)
  }
end


This 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]
  ...
end

Code 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)
  }
end
tmp.map do |disk|
  disk = disk.split
  caption = data[0]
  drivetype = data[1]
  freespace = data[2]
  size = data[3]
  ...
end

Context

StackExchange Code Review Q#39824, answer score: 2

Revisions (0)

No revisions yet.