paulscherrerinstitute / pcaspy

Portable Channel Access Server in Python
BSD 3-Clause "New" or "Revised" License
32 stars 24 forks source link

Log CA client's user/host name in write transactions #72

Open xiaoqiangwang opened 3 years ago

xiaoqiangwang commented 3 years ago

Similar to caPutLog in IOC, it would be good to be able to log CA client's information, i.e. user/host name. The following options are considered:

fernandohds564 commented 1 year ago

hi @xiaoqiangwang ! Any news on this thread?

xiaoqiangwang commented 1 year ago

I apologise for not tracking the issues for too long. I did a prototype immediately after our discussion, but did not pursue it to the end. I would need to find time to commit the changes.

xiaoqiangwang commented 1 year ago

I created a branch _logcaput implementing the 4th option "Use pcaspy builtin logger".

To enable the caput log,

import logging
logging.basicConfig(format='%(asctime)s: %(name)s %(message)s')
logging.getLogger('pcaspy.SimplePV.writeValue').setLevel(logging.INFO)

The log message is taken from caPutLog, i.e. "host user old= new=". But I am open to ideas.

2023-03-23 11:03:07,339: pcaspy.SimplePV.writeValue RAND: pc15530.psi.ch wang_x1  old=2.5 new=34.0
xresende commented 1 year ago

hi! let me give my share of opinion. Indeed having clean and standardized access to the CA client info at the Drive class level is very useful for logging and machine diagnostics purposes. I suggest implementing it as an additional argument do Driver.write method in a way that is does not break the previous signature use. CA client info being passed to Driver.write makes it very flexible for the CA server maintainer to implement whatever logging is needed (for example, in files separated from stdout, or in online databases, and so on...).

xiaoqiangwang commented 1 year ago

To keep backwards compatibility, I considered inspecting method write signature. If it has keyword arguments. But I decided against such magic.

class myDriver(Driver):

    def write(self, reason, value **kwargs):
        # use client info if it is present
        client = kwargs.get('client')

On the other hand, the logging module is sufficiently flexible to achieve what you described.

import logging

# Here comes first your general logging setup, e.g. two handlers with a custom formatter.
formatter = logging.Formatter('%(asctime)s: %(name)s %(message)s')
fh = logging.FileHandler('myioc.log')
fh.setFormatter(formatter)
sh = logging.StreamHandler()
sh.setFormatter(formatter)

# Now add these two log handlers to the caput logger
caput_logger = logging.getLogger('pcaspy.SimplePV.writeValue')
caput_logger.setLevel(logging.INFO)
caput_logger.addHandler(fh)
caput_logger.addHandler(sh)

For a database logger, you would need to write a custom handler.

fernandohds564 commented 1 year ago

Thank you for implementing this feature @xiaoqiangwang !

As you commented above, the most flexible solution would be passing this information to the Driver.write method, because this way we could decide how to use it, for example:

However, I understand your decision of not changing the signature of this method and I think this implementation fulfills our base case of use and will be very helpful on our ghost hunting here :ghost: :smile: .

xiaoqiangwang commented 1 year ago

I have put out the 0.8 release including the 4th option.