feature: Added Nvidia GPU application support #6024

Merged
merged 86 commits into from Mar 3, 2017

Conversation

Projects
None yet
6 participants
@VVelox
Contributor

VVelox commented Feb 27, 2017

DO NOT DELETE THIS TEXT

Please note

Please read this information carefully. You can run ./scripts/pre-commit.php to check your code before submitting.

  • Have you signed the Contributors agreement - please do NOT submit a pull request unless you have (signing the agreement in the same pull request is fine). Your commit message for signing the agreement must appear as per the docs.
  • Have you followed our code guidelines?

Testers

If you would like to test this pull request then please run: ./scripts/github-apply <pr_id>, i.e ./scripts/github-apply 5926

This monitors all Nvidia GPUs on a polled system, storing stats on each one in it's own RRD.

VVelox and others added some commits Feb 15, 2017

kitsune
*rename it so it can be called via the apps page
*setup the apps page to call it
@mention-bot

This comment has been minimized.

Show comment
Hide comment
@mention-bot

mention-bot Feb 27, 2017

Thank you for submitting a PR @VVelox! We have found the following @murrant, @crcro and @sorano based on the history of these files to review this PR.

Thank you for submitting a PR @VVelox! We have found the following @murrant, @crcro and @sorano based on the history of these files to review this PR.

@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment
@VVelox

This comment has been minimized.

Show comment
Hide comment
@VVelox

VVelox Feb 27, 2017

Contributor

This only supports of to 6 GPUs(limited by the use of the color pallate(mixed) used). Not seen any boxes used for number crunch that really have more than that.

Contributor

VVelox commented Feb 27, 2017

This only supports of to 6 GPUs(limited by the use of the color pallate(mixed) used). Not seen any boxes used for number crunch that really have more than that.

@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment
+ echo "file missing: $rrd_filename";
+}
+
+while (is_file($rrd_filename)) {

This comment has been minimized.

@laf

laf Mar 1, 2017

Member

Can't do is_file - we have the ability to use fully remote rrdcached so the poller / webui won't see the file physically.

@laf

laf Mar 1, 2017

Member

Can't do is_file - we have the ability to use fully remote rrdcached so the poller / webui won't see the file physically.

This comment has been minimized.

@VVelox

VVelox Mar 1, 2017

Contributor

What function do I use for checking if a RRD exists then?

@VVelox

VVelox Mar 1, 2017

Contributor

What function do I use for checking if a RRD exists then?

This comment has been minimized.

@laf

laf Mar 1, 2017

Member

You've done it above: rrdtool_check_rrd_exists

@laf

laf Mar 1, 2017

Member

You've done it above: rrdtool_check_rrd_exists

This comment has been minimized.

@VVelox

VVelox Mar 1, 2017

Contributor

Doh! Fixed now. :)

@VVelox

VVelox Mar 1, 2017

Contributor

Doh! Fixed now. :)

+ );
+
+ $int++;
+ $rrd_filename=rrd_name($device['hostname'], array('app', $app['app_type'], $app['app_id'], $int));

This comment has been minimized.

@laf

laf Mar 1, 2017

Member

Is this duplicated on purpose?

@laf

laf Mar 1, 2017

Member

Is this duplicated on purpose?

This comment has been minimized.

@VVelox

VVelox Mar 1, 2017

Contributor

Yup. Because there may be more than one GPU found on the system, in which case it will loop back around and add that one as well to RRD list.

@VVelox

VVelox Mar 1, 2017

Contributor

Yup. Because there may be more than one GPU found on the system, in which case it will loop back around and add that one as well to RRD list.

+
+$int=0;
+while (isset($gpuArray[$int])) {
+ list( $gpu, $pwr, $temp, $sm, $mem, $enc, $dec, $mclk, $pclk, $pviol, $tviol,

This comment has been minimized.

@laf

laf Mar 1, 2017

Member

Drop the space after list( and at the end.

@laf

laf Mar 1, 2017

Member

Drop the space after list( and at the end.

This comment has been minimized.

@VVelox

VVelox Mar 1, 2017

Contributor

Done. :)

@VVelox

VVelox Mar 1, 2017

Contributor

Done. :)

@laf

I have to laugh at this one, well done for putting it together :)

@laf laf changed the title from add Nvidia GPU monitor app to feature: Added Nvidia GPU application support Mar 1, 2017

@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment
@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment
@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment
@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment
@laf

This comment has been minimized.

Show comment
Hide comment
@laf

laf Mar 2, 2017

Member

Can you change all variables to lower case please.

Member

laf commented Mar 2, 2017

Can you change all variables to lower case please.

@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment
@scrutinizer-notifier

This comment has been minimized.

Show comment
Hide comment
@scrutinizer-notifier

scrutinizer-notifier Mar 3, 2017

The inspection completed: No new issues

The inspection completed: No new issues

@laf laf merged commit 671654d into librenms:master Mar 3, 2017

2 checks passed

Auto-Deploy Build finished.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment