[tor-commits] [bridgedb/master] Cleanup logging of memory addresses in prepopulateRings().

[isis at torproject.org]

commit 6673109c8bf0da40123e53be3d1b37696ee503ab
Author: Isis Lovecruft <isis at torproject.org>
Date:   Sun Jan 5 17:33:08 2014 +0000

    Cleanup logging of memory addresses in prepopulateRings().
    
    The logging before was quite messy.
    
    The sub hashrings are identified by the HMAC of a unique string. It
    turns out that the "unique string" which is used in this case is
    actually something which looks like the class.__repr__() for a
    bridgedb.Bridges.BridgeHolder subclass. This is *probably* unique
    because it contains memory addresses of the newly generated hmac_fn()s,
    which are likely (though not necessarily) unique. This is a bit gross,
    in my opinion, but it's not really a problem.
    
    However, the log messages this function generated are a problem. They
    contain memory addresses, which isn't the greatest idea, ignoring all
    other reasons, simply because it's illegible.
    
     * REFACTOR logging statements in
       bridgedb.Bridges.FilteredBridgeSplitter.prepopulateRings() to remove
       logging of hmac_fn() memory addresses.
     * ADD a long comment detailing the "unique strings" which identify a
       sub hashring, and how they should probably be changed someday.
---
 lib/bridgedb/Bridges.py |   41 ++++++++++++++++++++++++++++++++++-------
 1 file changed, 34 insertions(+), 7 deletions(-)

diff --git a/lib/bridgedb/Bridges.py b/lib/bridgedb/Bridges.py
index c59d5ee..5b07141 100644
--- a/lib/bridgedb/Bridges.py
+++ b/lib/bridgedb/Bridges.py
@@ -1004,30 +1004,57 @@ class FilteredBridgeSplitter(BridgeHolder):
         :returns: False if there was a problem adding the subring, True
                   otherwise.
         """
+        # XXX I think subring and ringname are switched in this function, or
+        # at least that whatever is passed into this function as as the
+        # `ringname` parameter from somewhere else is odd; for example, with
+        # the original code, which was `log.debug("Inserted %d bridges into
+        # hashring '%s'!" % (inserted, ringname))`, this log message appears:
+        #
+        # Jan 04 23:18:37 [INFO] Inserted 12 bridges into hashring
+        # frozenset([<function filterBridgesByIP4 at 0x2d67cf8>, <function
+        # filterAssignBridgesToRing(<function hmac_fn at 0x3778398>, 4, 0) at
+        # 0x37de578>])!
+        #
+        # I suppose since it contains memory addresses, it *is* technically
+        # likely to be a unique string, but it is messy.
+
+        hashringName = self.__class__.__name__
+        subringName  = subring.__class__
+        filterNames  = []
+
         if not isinstance(subring, BridgeHolder):
-            logging.fatal("Can't add '%s' to %s because '%s' isn't a hashring."
-                          % (ringname, self.__class__, ringname))
+            logging.fatal("Can't add %r to %s because %r isn't a hashring."
+                          % (ringname, hashringName, ringname))
             return False
         if ringname in self.filterRings.keys():
             logging.fatal("Hashring %s already has a subring named '%s'!"
                           % (self.__class__, ringname))
             return False
 
-        logging.debug("Adding subring '%s' to hashring %s..."
-                      % (ringname, self.__class__))
+        for filterName in [x.func_name for x in list(ringname)]:
+            # Using `filterAssignBridgesToRing.func_name` gives us a messy
+            # string which includes all parameters and memory addresses. Get
+            # rid of this by partitioning at the first `(`:
+            realFilterName = filterName.partition('(')[0]
+            filterNames.append(realFilterName)
+        filterNames = ' '.join(filterNames)
+
+        logging.debug("Adding subring to %s hashring:" % hashringName)
+        logging.debug("    Subring class: %r" % subringName)
+        logging.debug("    Subring filters: %s" % filterNames)
 
         #TODO: drop LRU ring if len(self.filterRings) > self.max_cached_rings
         self.filterRings[ringname] = (filterFn, subring)
 
         if populate_from:
-            logging.info("Populating hashring %s..." % ringname)
+            logging.info("Populating hashring %s..." % subringName)
             inserted = 0
             for bridge in populate_from:
                 if isinstance(bridge, Bridge) and filterFn(bridge):
                     subring.insert(bridge)
                     inserted += 1
-            logging.info("Inserted %d bridges into hashring %s!"
-                         % (inserted, ringname))
+            logging.info("Inserted %d bridges into hashring with filters: %s"
+                         % (inserted, filterNames))
 
         return True