[tor-bugs] #9199 [BridgeDB]: Rethink the logging of BridgeDB

[Tor Bug Tracker & Wiki]

#9199: Rethink the logging of BridgeDB
----------------------+-----------------------------------------------------
 Reporter:  asn       |          Owner:                   
     Type:  task      |         Status:  needs_information
 Priority:  normal    |      Milestone:                   
Component:  BridgeDB  |        Version:                   
 Keywords:            |         Parent:                   
   Points:            |   Actualpoints:                   
----------------------+-----------------------------------------------------

Comment(by sysrqb):

 Replying to [comment:4 isis]:
 > Oops. I forgot to mark on #3797 that I was working on the logger, but a
 general ticket is better. Also, oops: sysrqb, I don't mean to step on your
 toes if you're working on this, but I'll post what I've already done.

 1) Communication :)
 2) No worries, please continue! I'll stay away from rewriting the logger,
 and I'll concentrate on improving /what/ we log, but not /how/ we log it.
 It'll be easy to merge the former into any changes you make regarding the
 latter.

 >
 > In reply to [comment:2 asn]:
 > > Hm. How hard would it be to switch to twisted.logging? What are the
 advantages of using Twisted logging? Do we care about them?
 >
 > The stdlib logging module, and most of stdlib´s IO utility modules as
 well, are not compatible with Twisted, due to the default way that Python
 handles file locks. From what I've understood from discussions on the
 twisted-dev list, using any of these modules can result in the weird
 Twisted bugs that pop up one-in-every-thirty-seven-times the program is
 run, and in general suck to try to debug.
 >
 > My updates to the logging facilities (using Twisted's logger) and
 configuration loader so far are in
 [https://github.com/isislovecruft/bridgedb/tree/feature/9199-improved-
 logging this branch on Github].
 >

 Very nice! Some initial thoughts:
 1) if you rebase your commits, you might want to move the commit with the
 new log.py (552f12b5fd9a) to be the first commit because many later
 commits depend on it.
 2) If we try to touch/create a file/dir and fail, do we want to exit?
 3) I think creating a Util module is fine. We may have some use for it
 later. We can probably move getkey() there also.
 4) In 90628d663030d7cd1cd line 151
 {{{
 +        if os.access(self.file, os.R_OK):
 +            log.msg("Loading config file: %s" % self.file)
 +            new = execfile(self.file, self.settings)
 +            self.settings.update(**new)
 }}}

 Would it be better to handle the exception than deal with TOCTOU? This
 isn't really within the threat model of bridgedb, but I'll raise it for
 awareness.

 > As far as asn's ideas at the beginning of this ticket go, setting up a
 Filter for log emission to remove IP addresses shouldn't be too hard. And
 if there's any useful info that could safely be collected for metric or
 something else, that should be doable, though I don't know what info that
 might be as I am not super familiar with BridgeDB yet.

 In the past we've simply wrapped any PII with a function that either
 outputs "[scrubbed]" or
 outputs the input. I can easily integrate this into your branch.

 >
 > I was also thinking that it might be a good idea to rewrite the
 startup() parts of Main.py to be a twisted.application.Application, as
 then we would nearly-automatically get different logger contexts for
 different services/servers, daemonisation, resource control, and the like,
 though I should probably create a separate ticket for this.
 Yeah, this deserves its own ticket, but this sounds like a wise choice.

-- 
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/9199#comment:6>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online