Ok

I reviewed the snmp_get_multi code once again and i concluded that i think the line

list($oid, $index) = explode(".", $oid);

In the original code is just wrong and not intended and should be:

list($oid, $index) = explode(".", $oid,2);

This is the same code that is in snmpwalk_cache_oid and then correctly returns the full index. So this is the fix in snmp.inc.php and my worthless code in splitting at the dot should be ignored with the above fix in place.

However, my suggestion for the code for collect_table still stands so that you can use a full oid with a blank index in the table def.

I would appreciate a response on this to see if it can be incorporated as I would like to publish the Ubiquiti integration based on the table_def/collect_table method.

Also I have generated configs in graphtypes.inc.php and it occurred to me that this is vulnerable to SVN update overwrite. Would it not be a good idea to have a separate subdirectory for custom graph groups which are "auto included" in the main config - a bit like you do for mib processing in polling/graphs.

This means that if one develops graphs for a specific mib you give them the mib processor to put in polling/graphs and a graph config to go in the new directory. This would then be agnostic to SVN updates.

From: observium [mailto:observium-bounces@observium.org] On Behalf Of Mark Gibbons Sent: 27 March 2015 12:24 To: observium@observium.org Subject: [Observium] ISSUES/BUg with regards to collect_table Implementation- Suggested Code Changes

Hi

(especially adam - can you take a read please)

Background

I have been working on integrating support for Ubiquiti wireless products so that wireless stats can be shown on the "wireless" graph page for these devices. I created routines to process the mibs in /include/polling/graphs using the original loop method to parse the mib to generate and populate the RRD's and everything is working absolutely fine. I will of course submit this work for inclusion once tested/verified fully.

However, i read the README (you see people do read README's!!!!!!!!!!):

"Please make sure you use the *new* format of graphing poller. Generate a $graph_defs array to be used in collect_table."

And i thought I would have a go at the $table_def method. So I defined the MIB as per $tabledef and come across issues with multi indexed oids.

In this instance from a MIB walk we have two oids as follows:

ubntRadioRssi.1.1

ubntRadioRssi.1.2

So i put the following in table_def:

'ubntRadioRssi.1' => array('index' => '1', 'descr' => 'RSSI Chain 0', 'ds_type' => 'GAUGE', 'ds_name' => 'RadioRssi_0'),

'ubntRadioRssi.1' => array('index' => '2', 'descr' => 'RSSI Chain 1', 'ds_type' => 'GAUGE', 'ds_name' => 'RadioRssi_1'),

But, of course, (knowing what i know now) this does not work because the array index is the same and you only get the last entry.

So i thought try with a blank index specifying the full oid:

'ubntRadioRssi.1.1' => array('index' => '', 'descr' => 'RSSI Chain 0', 'ds_type' => 'GAUGE', 'ds_name' => 'RadioRssi_0'),

'ubntRadioRssi.1.2' => array('index' => '', 'descr' => 'RSSI Chain 1', 'ds_type' => 'GAUGE', 'ds_name' => 'RadioRssi_1'),

But this didn't work so I investgated. The issues surround the handling of the separation of oid and index in both collect table AND snmp_get_multi.

Let me Deal with snmp_get_multi first.

Snmp_get multi expects an array of full oid's - great.

However it spits back an array of oids and index separated. Because you use explode to breakup the oid it will only ever return the first index after the first "." rather than the full index after the first "."

I am talking about this code at approx Line 464 in /includes/snmp.inc.php

foreach (explode("\n", $data) as $entry)

{

list($oid,$value) = explode("=", $entry, 2);

$oid = trim($oid); $value = trim($value);

list($oid, $index) = explode(".", $oid);

if (!strstr($value, "at this OID") && isset($oid) && isset($index))

{

$array[$index][$oid] = $value;

}

}

This can be corrected if you change it to the following code. In this case the base oid is returned with the FULL index in the index. I can't see that this could/would break other stuff but your experience would know better!! :

foreach (explode("\n", $data) as $entry)

{

list($oid,$value) = explode("=", $entry, 2);

$oid = trim($oid); $value = trim($value);

$dotpos = strpos($oid, '.');

if ($dotpos != FALSE)

{

$index = substr($oid, $dotpos+1);

$oid = substr($oid, 0, $dotpos);

}

//list($oid, $index) = explode(".", $oid);

if (!strstr($value, "at this OID") && isset($oid) && isset($index))

{

$array[$index][$oid] = $value;

}

}

So the next issue is index handling within collect_table and the temporary index of oids created to generate rrdupdate. Current code is at approx 898 in /includes/polling/functions.inc.php:

$oids[] = $oid.'.'.$entry['index'];

$oids_index[] = array('index' => $entry['index'], 'oid' => $oid);

Again if this is changed to the following so that oid and index is re-split like the return from snmp_get_multi we have a working solution where you can specify the full oid as an index for the oid array in table_def with an oid index of '' (blank):

if ($entry['index'] != '')

{

$fulloid = $oid.'.'.$entry['index'];

} else {

$fulloid = $oid;

}

$oids[] = $fulloid;

$dotpos = strpos($fulloid, '.');

if ($dotpos != FALSE)

{

$splitindex = substr($fulloid, $dotpos+1);

$splitoid = substr($fulloid, 0, $dotpos);

}

//$oids_index[] = array('index' => $entry['index'], 'oid' => $oid);

$oids_index[] = array('index' => $splitindex, 'oid' => $splitoid);

So this is a possible solution to the multiple index problem on an oid. By specifying the full oid with blank index.

What do you think? Seems like a Win/Win to me but my experience of Observium php code and structure is < 1 week.

At least this solution means we can use the table_def method for us but it would mean a lot of maintenance with svn updates.

Hope this solution is of possible interest for inclusion.

KR's

Mark