[Kimchi-devel] [PATCH] [Wok 2/5] Issue #142 - Translate request log at reading-time

Lucio Correia luciojhc at linux.vnet.ibm.com
Mon Aug 22 17:56:44 UTC 2016


On 22-08-2016 11:05, Aline Manera wrote:
>
>
> On 08/09/2016 11:42 AM, Lucio Correia wrote:
>>   * Add info needed for translation to user request log
>> entries (without unsafe parameters like passwwords)
>>   * Log message untranslated to base file (wok-req.log).
>>   * Keep compatibility to prior format of base log file,
>> which does not have needed information to do translation.
>
>> Old log entry format:
>> [logrecord] >>> [translated message]
>
>
>> New log entry format:
>> [logrecord] >>> [message params] >>> [untranslated message]
>
> Why don't include the message params to the JSON?

That's because [logrecord] is the dict with all key=values passed to the 
request, and we should not mix the parameters to the error message 
there, because it can overwrite a request parameter (independently of 
key name I choose).

> Why do we need to have the untranslated message there? This log file is
> only used as input for the user log activity feature. It is not done to
> be read as debug or so. For debug, we have the official logs under
> /var/log/wok.

We don't need it, but why not?

Since the beginning, I've designed it to be also a log file, with useful 
info (JSON was chosen because of that).

Remember that prior versions wok-req.log files contain the translated 
message there. Now that this has changed, I decided to keep its English 
version.

Let me know if you want a patch to remove it.

