Clean up wbemconnection logger interface.

[KSchopmeyer]

We agreed that the current interfaces for defining logging for wbemconnections is not clean, to heavyweight and that we want to change it before we remove the experimental from this api.

[andy-maier]

We agreed that each of us would come forward with a proposal.

[KSchopmeyer]

Client logging at the api level and at the xml level. Uses two named loggers.

We agreed to drop the logger on the mocker since it duplicates what the cim_operations logger does.

We have the capability to define the loggers from a formatted string which defines each logger by name, the destination, and whether is is full display or summary display (i.e. 3 pieces of data for each logger) name:dest::detail or simething like that.

[KSchopmeyer]

We agreed with the release of 0.11.0 that the pywbemlogger class was an experimental means of configuring the pywbem logger. the goal of this was to define a convient means for users of pywbem to configure the specific loggers used by cim_operations to log request APIs and their responses and also the xml generated given that we wanted to add one new piece of functionality not normally in loggers, the ability to define multiple output formats (detail or summary) for each logger.

We have found several issues with this implementation but cannot find anything better given that we are adding data to the logger configuration.

Specifically:

The documentation is oriented towards the implementation, not the user.

The documentation is incomplete and a number of characteristics such as defaults is not clarified.

It is not really a singleton in that it does not require the creation of a single object but uses the class level variables to store the configuration data

We did agree to keep the method that uses a string input to define the loggers as a common tool for clis that use pywbem.

[KSchopmeyer]

I did the clean up and hate the result. Here are some issues I found and a proposed solution:

Some of the issues with the current implementation

1. We have logging configuration spread out in two places (the basic configuration is in PywbemLoggers) but we set the size parameter when we enable a logger by creating the instanceobject of LogOperationRecorder.

2. There is no simple default that allows a default logger to be setup with just a single statement (ile. add_logger(). If you do that now, nothing is logged since the default for logger destinations is None. The default ought to be something that creates a log output so that the only thing minimally necessary to set up a loger would be to:

   a. Create the wbemconnection b. conn.add_operation_recorderr(LogOperationRecorder())

This would create a default logger that actually output something for both xml and the apis.

3. Having to setup the logger in a separate class now means that you have to do three things to enable a logger. a. Create the wbemconnection b. Define the loggers in PywbemLoggers by using one of the create methods. ex. PywbemLogger.createLoggers('all=stdout:detail') c. conn.add_operation_recorder(LogOperationRecorder(max_log_entry_size=300))

4. The for PywbemLoggers is not really connected to the add_logger so you have to find the info in two different places. It would probably be more logical if you were to set the logger parameters to non-default values directly as part of the LogOperationRecorder init so that if init parameters were to be set they could be set as part of adding the logger.

   a. Create the wbemconnection conn = WBEMConnection(...) b. conn. add_operation_logger(LogOperationRecorder(loggerconfig=xxx, log_max_entry_size=300))

today is is: a. conn = WBEMConnection(...) b' PywbemLoggers.create_loggers('all=stderr,min') b. conn.add_operation_logger(LogOperationRecorder(loggerconfig=xxx, log_max_entry_size=300))

5. Logging is universal and cannot really differ by connection because we set a named logger with parameters in PyWBEMLoggers and then use this definition in multiple connections.

6. There are a couple of issues with this right now. a. using the logger by name means that we fix the characteristics of the logger once and tie it to a logger name and then use that name as the means to control logging in the recorder.

How we can do better.

Proposed Alternative

1. Drop the name loggers and use the LogOperationRecorder init as the config setting tool for the logger. All configuration will be internal to this constructor.

However, this represents a different problem if we want to configure both loggers, we have either many parameters on the init or have to have some means to set multiple loggers.

Using the string approach is simple since there is only a single parameter, a string..

conn.add_operation_logger(LogOperationRecorder(loggerconfig='all=stderr:detail', 
    log_max_entry_size=300))

Or better yet. lets build the size parameter into the log config definition: conn.add_operation_logger(LogOperationRecorder(config='all=stderr:detail:300')

But while that is one option it depends on a string with parameters to define the configuraiton and we really want an alternative that is more precise. But we want to possibly set up multiple loggers so the init would have to handle multiples of a config structure to do this.

Maybe something like: For the string config def: conn.add_operation_logger(LogOperationRecorder(configstr='all=stderr:detail:300')

and for input as multiple parameter sets, one for each logger.

    conn.add_operation_logger(LogOperationRecorder(config=[('ops', dest='stderr', detail='min'), 
        ('xml', dest=file)])

or to set up just a single logger conn.add_operation_logger(LogOperationRecorder(config=('ops', dest='stderr', detail='min')

  which would set up both loggers.  Note I would like to do something besides strings for things like log component but have not thought that out.  Implementation wise this looks like we could defined a named tuple and single or lists of the named tuple would be the input.

The changes would be:

a. Extend LogOperationRecorder constructor to handle the above formats so that all definition of the loggers would be part of the LogOperationRecorder constructor.

b. Discard the PyWbemLoggers class and redo some of PywbemLoggersmethods to be a local module that would be either part of the recorder or a separate internal component that the recorder init would call to process the constructor inputs.. I propose that it become part of the recorder module. This also means we get rid of the class level methods, etc. and simplify the whole thing significantly.

The LogOperationRecorder init could look somethig like:

  def __init__(self, configstr=None, config=None, size=DEFAULT_MAX_LOG_ENTRY_SIZE, 
                      log_file=None):   
    """   TODO """  
     configstr (:term:`string`)
       Logging configuration defined as a single string where the string format is as follows:

      input_str (:term:`string`): Specifies the logger definitions
        as follows:

        ``log_specs`` := ``log_spec`` [, ``log_spec`` ]

        ``log_spec`` := ``log_comp`` ['=' [ ``dest`` ][":"[ ``detail_level`` ]]]]

        where:

        * ``log_comp``: Must be one of strings in the
          :data:`~pywbem._logging.LOG_COMPONENTS` list.

        * ``detail_level``: Must be one of strings in the
          :data:`~pywbem._logging.LOG_DETAIL_LEVELS` list.

        * ``dest``: Must be one of strings in the
          :data:`~pywbem._logging.LOG_DESTINATIONS` list.

    config (list of named_tuple or named_tuple)
      Either the config parameter or the configstr parameter can be defined but NOT both.
     TODO: Flesh this definition out

   size (:term:`integer`):
        The maximum size for each log entry, in Bytes. This is primarily to
        limit response sizes since they could be very large.

        If `None`, the maximum size defaults to
        :attr:`~pywbem._logging.DEFAULT_MAX_LOG_ENTRY_SIZE`.

      log_filename (:term:`string`):
        Filename to use as logging file if the log destination is `file`.
        Ignored if log destination is not `file`. If value is `None` and
        this is a ``file`` log, ValueError is raised. 

[KSchopmeyer]

How do we fit this into a larger world where we are just a part of the infrastructure and the user wants more control over loggers.

Could we give control back completely to the user to set up the loggers by name. We could further break this down by using the hierarchical nature of the logger name.

Thus the basic logger for ops is pywbem.ops. we could log by connection by having pywbem.ops.<connectionid).

Lets map the detail level to info and debug level loggers.

We also propose that we internally add the recorder by adding a new parameter to the wbemconnection that will be None, short, detail. If not None this will set the logoperationlogger and the size parameters. Everything else is to be set up by the user in creating the logger name, either pywbem.xxx or pywbem.xxx. if they only want it for this connection.

We will keep the code for the string parser as a separate function.

[andy-maier]

Fixed in PR #1105.