Skip to content
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

Added "system name" for the "Services list" #6873

Merged
merged 6 commits into from
Jun 23, 2017
Merged

Added "system name" for the "Services list" #6873

merged 6 commits into from
Jun 23, 2017

Conversation

algebur
Copy link
Contributor

@algebur algebur commented Jun 21, 2017

service_01

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.

Testers

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

It is easy to watch hosts using system names and IP addresses
How wrote this code? 
I could not use additional plugins, because system adds "-H" when plugin runs. But some plugins don't use "-H"
@CLAassistant
Copy link

CLAassistant commented Jun 21, 2017

CLA assistant check
All committers have signed the CLA.

@@ -129,7 +129,7 @@ function poll_service($service)

// If we do not have a cmd from the check script, build one.
if ($check_cmd == "") {
$check_cmd = $config['nagios_plugins'] . "/check_" . $service['service_type'] . " -H " . ($service['service_ip'] ? $service['service_ip'] : $service['hostname']);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you removing this? It's needed for the checks

Copy link
Contributor Author

@algebur algebur Jun 21, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not needed for the checks, because it makes errors for plugins (nagios and others). Some plugins work, some don't.

For example, if you use it
"/check_" . $service['service_type'] . " -H " . ($service['service_ip'] ? $service['service_ip'] : $service['hostname']);

It runs "-H" by default. But some plugins don't need "-H" in command line, and you get "syntax error"

  1. Check APC
    apc_plugin_01
    apc_plugin_02

  2. Check Mitel
    mitel_plugin
    mitel_plugin_02

  3. Another strange thing is when you want to use "ping" from "Nagios plugins".
    the normal command for ping is:
    -H 172.20.1.56 -w 1000.0,20% -c 2000.0,60%
    but if you run it, it looks like:
    -H -H 172.20.1.56 -w 1000.0,20% -c 2000.0,60%
    and result will be:
    ping_plugin

But what is it? Is it normal result? It's garbage!!!

If you use "/check_" . $service['service_type'] ; , I mean without "-H"
And type normal Parameters (which usually we use from http://nagios-plugins.org/doc/man/check_ping.html):
ping_plugin_02

you will get true result, which must be:
ping_plugin_03

If you need to run service, please, type full command from plugin documentation.
I hope you understand what I mean!

verdict:

string

$check_cmd = $config['nagios_plugins'] . "/check_" . $service['service_type'] . " -H " . ($service['service_ip'] ? $service['service_ip'] : $service['hostname']);

should be

$check_cmd = $config['nagios_plugins'] . "/check_" . $service['service_type'] ;

and everything works OK!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're using them wrong.

If the device you want to do the check against is the hostname you've added the device as then don't set any parameters or the IP. If you want to use a different hostname or IP then set it in the IP box. You don't need to use -H in the parameters box at all. Removing the code you have done means you will break existing service checks for most people.

If a service check doesn't need the -H then you create a custom service check file like we already have, see includes/services/check_procs.inc.php

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for explaining!

If a service check doesn't need the -H then you create a custom service check file like we already have, see includes/services/check_procs.inc.php

Where I should create custom service check file? How to use it?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok! I got it. It works fine. Thanks a lot!
But any way it is better to add SysName near the Hostname. Please do it.
Also my question how to add SysName to Alert Widget window?
sysname
Because it is too hard to understand what is host in huge list

Copy link
Member

@murrant murrant Jun 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@algebur I see this all the time with people wanting to have devices show up as names but add them as an IP. There are two options.

  1. Set up a DNS server, even if it is just a local one on the LibreNMS box, but then you won't get the full benefit of the work to set it up.
  2. Use this setting $config['force_ip_to_sysname'] = true; If you see a spot that should respect that setting, but it does not, I suggest adding support.

P.S. https://github.com/librenms/librenms/blob/master/html/includes/table/alerts.inc.php

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello!
I added
$config['force_ip_to_sysname'] = true;
to file
/master/html/includes/table/alerts.inc.php
It works fine!
Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is Great Idea to add
$config['force_ip_to_sysname'] = true;
Can we add same code for emails?
I still receive emails with IP hostnames. It is hard to recognize what is that.
here is example of my email.
sysname_001

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to deal with that by using templates and setting what you want to see.

@algebur algebur changed the title Added "system name" and changed $check_cmd for services Added "system name" for the "Services list" Jun 23, 2017
@scrutinizer-notifier
Copy link

The inspection completed: No new issues

@laf
Copy link
Member

laf commented Jun 23, 2017

Please can you sign the CLA, we can't merge until this is done.

@algebur
Copy link
Contributor Author

algebur commented Jun 23, 2017

Ok. I did it

@laf laf merged commit 62cde81 into librenms:master Jun 23, 2017
@lock
Copy link

lock bot commented May 17, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed.

@lock lock bot locked as resolved and limited conversation to collaborators May 17, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants