BIND 10 #2746: allow loading log messages manually and use it for Python datasrc

[BIND 10 Development]

#2746: allow loading log messages manually and use it for Python datasrc
-------------------------------------+-------------------------------------
            Reporter:  jinmei        |                        Owner:
                Type:  defect        |                       Status:  new
            Priority:  medium        |                    Milestone:  Next-
           Component:  logging       |  Sprint-Proposed
            Keywords:                |                   Resolution:
           Sensitive:  0             |                 CVSS Scoring:
         Sub-Project:  Core          |              Defect Severity:  N/A
Estimated Difficulty:  0             |  Feature Depending on Ticket:
         Total Hours:  0             |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------
Description changed by jinmei:

Old description:

> The problem described in #2743 is because cfgmgr calls isc.log.init()
> in its module (called at import time) and then try to load datasrc
> module as a plugin when main() is called:
> {{{#!python
>         for ppath in PLUGIN_PATHS:
>             load_plugins(ppath, cm)
> }}}
>
> I suspect similar thing can happen for a C/C++ loadable module if the
> module has its own messages (defined in the module image) and is
> dynamically loaded, e.g., as a result of configuration update.
>
> So I propose generic solution that should work for all such cases:
>
> - update `PyInit_datasrc()` so that it'll call
>   `MessageInitializer::loadDictionary()`
>
> This should be sufficient for the specific issue described in #2743.
> Note also that if we do this, we won't have to merge #2743
> (trac2145-revert).  But for a comprehensive fix I propose something
> more:
>
> - update `MessageInitializer::loadDictionary()` so it can take extra
>   (optional) variable to ignore duplicates
> - in libdatasrc, define SQLite3 specific log messages separately
>   (moving from datasrc_mesasges.mes to, e.g.,
>   sqlite3_datasrc_messages.mes), build corresponding .cc only with
>   the sqlite3 accessor loadable module, and call `loadDictionary()`
>   from sqlite3_accessor_link.cc (probably from `createInstance()`).
>   Confirm the log messages are really shown.  Enable "ignore dup" when
>   calling `loadDictionary()` (otherwise if the module is unloaded and
>   loaded again we'll see duplicate warning).  This will work as a
>   proof-of-concept example.
> - (optional but) I also suggest changing `MessageInitializer` from a
>   class to a namespace; it doesn't seem to have to be a class and
>   actually only used as a namespace.  IMO it's an abuse of the concept
>   of class and is confusing.
> - (optional and not directly related) I suggest clearing the
>   "duplicate" vector in `LoggerManager::init` after warning about
>   duplicates; otherwise the vector will potentially get larger and
>   larger over time.  And, in doing so, I'd also suggest introducing a
>   separate method/function to do so and let `getDuplicates()` return a
>   const reference (Returning a mutable reference is generally a bad
>   practice.)
> - create a wiki page or some other form of doc for tips of developing
>   BIND 10 modules.  there, explain that native-C++ or python wrapper
>   modules should call `MessageInitializer::loadDictionary()` if that
>   module is supposed to use log messages.

New description:

 The problem described in #2743 is because cfgmgr calls isc.log.init()
 in its module (called at import time) and then try to load datasrc
 module as a plugin when main() is called:
 {{{#!python
         for ppath in PLUGIN_PATHS:
             load_plugins(ppath, cm)
 }}}

 I suspect similar thing can happen for a C/C++ loadable module if the
 module has its own messages (defined in the module image) and is
 dynamically loaded, e.g., as a result of configuration update.

 So I propose generic solution that should work for all such cases:

 - update `PyInit_datasrc()` so that it'll call
   `MessageInitializer::loadDictionary()`

 This should be sufficient for the specific issue described in #2743.
 Note also that if we do this, we won't have to merge #2743
 (trac2145-revert).  But for a comprehensive fix I propose something
 more:

 - update `MessageInitializer::loadDictionary()` so it can take extra
   (optional) variable to ignore duplicates
 - in libdatasrc, define SQLite3 specific log messages separately
   (moving from datasrc_mesasges.mes to, e.g.,
   sqlite3_datasrc_messages.mes), build corresponding .cc only with
   the sqlite3 accessor loadable module, and call `loadDictionary()`
   from sqlite3_accessor_link.cc (probably from `createInstance()`).
   Confirm the log messages are really shown.  Enable "ignore dup" when
   calling `loadDictionary()` (otherwise if the module is unloaded and
   loaded again we'll see duplicate warning).  This will work as a
   proof-of-concept example.
 - (optional but) I also suggest changing `MessageInitializer` from a
   class to a namespace; it doesn't seem to have to be a class and
   actually only used as a namespace.  IMO it's an abuse of the concept
   of class and is confusing.
 - (optional and not directly related) I suggest clearing the
   "duplicate" vector in `LoggerManager::init` after warning about
   duplicates; otherwise the vector will potentially get larger and
   larger over time.  And, in doing so, I'd also suggest introducing a
   separate method/function to do so and let `getDuplicates()` return a
   const reference (Returning a mutable reference is generally a bad
   practice.)
 - create a wiki page or some other form of doc for tips of developing
   BIND 10 modules.  there, explain that native-C++ or python wrapper
   modules should call `MessageInitializer::loadDictionary()` if that
   module is supposed to use log messages.  We should also note here
   that this is not a thread-safe operation, so if it's done in the
   middle of running a threaded application it should be ensured at a
   higher layer that other threads get access to logging.

--

-- 
Ticket URL: <http://bind10.isc.org/ticket/2746#comment:1>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development