[PATCH] openbsc[master]: twisted_ipa.py: make debug logging more robust

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

Max gerrit-no-reply at lists.osmocom.org
Tue Mar 21 13:07:05 UTC 2017


Hello Neels Hofmeyr, Jenkins Builder,

I'd like you to reexamine a change.  Please visit

    https://gerrit.osmocom.org/2133

to look at the new patch set (#6).

twisted_ipa.py: make debug logging more robust

Do not print anything to stdout directly - use proper logger object
instead: either the one supplied by IPAFactory user or default to NO-OP
NullHandler logger.

Change-Id: Ic3417095a6e8848f0acabb46a9e64c0197b736e2
Related: SYS#3028
---
M openbsc/contrib/twisted_ipa.py
1 file changed, 32 insertions(+), 23 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/openbsc refs/changes/33/2133/6

diff --git a/openbsc/contrib/twisted_ipa.py b/openbsc/contrib/twisted_ipa.py
index 0a08747..c143852 100755
--- a/openbsc/contrib/twisted_ipa.py
+++ b/openbsc/contrib/twisted_ipa.py
@@ -22,13 +22,13 @@
  */
 """
 
-__version__ = "0.4" # bump this on every non-trivial change
+__version__ = "0.5" # bump this on every non-trivial change
 
 from ipa import Ctrl, IPA
 from twisted.internet.protocol import ReconnectingClientFactory
 from twisted.internet import reactor
 from twisted.protocols import basic
-import argparse
+import argparse, logging
 
 class IPACommon(basic.Int16StringReceiver):
     """
@@ -39,8 +39,7 @@
         """
         Debug print helper
         """
-        if self.factory.debug:
-            print(line)
+        self.factory.log.debug(line)
 
     def osmo_CTRL(self, data):
         """
@@ -160,7 +159,8 @@
         We have to resetDelay() here to drop internal state to default values to make reconnection logic work
         Make sure to call this via super() if overriding to keep reconnection logic intact
         """
-        self.dbg('IPA connection made!')
+        addr = self.transport.getPeer()
+        self.dbg('IPA connected to %s:%d peer' % (addr.host, addr.port))
         self.factory.resetDelay()
 
 
@@ -259,7 +259,8 @@
         Keep reconnection logic working by calling routine from CCM
         Initiate CCM upon connection
         """
-        print('IPA server connection made!')
+        addr = self.transport.getPeer()
+        self.factory.log.info('IPA server: connection from %s:%d client' % (addr.host, addr.port))
         super(IPAServer, self).connectionMade()
         self.transport.write(IPA().id_get())
 
@@ -275,7 +276,8 @@
         Send TRAP upon connection
         Note: we can't use sendString() because of it's incompatibility with IPA interpretation of length prefix
         """
-        print('CTRL server connection made!')
+        addr = self.transport.getPeer()
+        self.factory.log.info('CTRL server: connection from %s:%d client' % (addr.host, addr.port))
         super(CtrlServer, self).connectionMade()
         self.transport.write(Ctrl().trap('LOL', 'what'))
         self.transport.write(Ctrl().trap('rulez', 'XXX'))
@@ -287,14 +289,14 @@
         """
         CTRL SET command: always succeed
         """
-        print('SET [%s] %s' % (op_id, v))
+        self.dbg('SET [%s] %s' % (op_id, v))
         self.reply('SET_REPLY %s %s' % (op_id, v))
 
     def ctrl_GET(self, data, op_id, v):
         """
         CTRL GET command: always fail
         """
-        print('GET [%s] %s' % (op_id, v))
+        self.dbg('GET [%s] %s' % (op_id, v))
         self.reply('ERROR %s No variable found' % op_id)
 
 
@@ -304,31 +306,33 @@
     Note: so far we do not really need separate Factory for acting as a server due to protocol simplicity
     """
     protocol = IPACommon
-    debug = False
+    log = None
     ccm_id = IPA().identity(unit=b'1515/0/1', mac=b'b0:0b:fa:ce:de:ad:be:ef', utype=b'sysmoBTS', name=b'StingRay', location=b'hell', sw=IPA.version.encode('utf-8'))
 
-    def __init__(self, proto=None, debug=False, ccm_id=None):
+    def __init__(self, proto=None, log=None, ccm_id=None):
         if proto:
             self.protocol = proto
-        if debug:
-            self.debug = debug
         if ccm_id:
             self.ccm_id = ccm_id
+        if log:
+            self.log = log
+        else:
+            self.log = logging.getLogger('IPAFactory')
+            log.setLevel(logging.CRITICAL)
+            log.addHandler(logging.NullHandler)
 
     def clientConnectionFailed(self, connector, reason):
         """
         Only necessary for as debugging aid - if we can somehow set parent's class noisy attribute then we can omit this method
         """
-        if self.debug:
-            print('IPAFactory connection failed:', reason.getErrorMessage())
+        self.log.warning('IPAFactory connection failed: %s' % reason.getErrorMessage())
         ReconnectingClientFactory.clientConnectionFailed(self, connector, reason)
 
     def clientConnectionLost(self, connector, reason):
         """
         Only necessary for as debugging aid - if we can somehow set parent's class noisy attribute then we can omit this method
         """
-        if self.debug:
-            print('IPAFactory connection lost:', reason.getErrorMessage())
+        self.log.warning('IPAFactory connection lost: %s' % reason.getErrorMessage())
         ReconnectingClientFactory.clientConnectionLost(self, connector, reason)
 
 
@@ -345,29 +349,34 @@
     ic.add_argument("--ctrl", action='store_true', help="use CTRL protocol")
     args = p.parse_args()
     test = False
+
+    log = logging.getLogger('TwistedIPA')
+    log.setLevel(logging.DEBUG)
+    log.addHandler(logging.StreamHandler(sys.stdout))
+
     if args.ctrl:
         if args.client:
             # Start osmo-bsc to receive TRAP messages when osmo-bts-* connects to it
             print('CTRL client, connecting to %s:%d' % (args.host, args.port))
-            reactor.connectTCP(args.host, args.port, IPAFactory(CTRL, debug=True))
+            reactor.connectTCP(args.host, args.port, IPAFactory(CTRL, log))
             test = True
         if args.server:
             # Use bsc_control.py to issue set/get commands
             print('CTRL server, listening on port %d' % args.port)
-            reactor.listenTCP(args.port, IPAFactory(CtrlServer, debug=True))
+            reactor.listenTCP(args.port, IPAFactory(CtrlServer, log))
             test = True
     if args.ipa:
         if args.client:
             # Start osmo-nitb which would initiate A-bis/IP session
             print('IPA client, connecting to %s ports %d and %d' % (args.host, IPA.TCP_PORT_OML, IPA.TCP_PORT_RSL))
-            reactor.connectTCP(args.host, IPA.TCP_PORT_OML, IPAFactory(CCM, debug=True))
-            reactor.connectTCP(args.host, IPA.TCP_PORT_RSL, IPAFactory(CCM, debug=True))
+            reactor.connectTCP(args.host, IPA.TCP_PORT_OML, IPAFactory(CCM, log))
+            reactor.connectTCP(args.host, IPA.TCP_PORT_RSL, IPAFactory(CCM, log))
             test = True
         if args.server:
             # Start osmo-bts-* which would attempt to connect to us
             print('IPA server, listening on ports %d and %d' % (IPA.TCP_PORT_OML, IPA.TCP_PORT_RSL))
-            reactor.listenTCP(IPA.TCP_PORT_RSL, IPAFactory(IPAServer, debug=True))
-            reactor.listenTCP(IPA.TCP_PORT_OML, IPAFactory(IPAServer, debug=True))
+            reactor.listenTCP(IPA.TCP_PORT_RSL, IPAFactory(IPAServer, log))
+            reactor.listenTCP(IPA.TCP_PORT_OML, IPAFactory(IPAServer, log))
             test = True
     if test:
         reactor.run()

-- 
To view, visit https://gerrit.osmocom.org/2133
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic3417095a6e8848f0acabb46a9e64c0197b736e2
Gerrit-PatchSet: 6
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Max <msuraev at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Max <msuraev at sysmocom.de>
Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de>



More information about the gerrit-log mailing list