Improve debug messages (#5918)

* Improve debug messages

The default string representation of pysnmp types is super verbose,
let's add a custom formatter to display better information.

* Handle review comments

* Handle different values

* Add type hints

snmp/datadog_checks/snmp/snmp.py

@@ -25,7 +25,7 @@
 
 from .compat import read_persistent_cache, total_time_to_temporal_percent, write_persistent_cache
 from .config import InstanceConfig, ParsedMetric, ParsedMetricTag, ParsedTableMetric
-from .utils import get_profile_definition, oid_pattern_specificity, recursively_expand_base_profiles
+from .utils import OIDPrinter, get_profile_definition, oid_pattern_specificity, recursively_expand_base_profiles
 
 # Additional types that are not part of the SNMP protocol. cf RFC 2856
 CounterBasedGauge64, ZeroBasedCounter64 = builder.MibBuilder().importSymbols(
@@ -226,7 +226,7 @@ def fetch_results(self, config, all_oids, bulk_oids):
         for result_oid, value in all_binds:
             metric, indexes = config.resolve_oid(result_oid)
             results[metric][indexes] = value
-        self.log.debug('Raw results: %s', results)
+        self.log.debug('Raw results: %s', OIDPrinter(results, with_values=False))
         # Freeze the result
         results.default_factory = None
         return results, error
@@ -245,11 +245,11 @@ def fetch_oids(self, config, oids, enforce_constraints):
         while first_oid < len(oids):
             try:
                 oids_batch = oids[first_oid : first_oid + self.oid_batch_size]
-                self.log.debug('Running SNMP command get on OIDS %s', oids_batch)
+                self.log.debug('Running SNMP command get on OIDS: %s', OIDPrinter(oids_batch, with_values=False))
                 error_indication, error_status, _, var_binds = next(
                     config.call_cmd(hlapi.getCmd, *oids_batch, lookupMib=enforce_constraints)
                 )
-                self.log.debug('Returned vars: %s', var_binds)
+                self.log.debug('Returned vars: %s', OIDPrinter(var_binds, with_values=True))
 
                 self.raise_on_error_indication(error_indication, config.ip_address)
 
@@ -266,7 +266,9 @@ def fetch_oids(self, config, oids, enforce_constraints):
                 if missing_results:
                     # If we didn't catch the metric using snmpget, try snmpnext
                     # Don't walk through the entire MIB, stop at end of table
-                    self.log.debug('Running SNMP command getNext on OIDS %s', missing_results)
+                    self.log.debug(
+                        'Running SNMP command getNext on OIDS: %s', OIDPrinter(missing_results, with_values=False)
+                    )
                     binds_iterator = config.call_cmd(
                         hlapi.nextCmd,
                         *missing_results,
@@ -293,10 +295,10 @@ def fetch_sysobject_oid(self, config):
         """Return the sysObjectID of the instance."""
         # Reference sysObjectID directly, see http://oidref.com/1.3.6.1.2.1.1.2
         oid = hlapi.ObjectType(hlapi.ObjectIdentity((1, 3, 6, 1, 2, 1, 1, 2)))
-        self.log.debug('Running SNMP command on OID %r', oid)
+        self.log.debug('Running SNMP command on OID: %r', OIDPrinter((oid,), with_values=False))
         error_indication, _, _, var_binds = next(config.call_cmd(hlapi.nextCmd, oid, lookupMib=False))
         self.raise_on_error_indication(error_indication, config.ip_address)
-        self.log.debug('Returned vars: %s', var_binds)
+        self.log.debug('Returned vars: %s', OIDPrinter(var_binds, with_values=True))
         return var_binds[0][1].prettyPrint()
 
     def _profile_for_sysobject_oid(self, sys_object_oid):
@@ -319,7 +321,7 @@ def _consume_binds_iterator(self, binds_iterator, config):
         error = None  # type: Optional[str]
 
         for error_indication, error_status, _, var_binds_table in binds_iterator:
-            self.log.debug('Returned vars: %s', var_binds_table)
+            self.log.debug('Returned vars: %s', OIDPrinter(var_binds_table, with_values=True))
 
             self.raise_on_error_indication(error_indication, config.ip_address)
 

snmp/datadog_checks/snmp/utils.py

@@ -1,7 +1,14 @@
+# (C) Datadog, Inc. 2020-present
+# All rights reserved
+# Licensed under Simplified BSD License (see LICENSE)
 import os
 from typing import Any, Dict, Tuple
 
 import yaml
+from pysnmp import hlapi
+from pysnmp.proto.rfc1902 import ObjectName
+from pysnmp.smi.error import SmiError
+from pysnmp.smi.exval import endOfMibView, noSuchInstance
 
 from .compat import get_config
 
@@ -92,3 +99,91 @@ def oid_pattern_specificity(pattern):
         len(parts),  # Shorter OIDs are less specific than longer OIDs, regardless of their contents.
         parts,  # For same-length OIDs, compare their contents (integer parts).
     )
+
+
+class OIDPrinter(object):
+    """Utility class to display OIDs efficiently.
+
+    This is only meant for debugging, as it makes some assumptions on the data
+    managed, and can be more expensive to use than regular display.
+    """
+
+    def __init__(self, oids, with_values):
+        self.oids = oids
+        self.with_values = with_values
+
+    def oid_str(self, oid):
+        # type: (hlapi.ObjectType) -> str
+        """Display an OID object (or MIB symbol), even if the object is not initialized by PySNMP.
+
+        Output:
+            1.3.4.8.3.4
+        """
+        try:
+            return oid[0].getOid().prettyPrint()
+        except SmiError:
+            # PySNMP screams when we try to access the name of an OID
+            # that has no value yet. Fine, let's work around this arbitrary limitation...
+            arg = oid._ObjectType__args[0]._ObjectIdentity__args[-1]
+            if isinstance(arg, tuple):
+                arg = '.'.join(map(str, arg))
+
+            return arg
+
+    def oid_str_value(self, oid):
+        # type: (hlapi.ObjectType) -> str
+        """Display an OID object and its associated value.
+
+        Output:
+            '1.3.4.5.6': 57
+        """
+        if noSuchInstance.isSameTypeWith(oid[1]):
+            value = "'NoSuchInstance'"
+        elif endOfMibView.isSameTypeWith(oid[1]):
+            value = "'EndOfMibView'"
+        else:
+            value = oid[1].prettyPrint()
+            try:
+                value = str(int(value))
+            except (TypeError, ValueError):
+                value = "'{}'".format(value)
+        key = oid[0]
+        if not isinstance(key, ObjectName):
+            key = key.getOid()
+        return "'{}': {}".format(key.prettyPrint(), value)
+
+    def oid_dict(self, key, value):
+        # type: (str, Dict[Any, Any]) -> str
+        """Display a dictionary of OID results with indexes.
+
+        This is tailored made for the structure we build for results in the check.
+
+        Output:
+            'ifInOctets: {'0': 3123, '1': 728}
+        """
+        values = []
+        have_indexes = False
+        for indexes, data in value.items():
+            try:
+                data = int(data)
+            except (TypeError, ValueError):
+                data = "'{}'".format(data)
+            if indexes:
+                values.append("'{}': {}".format('.'.join(indexes), data))
+                have_indexes = True
+            else:
+                values.append(str(data))
+
+        if not have_indexes:
+            displayed = values[0]
+        else:
+            displayed = '{{{}}}'.format(', '.join(values))
+        return "'{}': {}".format(key, displayed)
+
+    def __str__(self):
+        if isinstance(self.oids, dict):
+            return '{{{}}}'.format(', '.join(self.oid_dict(key, value) for (key, value) in self.oids.items()))
+        if self.with_values:
+            return '{{{}}}'.format(', '.join(self.oid_str_value(oid) for oid in self.oids))
+        else:
+            return '({})'.format(', '.join("'{}'".format(self.oid_str(oid)) for oid in self.oids))