From 34ca7ab4b5f5c04986bc97e13cf411a1e8d7334e Mon Sep 17 00:00:00 2001 From: Ilya Shakhat Date: Mon, 8 Apr 2019 16:43:45 +0200 Subject: Handle driver initialization errors to avoid service crash This patch fixes the issue when failed osprofiler driver brings the whole service down. With this patch the default no-op behaviour is used in case of initialization failure. Change-Id: I6ebc393576f4fc3f8b4134164bafc2e09f102ebd --- osprofiler/drivers/elasticsearch_driver.py | 6 ++--- osprofiler/drivers/jaeger.py | 4 ++-- osprofiler/drivers/mongodb.py | 6 ++--- osprofiler/drivers/redis_driver.py | 6 ++--- osprofiler/notifier.py | 35 +++++++++++++++++++++--------- osprofiler/tests/unit/test_notifier.py | 9 ++++++++ 6 files changed, 45 insertions(+), 21 deletions(-) diff --git a/osprofiler/drivers/elasticsearch_driver.py b/osprofiler/drivers/elasticsearch_driver.py index 85bab74..c24eb60 100644 --- a/osprofiler/drivers/elasticsearch_driver.py +++ b/osprofiler/drivers/elasticsearch_driver.py @@ -36,9 +36,9 @@ class ElasticsearchDriver(base.Driver): from elasticsearch import Elasticsearch except ImportError: raise exc.CommandError( - "To use this command, you should install " - "'elasticsearch' manually. Use command:\n " - "'pip install elasticsearch'.") + "To use OSProfiler with ElasticSearch driver, " + "please install `elasticsearch` library. " + "To install with pip:\n `pip install elasticsearch`.") client_url = parser.urlunparse(parser.urlparse(self.connection_str) ._replace(scheme="http")) diff --git a/osprofiler/drivers/jaeger.py b/osprofiler/drivers/jaeger.py index 669aa13..82b7a23 100644 --- a/osprofiler/drivers/jaeger.py +++ b/osprofiler/drivers/jaeger.py @@ -40,8 +40,8 @@ class Jaeger(base.Driver): except ImportError: raise exc.CommandError( "To use OSProfiler with Uber Jaeger tracer, " - "you have to install `jaeger-client` manually. " - "Install with pip:\n `pip install jaeger-client`." + "please install `jaeger-client` library. " + "To install with pip:\n `pip install jaeger-client`." ) parsed_url = parser.urlparse(connection_str) diff --git a/osprofiler/drivers/mongodb.py b/osprofiler/drivers/mongodb.py index 0ca1928..a0372f6 100644 --- a/osprofiler/drivers/mongodb.py +++ b/osprofiler/drivers/mongodb.py @@ -28,9 +28,9 @@ class MongoDB(base.Driver): from pymongo import MongoClient except ImportError: raise exc.CommandError( - "To use this command, you should install " - "'pymongo' manually. Use command:\n " - "'pip install pymongo'.") + "To use OSProfiler with MongoDB driver, " + "please install `pymongo` library. " + "To install with pip:\n `pip install pymongo`.") client = MongoClient(self.connection_str, connect=False) self.db = client[db_name] diff --git a/osprofiler/drivers/redis_driver.py b/osprofiler/drivers/redis_driver.py index 250a81c..0952702 100644 --- a/osprofiler/drivers/redis_driver.py +++ b/osprofiler/drivers/redis_driver.py @@ -40,9 +40,9 @@ class Redis(base.Driver): from redis import StrictRedis except ImportError: raise exc.CommandError( - "To use this command, you should install " - "'redis' manually. Use command:\n " - "'pip install redis'.") + "To use OSProfiler with Redis driver, " + "please install `redis` library. " + "To install with pip:\n `pip install redis`.") # only connection over network is supported with schema # redis://[:password]@host[:port][/db] diff --git a/osprofiler/notifier.py b/osprofiler/notifier.py index 3bd4412..8b909f0 100644 --- a/osprofiler/notifier.py +++ b/osprofiler/notifier.py @@ -13,16 +13,21 @@ # License for the specific language governing permissions and limitations # under the License. +import logging + from osprofiler.drivers import base +LOG = logging.getLogger(__name__) + + def _noop_notifier(info, context=None): """Do nothing on notify().""" # NOTE(boris-42): By default we are using noop notifier. __notifier = _noop_notifier -__driver_cache = {} +__notifier_cache = {} # map: connection-string -> notifier def notify(info): @@ -54,14 +59,24 @@ def create(connection_string, *args, **kwargs): :param connection_string: connection string which specifies the storage driver for notifier - :param *args: args that will be passed to the driver's __init__ method - :param **kwargs: kwargs that will be passed to the driver's __init__ method + :param args: args that will be passed to the driver's __init__ method + :param kwargs: kwargs that will be passed to the driver's __init__ method :returns: Callable notifier method - :raises TypeError: In case of invalid name of plugin raises TypeError """ - global __driver_cache - if connection_string not in __driver_cache: - __driver_cache[connection_string] = base.get_driver(connection_string, - *args, - **kwargs).notify - return __driver_cache[connection_string] + global __notifier_cache + if connection_string not in __notifier_cache: + try: + driver = base.get_driver(connection_string, *args, **kwargs) + __notifier_cache[connection_string] = driver.notify + LOG.info("osprofiler is enabled with connection string: %s", + connection_string) + except Exception: + LOG.exception("Could not initialize driver for connection string " + "%s, osprofiler is disabled", connection_string) + __notifier_cache[connection_string] = _noop_notifier + + return __notifier_cache[connection_string] + + +def clear_notifier_cache(): + __notifier_cache.clear() diff --git a/osprofiler/tests/unit/test_notifier.py b/osprofiler/tests/unit/test_notifier.py index 7cccdad..8e658fe 100644 --- a/osprofiler/tests/unit/test_notifier.py +++ b/osprofiler/tests/unit/test_notifier.py @@ -23,6 +23,7 @@ class NotifierTestCase(test.TestCase): def tearDown(self): notifier.set(notifier._noop_notifier) # restore defaults + notifier.clear_notifier_cache() super(NotifierTestCase, self).tearDown() def test_set(self): @@ -49,3 +50,11 @@ class NotifierTestCase(test.TestCase): result = notifier.create("test", 10, b=20) mock_factory.assert_called_once_with("test", 10, b=20) self.assertEqual(mock_factory.return_value.notify, result) + + @mock.patch("osprofiler.notifier.base.get_driver") + def test_create_driver_init_failure(self, mock_get_driver): + mock_get_driver.side_effect = Exception() + + result = notifier.create("test", 10, b=20) + mock_get_driver.assert_called_once_with("test", 10, b=20) + self.assertEqual(notifier._noop_notifier, result) -- cgit v1.2.1