Add Canopsis transport #3299

Merged
merged 13 commits into from Mar 31, 2016

Projects

None yet

4 participants

@gcoeugnet
Contributor

As suggested in a previous issue, this update is a proposal to include a new transport for canopsis which is an hypervision tool with an event console.

Canopsis is working with AMQP messaging system and then needs this package to work : php-amqplib (ubuntu)

I had to include AMQP classes in includes/alerts/transport.canopsis.php but this leads to an issue. LibreNMS transports are called from an anonymous function :$tmp = $tmp($obj,$opts);
But PHP doesn't allow external classes (with 'use' keyword) to be called from such function. I had then to insert an if statement (see below) to take this in charge. My proposal is probably not the best one (not a PHP expert :)) but after some searchs/tries, I don't succeed in finding something better. Any suggestions are welcome.

if ($transport == "canopsis")
    $tmp = eval ('global $config; '.file_get_contents($config['install_dir'].'/includes/alerts/transport.'.$transport.'.php').' return false;');
else {
    eval('$tmp = function($obj,$opts) { global $config; '.file_get_contents($config['install_dir'].'/includes/alerts/transport.'.$transport.'.php').' return false; };');
    $tmp = $tmp($obj,$opts);
}

This new configuration options have to be included in database :

insert into config values ('','alert.transports.canopsis.user','','','Canopsis User','alerting',0, 'transports', 0, 0, 0);
insert into config values ('','alert.transports.canopsis.passwd','','','Canopsis Password','alerting',0, 'transports', 0, 0, 0);
insert into config values ('','alert.transports.canopsis.host','','','Canopsis Hostname','alerting',0, 'transports', 0, 0, 0);
insert into config values ('','alert.transports.canopsis.vhost','','','Canopsis vHost','alerting',0, 'transports', 0, 0, 0);

A canopsis developper told me that a proprietary version of this transport exists and is probably not supposed to be open source. Indeed, two more commercial versions of the product exists with the open source version.

Transport code has been taken from here and has been updated to work with libreNMS.

gcoeugnet added some commits Mar 29, 2016
@gcoeugnet gcoeugnet Create transport.canopsis.php
Add canopsis transport file
82b639e
@gcoeugnet gcoeugnet Update alerts.php
Update to make canopsis transport works. Canopsis transport use AMQP PHP libraries which needs to be installed and included at the begining of the transport but PHP doesn't allow this include to occured in an anonyous function.
0e5f4f4
@gcoeugnet gcoeugnet Update test-transport.inc.php
Add test for canopsis transport
293bef2
@gcoeugnet gcoeugnet Update alerting.inc.php
7584539
@gcoeugnet gcoeugnet referenced this pull request in capensis/canopsis Mar 29, 2016
Closed

[Résolu] Compréhension bac à événement #227

@laf
Member
laf commented Mar 30, 2016

I'm not sure of the best way to do the eval side of things ( @f0o ?) but I've left inline comments.

We also need you to sign the contributors agreement as per: docs.librenms.org/General/Contributing/

@laf
Member
laf commented Mar 30, 2016

p.s thanks for the pull request :)

@laf laf added the Alerting label Mar 30, 2016
@laf laf and 2 others commented on an outdated diff Mar 30, 2016
includes/alerts/transport.canopsis.php
@@ -0,0 +1,64 @@
+require_once "/usr/share/php/PhpAmqpLib/autoload.php";
@laf
laf Mar 30, 2016 Member

Can we be sure that this is always present in this location?

@gcoeugnet
gcoeugnet Mar 30, 2016 Contributor

Hum.... Good question. As far as I can see, only Ubuntu and Debian bring amqplib-php. I don't find it on RHEL like distribs...There is no reason that Ubuntu package change this location... Xenial package will keep the same path.

If people install amqplib-php on distribs other than Debian like, by compiling it for example, it will certainly not be in the same path ... I thought notify users of this dependency during install process but install process of libreNMS is just a git clone, so not possible to hot compile an amqplib-php during install of libreNMS.

What do you think ?

@f0o
f0o Mar 30, 2016 Member

What about using composer or just shipping one in the libs dir?

On 30 March 2016 14:17:01 CEST, gcoeugnet notifications@github.com wrote:

@@ -0,0 +1,64 @@
+require_once "/usr/share/php/PhpAmqpLib/autoload.php";

Hum.... Good question. As far as I can see, only Ubuntu and Debian
bring amqplib-php. I don't find it on RHEL like distribs...There is no
reason that Ubuntu package change this location... Xenial package will
keep the same path.

If people install amqplib-php on distribs other than Debian like, by
compiling it for example, it will certainly not be in the same path ...
I thought notify users of this dependency during install process but
install process of libreNMS is just a git clone, so not possible to hot
compile an amqplib-php during install of libreNMS.

What do you think ?


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
https://github.com/librenms/librenms/pull/3299/files/758453938ee2176becdf134892f4b7ab79d65d65#r57878103

@laf
laf Mar 30, 2016 Member

Shipping the lib is probably best if it's gplv3 compatible.

@gcoeugnet
gcoeugnet Mar 30, 2016 Contributor

It seems that the amqplib-php software is under GPL
https://github.com/php-amqplib/php-amqplib/blob/master/LICENSE

