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

Aline Manera alinefm at linux.vnet.ibm.com
Mon Aug 22 18:36:23 UTC 2016



On 08/22/2016 02:56 PM, Lucio Correia wrote:
> 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).

I am not sure I got your point.

Why can't we add the params to the JSON?

For example:

error: {code: XXX, params: {<msg params>}}

>
>> 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.
>

Yeap! I'd say to remove it and make it a pure JSON file. So we identify 
it as an input file for a feature.

> 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'],
>>
>
>




More information about the Kimchi-devel mailing list