Merge branch 'SAR-103-support-remote-syslog' into 'master'

SAR-103 "Support remote syslog"

See merge request ska-telescope/lmc-base-classes!19

.release

-release=0.5.2
-tag=lmcbaseclasses-0.5.2
+release=0.5.3
+tag=lmcbaseclasses-0.5.3

README.md

@@ -25,6 +25,12 @@ The lmc-base-classe repository contains set of eight classes as mentioned in SKA
 
 ## Version History
 
+#### 0.5.3
+- Setting `loggingTargets` attribute to empty list no longer raises exception.
+- Change syslog targets in `loggingTargets` attribute to a full URL so that remote syslog servers can be specified.
+  For example, `"syslog::udp://server.domain:514"`, would send logs to `server.domain` via UDP port 514.
+  Specifying a path without a protocol, like `"syslog::/var/log"`, is deprecated.
+
 #### 0.5.2
 - Change ska_logger dependency to use ska-namespaced package (v0.3.0).  No change to usage.
 
@@ -251,8 +257,8 @@ current_targets = proxy.loggingTargets
 new_targets = list(current_targets) + ["file::/tmp/my.log"]
 proxy.loggingTargets = new_targets
 
-# disable all additional targets (empty list breaks, so include an empty string!)
-proxy.loggingTargets = ['']
+# disable all additional targets
+proxy.loggingTargets = []
 ```
 
 Currently there are three types of targets implemented:
@@ -271,12 +277,21 @@ the Tango name.  E.g., "my/test/device" would get the file "my_test_device.log".
 Currently, we using a `logging.handlers.RotatingFileHandler` with a 1 MB limit and
 just 2 backups.  This could be modified in future.
 
-For syslog, the syslog target address details must be provided after the `::`.
-This string is what ever you would pass to `logging.handlers.SysLogHandler`'s `address`
-argument.  E.g. `proxy.loggingTargets = ["syslog::/dev/log"]`.
+For syslog, the syslog target address details must be provided after the `::` as a URL.
+The following types are supported:
+- File, `file://<path>`
+  - E.g., for `/dev/log` use `file:///dev/log`.
+  - If the protocol is omitted, it is assumed to be `file://`.  Note: this is deprecated.
+    Support will be removed in v0.6.0.
+- Remote UDP server, `udp://<hostname>:<port>`
+  -  E.g., for `server.domain` on UDP port 514 use `udp://server.domain:514`.
+- Remote TCP server, `tcp://<hostname>:<port>`
+  -  E.g., for `server.domain` on TCP port 601 use `tcp://server.domain:601`.
+
+Example of usage:  `proxy.loggingTargets = ["syslog::udp://server.domain:514"]`.
 
 If you want file and syslog targets, you could do something like:
-`proxy.loggingTargets = ["file::/tmp/my.log", "syslog::/dev/log"]`.
+`proxy.loggingTargets = ["file::/tmp/my.log", "syslog::udp://server.domain:514"]`.
 
 **Note:**  There is a limit of 3 additional handlers.  That the maximum length
 of the spectrum attribute. We could change this if there is a reasonable use

src/ska/base/base_device.py

@@ -15,8 +15,13 @@ import json
 import logging
 import logging.handlers
 import os
+import socket
 import sys
 import threading
+import warnings
+
+from urllib.parse import urlparse
+from urllib.request import url2pathname
 
 # Tango imports
 import tango
@@ -75,7 +80,7 @@ class LoggingUtils:
 
         :param target:
             List of candidate logging target strings, like '<type>[::<name>]'
-            Empty and whitespace-only strings are ignored.
+            Empty and whitespace-only strings are ignored.  Can also be None.
 
         :param device_name:
             TANGO device name, like 'domain/family/member', used
@@ -91,6 +96,7 @@ class LoggingUtils:
             "syslog": None}
 
         valid_targets = []
+        if targets:
             for target in targets:
                 target = target.strip()
                 if not target:
@@ -114,6 +120,70 @@ class LoggingUtils:
 
         return valid_targets
 