@laf laf commented on an outdated diff Mar 30, 2016
includes/alerts/transport.canopsis.php
+ "resource" => $obj['device_id'],
+ "state" => $state,
+ "state_type" => 1,
+ "output" => $obj['msg'],
+ "long_output" => $obj['msg'],
+ "display_name" =>"librenms_test"
+);
+$msg_raw = json_encode($msg_body);
+
+// Build routing key
+if ($msg_body['source_type'] == "resource")
+ $msg_rk = $msg_rk . "." . $msg_body['resource'];
+else
+ $msg_rk = $msg_body['connector'].".".$msg_body['connector_name'].".".$msg_body['event_type'].".".$msg_body['source_type'].".".$msg_body['component'];
+
+#echo "JSON Event: " . $msg_raw . "\n";
@laf
laf Mar 30, 2016 Member

May as well remove this (appears to be debug output)

@laf laf commented on an outdated diff Mar 30, 2016
includes/alerts/transport.canopsis.php
+ "state" => $state,
+ "state_type" => 1,
+ "output" => $obj['msg'],
+ "long_output" => $obj['msg'],
+ "display_name" =>"librenms_test"
+);
+$msg_raw = json_encode($msg_body);
+
+// Build routing key
+if ($msg_body['source_type'] == "resource")
+ $msg_rk = $msg_rk . "." . $msg_body['resource'];
+else
+ $msg_rk = $msg_body['connector'].".".$msg_body['connector_name'].".".$msg_body['event_type'].".".$msg_body['source_type'].".".$msg_body['component'];
+
+#echo "JSON Event: " . $msg_raw . "\n";
+#echo "Routing-key: " . $msg_rk . "\n";
@laf
laf Mar 30, 2016 Member

Same with this.

@laf
Member
laf commented Mar 30, 2016

For the schema updates, you need to add those lines to a schema file, at present if you use:

sql-schema/110.sql

We should be ok for a merge.

importepeu and others added some commits Mar 30, 2016
@importepeu importepeu remove debug lines
290bb96
@importepeu importepeu Change event resource
5d21bf9
@gcoeugnet gcoeugnet Create 110.sql
Add options to config table for canopsis transport.
1fc742e
@gcoeugnet
Contributor

I had a 110.sql with the four SQL requests above.
I removed debug command lines in the transport file and change an event field for detail to be more accurate.

@f0o f0o and 1 other commented on an outdated diff Mar 30, 2016
@@ -340,8 +340,13 @@ function ExtTransports($obj) {
$msg = FormatAlertTpl($obj);
$obj['msg'] = $msg;
echo $transport.' => ';
- eval('$tmp = function($obj,$opts) { global $config; '.file_get_contents($config['install_dir'].'/includes/alerts/transport.'.$transport.'.php').' return false; };');
- $tmp = $tmp($obj,$opts);
+ if ($transport == "canopsis") {
@f0o
f0o Mar 30, 2016 Member

I dont like this part, any reason why you dont use fully qualified class names instead?

@gcoeugnet
gcoeugnet Mar 30, 2016 Contributor

No, no particular reason. I just tried to use fully qualified class names in the transport code but when I took a look in the AMQPConnection class, I saw that used namespace which is a PHP functionnality that I don't use. I keep on receiving error while trying to use this. I can try again to take a look in that way but namespace class is a thing that I don't masterize at all... :)

# cat /usr/share/php/PhpAmqpLib/Connection/AMQPConnection.php
<?php
namespace PhpAmqpLib\Connection;

/**
 * Class AMQPConnection
 *
 * Kept for BC
 *
 * @deprecated
 */
class AMQPConnection extends AMQPStreamConnection
{
}

EDIT: after reading some docs it appears to be simpler than expected. I'll give this a shot and see if I can do something better...

@gcoeugnet
Contributor

I modify some stuff regarding f0o comments. Do this updates seem better to you ?

  • I included the AMQP library in /lib
  • I refered to this lib in the transport (line require_once)
  • I removed the namespace to avoid "use" call from the transport which all to remove the canopsis if statement included before.
  • I confirm that the piece of code used for the transport is not under any copyright. I contacted the author who told me that his code is under this licence : http://choosealicense.com/licenses/unlicense/

So, the files alert.php and /html/includes/forms/test-transport.inc.php can be discarded and we can keep your copies... I'm discovering git as I'm discovering libreNMS and other stuffs, I didn't succeed yet in reverting thoose files from your repository.

@laf laf removed the Blocker label Mar 31, 2016
@laf
Member
laf commented Mar 31, 2016

Looks good to me. License for this lib is LGPL2.1 so fine with GPLv3

@laf
Member
laf commented Mar 31, 2016

Will merge shortly unless someone else does.

@gcoeugnet gcoeugnet Update AUTHORS.md
I agree to the conditions of the Contributor Agreement contained in doc/General/Contributing.md.
0bff692
@gcoeugnet
Contributor

Ok, I filled the AUTHORS.md as asked before.
Hope this will help others...

@laf laf merged commit b1abe91 into librenms:master Mar 31, 2016

2 checks passed

Scrutinizer No new issues
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@laf
Member
laf commented Mar 31, 2016

Totally forgot to ask you to add docs for this:

docs.librenms.org/Extensions/Alerting/#transports

Can you do a separate PR with the updated docs please @gcoeugnet ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment