-
Notifications
You must be signed in to change notification settings - Fork 2.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add StorCli text collector example script #320
Conversation
3268c69
to
9fa8ee3
Compare
|
||
|
||
def get_store_cli_json(): | ||
storcli_cmd = ['/opt/MegaRAID/storcli/storcli64', 'show', 'all', 'J'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This path needs to be configurable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left it hardcoded as I figured someone using this would likely copy and adapt it (as an example), but agree it's nicer to make it configurable. Would you prefer a commandline flag?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, a binary-path flag would be great.
} | ||
|
||
for name, value in controller_metrics.iteritems(): | ||
print("{}{}{} {}".format(METRIC_PREFIX, name, labels, value)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is only writign to stdout, it should take care of creating the file and atomically changing it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is intentional as I didn't want the script to have knowledge of where it should put the metrics, since that information will likely be repeated in each different script it seems better to put that logic in the cronjob or whatever calls the script.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, I think we should have a standard interface for all of these scripts. I also would prefer stdout and not have to implement atomic writing in every script, and have a simple atomic write wrapper that can be used to put the metrics files in the correct place.
Also model doesn't belong as a label, the controller number is sufficient to identify the controller. I'd suggest using the machine role approach for it. |
I hesitated to rely on the controller number as I didn't know how stable it is. Happy to remove the model label. |
If it's not stable, the model likely doesn't help you as the chances are you have identical models in a given machine. |
I would recommend a |
Ping, any progress on finishing this? |
Sorry for the delay, will get to this soon. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please address comments
Collect metrics from the StorCLI utility on the health of MegaRAID hardware RAID controllers and write them to stdout so that they can be used by the textfile collector. We parse the JSON output that StorCLI provides. Script must be run as root or with appropriate capabilities for storcli to access the RAID card. Designed to run under Python 2.7, using the system Python provided with many Linux distributions. The metrics look like this: mbostock@host:~$ sudo ./storcli.py megaraid_status_code 0 megaraid_controllers_count 1 megaraid_emergency_hot_spare{controller="0"} 1 megaraid_scheduled_patrol_read{controller="0"} 1 megaraid_virtual_drives{controller="0"} 1 megaraid_drive_groups{controller="0"} 1 megaraid_virtual_drives_optimal{controller="0"} 1 megaraid_degraded{controller="0"} 0 megaraid_battery_backup_healthy{controller="0"} 1 megaraid_ports{controller="0"} 8 megaraid_failed{controller="0"} 0 megaraid_drive_groups_optimal{controller="0"} 1 megaraid_healthy{controller="0"} 1 megaraid_physical_drives{controller="0"} 24 megaraid_controller_info{controller="0", model="AVAGOMegaRAIDSASPCIExpressROMB"} 1 mbostock@host:~$
9fa8ee3
to
004bdca
Compare
@discordianfish @brian-brazil @SuperQ: I've amended to address the comments in this PR:
I've also added Any suggestions for changes welcome. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great, LGMT!
Collect metrics from the StorCLI utility on the health of MegaRAID
hardware RAID controllers and write them to stdout so that they can be
used by the textfile collector.
We parse the JSON output that StorCLI provides.
Script must be run as root or with appropriate capabilities for storcli
to access the RAID card.
Designed to run under Python 2.7, using the system Python provided with
many Linux distributions.
The metrics look like this:
I don't code regularly in Python so any suggestions for improvements welcome.