Log temperature from juniper devices #2364
Conversation
There was a problem hiding this comment.
Although this implementation is fine: IMNSHO, temperature logging is better suited for the environment sensors framework of NAV, than it is for the "system" framework.
Sensors are registered as separate entitites in the NAV database, and they can also be associated with known sub-components of the device. Dashboard widgets exist to present temperature readings from sensors, etc.
Retrieving and storing operating contents of a Juniper device may still not be properly supported by NAV, since Juniper is one of very few vendors that do not have proper support for ENTITY-MIB, but we have issues on our roadmap to do something about it - and once we get there, it would be grand to attach these temperature "sensors" to the correct sub-components.
How to make a sensor implementation is described in https://nav.readthedocs.io/en/latest/hacking/adding-environment-probe-support.html - but it basically reduces your effort to implementing a get_all_sensors() method for the JuniperMib subclass.
stveit
force-pushed
the
juniper-temperature
branch
3 times, most recently
from
2042cf6 to
c4dab9f
Compare
| @@ -77,7 +77,6 @@ def handle(self): | |||
| "Found %d sensors from %s", len(all_sensors), type(mib).__name__ | |||
| ) | |||
| self._store_sensors(all_sensors) | |||
| break | |||
There was a problem hiding this comment.
This was done so sensors are collected from all the MIBs listed in JUNIPER_NETWORKS_INC line in the config instead of stopping at the first one where it finds any sensors. This will impact all sensor MIBs, so I'm wondering 1) is there a reason why it should stop after finding sensors in one MIB? 2) If so, ideas for config that will allow only Juniper devices to collect sensors from multiple MIBs?
There was a problem hiding this comment.
Very good point. The juniper chassis error counts will also need to look into two different MIBs if we are to support all juniper equipment that has these counters.
There was a problem hiding this comment.
- is there a reason why it should stop after finding sensors in one MIB?
I think the main reason is that some of the first devices this code was tested
on would report the same sensors in multiple MIBs (i.e. the sensors reported in
a proprietary MIB would also appear in ENTITY-SENSOR-MIB, and there is no
generic way of determining that these records represent the same thing).
- If so, ideas for config that will allow only Juniper devices to collect
sensors from multiple MIBs?
Naw. I think the original idea is a bit defeatist. If a device wants to report
multiple versions of the same sensors, that's not our problem, that's the
vendor's problem (and a little bit, the user's: Now ipdevpoll will spend twice
the time collecting data from this device, and twice the storage space to
persist it - but hey, storage is cheap these days).
So, IMO: It's a good idea to remove the break statement!
There was a problem hiding this comment.
@hmpf wrote:
Very good point. The juniper chassis error counts will also need to look into
two different MIBs if we are to support all juniper equipment that has these
counters.
The point is good, but I don't see how it related to the ongoing alert count
collection feature. It's unlikely this will touch the sensors framework at all
(unless you plan to also keep time series data of alert counts).
Codecov Report
@@ Coverage Diff @@
## master #2364 +/- ##
==========================================
- Coverage 52.56% 52.55% -0.02%
==========================================
Files 552 552
Lines 40118 40130 +12
==========================================
+ Hits 21089 21091 +2
- Misses 19029 19039 +10
Continue to review full report at Codecov.
|
| @@ -77,7 +77,6 @@ def handle(self): | |||
| "Found %d sensors from %s", len(all_sensors), type(mib).__name__ | |||
| ) | |||
| self._store_sensors(all_sensors) | |||
| break | |||
There was a problem hiding this comment.
- is there a reason why it should stop after finding sensors in one MIB?
I think the main reason is that some of the first devices this code was tested
on would report the same sensors in multiple MIBs (i.e. the sensors reported in
a proprietary MIB would also appear in ENTITY-SENSOR-MIB, and there is no
generic way of determining that these records represent the same thing).
- If so, ideas for config that will allow only Juniper devices to collect
sensors from multiple MIBs?
Naw. I think the original idea is a bit defeatist. If a device wants to report
multiple versions of the same sensors, that's not our problem, that's the
vendor's problem (and a little bit, the user's: Now ipdevpoll will spend twice
the time collecting data from this device, and twice the storage space to
persist it - but hey, storage is cheap these days).
So, IMO: It's a good idea to remove the break statement!
| @@ -77,7 +77,6 @@ def handle(self): | |||
| "Found %d sensors from %s", len(all_sensors), type(mib).__name__ | |||
| ) | |||
| self._store_sensors(all_sensors) | |||
| break | |||
There was a problem hiding this comment.
@hmpf wrote:
Very good point. The juniper chassis error counts will also need to look into
two different MIBs if we are to support all juniper equipment that has these
counters.
The point is good, but I don't see how it related to the ongoing alert count
collection feature. It's unlikely this will touch the sensors framework at all
(unless you plan to also keep time series data of alert counts).
python/nav/mibs/juniper_mib.py
Outdated
| 'description': descr, | ||
| 'name': descr, | ||
| 'internal_name': internal_name, | ||
| 'mib': 'SPAGENT-MIB', |
There was a problem hiding this comment.
The MIB is definitely not SPAGENT-MIB in this case :-)
| 'mib': 'SPAGENT-MIB', | |
| 'mib': self.get_module_name(), |
I have tested now, and it seems to work beautifully 👍 |
stveit
force-pushed
the
juniper-temperature
branch
from
91069ba to
9a08b33
Compare
Previously the code would stop after finding sensors from one of the Mibs gotten from self.mibfactory(). Now it goes through all the mibs and stores sensors from all of them This makes it possible to get JuniperDomMib sensors and JuniperMib temperature data but does it break other stuff?
stveit
force-pushed
the
juniper-temperature
branch
from
9a08b33 to
6a7675f
Compare
|
Kudos, SonarCloud Quality Gate passed!
|
Fixes #2342
Adds capacity for polling temperature data from Juniper devices.
Basically copies code for logging CPU data.