+    @staticmethod
+    def get_syslog_address_and_socktype(url):
+        """Parse syslog URL and extract address and socktype parameters for SysLogHandler.
+
+        :param url:
+            Universal resource locator string for syslog target.  Three types are supported:
+            file path, remote UDP server, remote TCP server.
+            - Output to a file:  'file://<path to file>'
+              Example:  'file:///dev/log' will write to '/dev/log'
+            - Output to remote server over UDP:  'udp://<hostname>:<port>'
+              Example:  'udp://syslog.com:514' will send to host 'syslog.com' on UDP port 514
+            - Output to remote server over TCP:  'tcp://<hostname>:<port>'
+              Example:  'tcp://rsyslog.com:601' will send to host 'rsyslog.com' on TCP port 601
+            For backwards compatibility, if the protocol prefix is missing, the type is
+            interpreted as file.  This is deprecated.
+            - Example:  '/dev/log' is equivalent to 'file:///dev/log'
+
+        :return: (address, socktype)
+            For file types:
+            - address is the file path as as string
+            - socktype is None
+            For UDP and TCP:
+            - address is tuple of (hostname, port), with hostname a string, and port an integer.
+            - socktype is socket.SOCK_DGRAM for UDP, or socket.SOCK_STREAM for TCP.
+
+        :raises: LoggingTargetError for invalid url string
+        """
+        address = None
+        socktype = None
+        parsed = urlparse(url)
+        if parsed.scheme in ["file", ""]:
+            address = url2pathname(parsed.netloc + parsed.path)
+            socktype = None
+            if not address:
+                raise LoggingTargetError(
+                    "Invalid syslog URL - empty file path from '{}'".format(url)
+                )
+            if parsed.scheme == "":
+                warnings.warn(
+                    "Specifying syslog URL without protocol is deprecated, "
+                    "use 'file://{}' instead of '{}'".format(url, url),
+                    DeprecationWarning,
+                )
+        elif parsed.scheme in ["udp", "tcp"]:
+            if not parsed.hostname:
+                raise LoggingTargetError(
+                    "Invalid syslog URL - could not extract hostname from '{}'".format(url)
+                )
+            try:
+                port = int(parsed.port)
+            except (TypeError, ValueError):
+                raise LoggingTargetError(
+                    "Invalid syslog URL - could not extract integer port number from '{}'".format(
+                        url
+                    )
+                )
+            address = (parsed.hostname, port)
+            socktype = socket.SOCK_DGRAM if parsed.scheme == "udp" else socket.SOCK_STREAM
+        else:
+            raise LoggingTargetError(
+                "Invalid syslog URL - expected file, udp or tcp protocol scheme in '{}'".format(url)
+            )
+        return address, socktype
+
     @staticmethod
     def create_logging_handler(target):
         """Create a Python log handler based on the target type (console, file, syslog)
@@ -136,8 +206,11 @@ class LoggingUtils:
             handler = logging.handlers.RotatingFileHandler(
                 log_file_name, 'a', LOG_FILE_SIZE, 2, None, False)
         elif target_type == "syslog":
+            address, socktype = LoggingUtils.get_syslog_address_and_socktype(target_name)
             handler = logging.handlers.SysLogHandler(
-                    address=target_name, facility=logging.handlers.SysLogHandler.LOG_SYSLOG)
+                address=address,
+                facility=logging.handlers.SysLogHandler.LOG_SYSLOG,
+                socktype=socktype)
         else:
             raise LoggingTargetError(
                 "Invalid target type requested: '{}' in '{}'".format(target_type, target))

src/ska/base/release.py

@@ -7,7 +7,7 @@
 """Release information for lmc-base-classes Python Package"""
 
 name = """lmcbaseclasses"""
-version = "0.5.2"
+version = "0.5.3"
 version_info = version.split(".")
 description = """A set of generic base devices for SKA Telescope."""
 author = "SKA India and SARAO"

tests/test_base_device.py

@@ -16,6 +16,8 @@ import pytest
 
 # PROTECTED REGION ID(SKABaseDevice.test_additional_imports) ENABLED START #
 import logging
+import socket
+
 from unittest import mock
 from tango import DevFailed, DevState
 from ska.base.control_model import (
@@ -32,6 +34,7 @@ from ska.base.base_device import (
 
 class TestLoggingUtils:
     @pytest.fixture(params=[
+            (None, []),
             ([""], []),
             ([" \n\t "], []),
             (["console"], ["console::cout"]),
@@ -41,7 +44,9 @@ class TestLoggingUtils:
             (["file"], ["file::my_dev_name.log"]),
             (["file::"], ["file::my_dev_name.log"]),
             (["file::/tmp/dummy"], ["file::/tmp/dummy"]),
-            (["syslog::some/address"], ["syslog::some/address"]),
+            (["syslog::some/path"], ["syslog::some/path"]),
+            (["syslog::file://some/path"], ["syslog::file://some/path"]),
+            (["syslog::protocol://somehost:1234"], ["syslog::protocol://somehost:1234"]),
             (["console", "file"], ["console::cout", "file::my_dev_name.log"]),
         ])
     def good_logging_targets(self, request):
@@ -61,18 +66,58 @@ class TestLoggingUtils:
         dev_name = "my/dev/name"
         return targets_in, dev_name
 
-
     def test_sanitise_logging_targets_success(self, good_logging_targets):
         targets_in, dev_name, expected = good_logging_targets
         actual = LoggingUtils.sanitise_logging_targets(targets_in, dev_name)
         assert actual == expected
 
-
     def test_sanitise_logging_targets_fail(self, bad_logging_targets):
         targets_in, dev_name = bad_logging_targets
         with pytest.raises(LoggingTargetError):
             LoggingUtils.sanitise_logging_targets(targets_in, dev_name)
 
+    @pytest.fixture(params=[
+            ("deprecated/path", ["deprecated/path", None]),
+            ("file:///abs/path", ["/abs/path", None]),
+            ("file://relative/path", ["relative/path", None]),
+            ("file://some/spaced%20path", ["some/spaced path", None]),
+            ("udp://somehost.domain:1234", [("somehost.domain", 1234), socket.SOCK_DGRAM]),
+            ("udp://127.0.0.1:1234", [("127.0.0.1", 1234), socket.SOCK_DGRAM]),
+            ("tcp://somehost:1234", [("somehost", 1234), socket.SOCK_STREAM]),
+            ("tcp://127.0.0.1:1234", [("127.0.0.1", 1234), socket.SOCK_STREAM]),
+    ])
+    def good_syslog_url(self, request):
+        url, (expected_address, expected_socktype) = request.param
+        return url, (expected_address, expected_socktype)
+
+    @pytest.fixture(params=[
+        None,
+        "",
+        "file://",
+        "udp://",
+        "udp://somehost",
+        "udp://somehost:",
+        "udp://somehost:not_integer_port",
+        "udp://:1234",
+        "tcp://",
+        "tcp://somehost",
+        "tcp://somehost:",
+        "tcp://somehost:not_integer_port",
+        "tcp://:1234",
+        "invalid://somehost:1234"
+    ])
+    def bad_syslog_url(self, request):
+        return request.param
+
+    def test_get_syslog_address_and_socktype_success(self, good_syslog_url):
+        url, (expected_address, expected_socktype) = good_syslog_url
+        actual_address, actual_socktype = LoggingUtils.get_syslog_address_and_socktype(url)
+        assert actual_address == expected_address
+        assert actual_socktype == expected_socktype
+
+    def test_get_syslog_address_and_socktype_fail(self, bad_syslog_url):
+        with pytest.raises(LoggingTargetError):
+            LoggingUtils.get_syslog_address_and_socktype(bad_syslog_url)
 
     @mock.patch('logging.handlers.SysLogHandler')
     @mock.patch('logging.handlers.RotatingFileHandler')
@@ -101,7 +146,22 @@ class TestLoggingUtils:
         assert handler == mock_file_handler()
         handler.setFormatter.assert_called_once_with(mock_formatter)
 
-        handler = LoggingUtils.create_logging_handler("syslog::some/address")
+        handler = LoggingUtils.create_logging_handler("syslog::udp://somehost:1234")
+        mock_syslog_handler.assert_called_once_with(
+            address=("somehost", 1234),
+            facility=mock_syslog_handler.LOG_SYSLOG,
+            socktype=socket.SOCK_DGRAM
+        )
+        assert handler == mock_syslog_handler()
+        handler.setFormatter.assert_called_once_with(mock_formatter)
+
+        mock_syslog_handler.reset_mock()
+        handler = LoggingUtils.create_logging_handler("syslog::file:///tmp/path")
+        mock_syslog_handler.assert_called_once_with(
+            address="/tmp/path",
+            facility=mock_syslog_handler.LOG_SYSLOG,
+            socktype=None
+        )
         assert handler == mock_syslog_handler()
         handler.setFormatter.assert_called_once_with(mock_formatter)
 
@@ -299,15 +359,19 @@ class TestSKABaseDevice(object):
             # test adding file and syslog targets (already have console)
             mocked_creator.reset_mock()
             tango_context.device.loggingTargets = [
-                "console::cout", "file::/tmp/dummy", "syslog::some/address"]
+                "console::cout", "file::/tmp/dummy", "syslog::udp://localhost:514"]
             assert tango_context.device.loggingTargets == (
-                "console::cout", "file::/tmp/dummy", "syslog::some/address")
+                "console::cout", "file::/tmp/dummy", "syslog::udp://localhost:514")
             assert mocked_creator.call_count == 2
             mocked_creator.assert_has_calls(
                 [mock.call("file::/tmp/dummy"),
-                 mock.call("syslog::some/address")],
+                 mock.call("syslog::udp://localhost:514")],
                 any_order=True)
 
+            # test clearing all targets (note: PyTango returns None for empty spectrum attribute)
+            tango_context.device.loggingTargets = []
+            assert tango_context.device.loggingTargets is None
+
             mocked_creator.reset_mock()
             with pytest.raises(DevFailed):
                 tango_context.device.loggingTargets = ["invalid::type"]