Thanks,
>
>> Signed-off-by: Lucio Correia <luciojhc at linux.vnet.ibm.com>
>> ---
>>   src/wok/control/base.py | 13 ++++++------
>>   src/wok/reqlogger.py    | 53
>> ++++++++++++++++++++++++++++++++++++++++---------
>>   src/wok/root.py         | 12 +++++------
>>   3 files changed, 56 insertions(+), 22 deletions(-)
>>
>> diff --git a/src/wok/control/base.py b/src/wok/control/base.py
>> index 6dfc977..f563aed 100644
>> --- a/src/wok/control/base.py
>> +++ b/src/wok/control/base.py
>> @@ -31,7 +31,6 @@ from wok.control.utils import get_class_name,
>> internal_redirect, model_fn
>>   from wok.control.utils import parse_request, validate_method
>>   from wok.control.utils import validate_params
>>   from wok.exception import InvalidOperation, UnauthorizedError,
>> WokException
>> -from wok.message import WokMessage
>>   from wok.reqlogger import RequestRecord
>>   from wok.stringutils import encode_value, utf8_dict
>>   from wok.utils import get_plugin_from_request, wok_log
>> @@ -156,10 +155,10 @@ class Resource(object):
>>                   # log request
>>                   code = self.getRequestMessage(method, action_name)
>>                   reqParams = utf8_dict(self.log_args, request)
>> -                msg = WokMessage(code,
>> reqParams).get_text(prepend_code=False)
>>                   RequestRecord(
>> -                    msg,
>> +                    reqParams,
>>                       app=get_plugin_from_request(),
>> +                    msgCode=code,
>>                       req=method,
>>                       status=status,
>>                       user=cherrypy.session.get(USER_NAME, 'N/A'),
>> @@ -216,10 +215,10 @@ class Resource(object):
>>               # log request
>>               if method not in LOG_DISABLED_METHODS:
>>                   code = self.getRequestMessage(method)
>> -                msg = WokMessage(code, self.log_args)
>>                   RequestRecord(
>> -                    msg.get_text(prepend_code=False),
>> +                    self.log_args,
>>                       app=get_plugin_from_request(),
>> +                    msgCode=code,
>>                       req=method,
>>                       status=status,
>>                       user=cherrypy.session.get(USER_NAME, 'N/A'),
>> @@ -455,10 +454,10 @@ class Collection(object):
>>                   # log request
>>                   code = self.getRequestMessage(method)
>>                   reqParams = utf8_dict(self.log_args, params)
>> -                msg = WokMessage(code,
>> reqParams).get_text(prepend_code=False)
>>                   RequestRecord(
>> -                    msg,
>> +                    reqParams,
>>                       app=get_plugin_from_request(),
>> +                    msgCode=code,
>>                       req=method,
>>                       status=status,
>>                       user=cherrypy.session.get(USER_NAME, 'N/A'),
>> diff --git a/src/wok/reqlogger.py b/src/wok/reqlogger.py
>> index fd02382..f82ae6b 100644
>> --- a/src/wok/reqlogger.py
>> +++ b/src/wok/reqlogger.py
>> @@ -30,6 +30,7 @@ from tempfile import NamedTemporaryFile
>>
>>   from wok.config import config, get_log_download_path
>>   from wok.exception import InvalidParameter, OperationFailed
>> +from wok.message import WokMessage
>>   from wok.stringutils import ascii_dict
>>   from wok.utils import remove_old_files
>>
>> @@ -55,6 +56,7 @@ SECONDS_PER_HOUR = 360
>>   TS_DATE_FORMAT = "%Y-%m-%d"
>>   TS_TIME_FORMAT = "%H:%M:%S"
>>   TS_ZONE_FORMAT = "%Z"
>> +UNSAFE_REQUEST_PARAMETERS = ['password', 'passwd']
>>
>>   # Log handler setup
>>   REQUEST_LOG_FILE = "wok-req.log"
>> @@ -112,6 +114,16 @@ class RequestParser(object):
>>
>>           return LOG_DOWNLOAD_URI % os.path.basename(fd.name)
>
>> +    def getTranslatedMessage(self, record, params):
>> +        code = record.get('msgCode', '')
>> +        app = record.get('app', 'wok')
>> +        plugin = None
>> +        if app != 'wok':
>> +            plugin = "/plugins/%s" % app
>> +
>
> You should not assume the plugin URI is something starting with '/plugins/'
> This is a plugin configuration and can be changed as the user wants.

OK, I will send a patch to fix this.

Thanks for the good catch.

>
>> +        msg = WokMessage(code, params, plugin)
>> +        return msg.get_text(prepend_code=False, translate=True)
>> +
>>       def getRecords(self):
>>           records = self.getRecordsFromFile(self.baseFile)
>>
>> @@ -140,7 +152,17 @@ class RequestParser(object):
>>                       data = line.split(">>>")
>>                       if len(data) > 1:
>>                           record = json.JSONDecoder().decode(data[0])
>> -                        record['message'] = data[1].strip()
>> +
>> +                        if len(data) > 2:
>> +                            # new log format: translate message on
>> the fly
>> +                            params = json.JSONDecoder().decode(data[1])
>> +                            msg = self.getTranslatedMessage(record,
>> params)
>> +                        else:
>> +                            # make it compatible with v2.2 log files,
>> which
>> +                            # messages are already translated
>> +                            msg = data[1].strip()
>> +
>> +                        record['message'] = msg
>>                           records.append(record)
>>
>>                       line = f.readline()
>> @@ -181,19 +203,32 @@ class RequestParser(object):
>>
>>
>>   class RequestRecord(object):
>> -    def __init__(self, message, **kwargs):
>> -        self.message = message
>> -        self.kwargs = kwargs
>> +    def __init__(self, msgParams, **kwargs):
>> +        # log record data
>> +        self.logData = kwargs
>> +
>> +        # data for message translation
>> +        self.code = self.logData['msgCode']
>> +        self.params = self.getSafeReqParams(msgParams)
>>
>>           # register timestamp in local time
>>           timestamp = time.localtime()
>> -        self.kwargs['date'] = time.strftime(TS_DATE_FORMAT, timestamp)
>> -        self.kwargs['time'] = time.strftime(TS_TIME_FORMAT, timestamp)
>> -        self.kwargs['zone'] = time.strftime(TS_ZONE_FORMAT, timestamp)
>> +        self.logData['date'] = time.strftime(TS_DATE_FORMAT, timestamp)
>> +        self.logData['time'] = time.strftime(TS_TIME_FORMAT, timestamp)
>> +        self.logData['zone'] = time.strftime(TS_ZONE_FORMAT, timestamp)
>> +
>> +    def getSafeReqParams(self, params):
>> +        result = params.copy()
>> +        for param in UNSAFE_REQUEST_PARAMETERS:
>> +            result.pop(param, None)
>> +        return result
>>
>>       def __str__(self):
>> -        info = json.JSONEncoder().encode(self.kwargs)
>> -        return '%s >>> %s' % (info, self.message)
>> +        msg = WokMessage(self.code, self.params)
>> +        msgText = msg.get_text(prepend_code=False, translate=False)
>> +        logData = json.JSONEncoder().encode(self.logData)
>> +        msgParams = json.JSONEncoder().encode(self.params)
>> +        return '%s >>> %s >>> %s' % (logData, msgParams, msgText)
>>
>>       def log(self):
>>           reqLogger = logging.getLogger(WOK_REQUEST_LOGGER)
>> diff --git a/src/wok/root.py b/src/wok/root.py
>> index 8601b71..08f4981 100644
>> --- a/src/wok/root.py
>> +++ b/src/wok/root.py
>> @@ -32,7 +32,6 @@ from wok.control import sub_nodes
>>   from wok.control.base import Resource
>>   from wok.control.utils import parse_request
>>   from wok.exception import MissingParameter
>> -from wok.message import WokMessage
>>   from wok.reqlogger import RequestRecord
>>
>>
>> @@ -161,13 +160,13 @@ class WokRoot(Root):
>>
>>           try:
>>               params = parse_request()
>> -            msg = WokMessage(code, params).get_text(prepend_code=False)
>>               username = params['username']
>>               password = params['password']
>>           except KeyError, item:
>>               RequestRecord(
>> -                msg,
>> +                params,
>>                   app=app,
>> +                msgCode=code,
>>                   req=method,
>>                   status=400,
>>                   user='N/A',
>> @@ -185,8 +184,9 @@ class WokRoot(Root):
>>               raise
>>           finally:
>>               RequestRecord(
>> -                msg,
>> +                params,
>>                   app=app,
>> +                msgCode=code,
>>                   req=method,
>>                   status=status,
>>                   user='N/A',
>> @@ -200,14 +200,14 @@ class WokRoot(Root):
>>           method = 'POST'
>>           code = self.getRequestMessage(method, 'logout')
>>           params = {'username': cherrypy.session.get(auth.USER_NAME,
>> 'N/A')}
>> -        msg = WokMessage(code, params).get_text(prepend_code=False)
>>           ip = cherrypy.request.remote.ip
>>
>>           auth.logout()
>>
>>           RequestRecord(
>> -            msg,
>> +            params,
>>               app='wok',
>> +            msgCode=code,
>>               req=method,
>>               status=200,
>>               user=params['username'],
>


-- 
Lucio Correia
Software Engineer
IBM LTC Brazil




More information about the Kimchi-devel mailing list