From 050b67c9d6ae3915a1ea3cb25095da4b7252de7f Mon Sep 17 00:00:00 2001 From: Victor Shyba Date: Tue, 18 Feb 2020 18:57:52 -0300 Subject: [PATCH 1/4] apply share_usage_data as its set --- lbry/extras/cli.py | 7 +++---- lbry/extras/daemon/analytics.py | 17 +++++++++++------ lbry/extras/daemon/loggly_handler.py | 15 +++++++++++---- tests/integration/other/test_other_commands.py | 15 +++++++++++++++ tests/unit/test_cli.py | 18 +++++++++++++----- 5 files changed, 53 insertions(+), 19 deletions(-) diff --git a/lbry/extras/cli.py b/lbry/extras/cli.py index e0f6de36a..5752286f8 100644 --- a/lbry/extras/cli.py +++ b/lbry/extras/cli.py @@ -256,10 +256,9 @@ def setup_logging(logger: logging.Logger, args: argparse.Namespace, conf: Config else: logger.getChild('lbry').setLevel(logging.DEBUG) - if conf.share_usage_data: - loggly_handler = get_loggly_handler() - loggly_handler.setLevel(logging.ERROR) - logger.getChild('lbry').addHandler(loggly_handler) + loggly_handler = get_loggly_handler(lambda: conf.share_usage_data) + loggly_handler.setLevel(logging.ERROR) + logger.getChild('lbry').addHandler(loggly_handler) def run_daemon(args: argparse.Namespace, conf: Config): diff --git a/lbry/extras/daemon/analytics.py b/lbry/extras/daemon/analytics.py index ad83ed39b..daa107791 100644 --- a/lbry/extras/daemon/analytics.py +++ b/lbry/extras/daemon/analytics.py @@ -110,7 +110,6 @@ class AnalyticsManager: self.cookies = {} self.url = ANALYTICS_ENDPOINT self._write_key = utils.deobfuscate(ANALYTICS_TOKEN) - self._enabled = conf.share_usage_data self._tracked_data = collections.defaultdict(list) self.context = _make_context(system_info.get_platform()) self.installation_id = installation_id @@ -118,20 +117,26 @@ class AnalyticsManager: self.task: typing.Optional[asyncio.Task] = None self.external_ip: typing.Optional[str] = None + @property + def enabled(self): + return self.conf.share_usage_data + @property def is_started(self): return self.task is not None async def start(self): - if self._enabled and self.task is None: - self.external_ip = await utils.get_external_ip() + if self.task is None: + self.external_ip = await utils.get_external_ip() if self.enabled else None self.task = asyncio.create_task(self.run()) async def run(self): while True: - await self._send_heartbeat() + if self.enabled: + await self._send_heartbeat() await asyncio.sleep(1800) - self.external_ip = await utils.get_external_ip() + if self.enabled: + self.external_ip = await utils.get_external_ip() def stop(self): if self.task is not None and not self.task.done(): @@ -154,7 +159,7 @@ class AnalyticsManager: async def track(self, event: typing.Dict): """Send a single tracking event""" - if self._enabled: + if self.enabled: log.debug('Sending track event: %s', event) await self._post(event) diff --git a/lbry/extras/daemon/loggly_handler.py b/lbry/extras/daemon/loggly_handler.py index f69c245ef..3b709fb41 100644 --- a/lbry/extras/daemon/loggly_handler.py +++ b/lbry/extras/daemon/loggly_handler.py @@ -36,7 +36,7 @@ class JsonFormatter(logging.Formatter): class HTTPSLogglyHandler(logging.Handler): - def __init__(self, loggly_token: str, fqdn=False, localname=None, facility=None, cookies=None): + def __init__(self, loggly_token: str, fqdn=False, localname=None, facility=None, cookies=None, feature_toggle=None): super().__init__() self.fqdn = fqdn self.localname = localname @@ -47,6 +47,11 @@ class HTTPSLogglyHandler(logging.Handler): ) self._loop = asyncio.get_event_loop() self._session = aiohttp.ClientSession() + self._toggle = feature_toggle + + @property + def enabled(self): + return self._toggle is None or (self._toggle and self._toggle()) @staticmethod def get_full_message(record): @@ -62,12 +67,14 @@ class HTTPSLogglyHandler(logging.Handler): cookies=self.cookies) as response: self.cookies.update(response.cookies) except ClientError: - if self._loop.is_running() and retry: + if self._loop.is_running() and retry and self.enabled: await self._session.close() self._session = aiohttp.ClientSession() return await self._emit(record, retry=False) def emit(self, record): + if not self.enabled: + return try: asyncio.ensure_future(self._emit(record), loop=self._loop) except RuntimeError: # TODO: use a second loop @@ -83,7 +90,7 @@ class HTTPSLogglyHandler(logging.Handler): pass -def get_loggly_handler(): - handler = HTTPSLogglyHandler(LOGGLY_TOKEN) +def get_loggly_handler(feature_toggle): + handler = HTTPSLogglyHandler(LOGGLY_TOKEN, feature_toggle=feature_toggle) handler.setFormatter(JsonFormatter()) return handler diff --git a/tests/integration/other/test_other_commands.py b/tests/integration/other/test_other_commands.py index a28afa525..6cdc3cf07 100644 --- a/tests/integration/other/test_other_commands.py +++ b/tests/integration/other/test_other_commands.py @@ -1,3 +1,4 @@ +from lbry.extras.daemon.loggly_handler import get_loggly_handler from lbry.testcase import CommandTestCase @@ -11,6 +12,7 @@ class AddressManagement(CommandTestCase): self.assertItemCount(single, 1) self.assertEqual(single['items'][0], addresses['items'][11]) + class SettingsManagement(CommandTestCase): async def test_settings(self): @@ -23,3 +25,16 @@ class SettingsManagement(CommandTestCase): setting = self.daemon.jsonrpc_settings_clear('lbryum_servers') self.assertEqual(setting['lbryum_servers'][0], ('spv11.lbry.com', 50001)) self.assertEqual(self.daemon.jsonrpc_settings_get()['lbryum_servers'][0], ('spv11.lbry.com', 50001)) + + # test_privacy_settings (merged for reducing test time, unmerge when its fast) + # tests that changing share_usage_data propagates to the relevant properties + self.assertFalse(self.daemon.jsonrpc_settings_get()['share_usage_data']) + loggly = get_loggly_handler(lambda: self.daemon.conf.share_usage_data) + self.addCleanup(loggly.close) + self.assertFalse(self.daemon.analytics_manager.enabled) + self.assertFalse(loggly.enabled) + self.daemon.jsonrpc_settings_set('share_usage_data', True) + self.assertTrue(self.daemon.jsonrpc_settings_get()['share_usage_data']) + self.assertTrue(self.daemon.analytics_manager.enabled) + self.assertTrue(loggly.enabled) + self.daemon.jsonrpc_settings_set('share_usage_data', False) diff --git a/tests/unit/test_cli.py b/tests/unit/test_cli.py index 76616d298..64ad1a3f0 100644 --- a/tests/unit/test_cli.py +++ b/tests/unit/test_cli.py @@ -64,8 +64,9 @@ class CLILoggingTest(AsyncioTestCase): self.assertTrue(log.isEnabledFor(logging.INFO)) self.assertFalse(log.isEnabledFor(logging.DEBUG)) self.assertFalse(log.isEnabledFor(logging.DEBUG)) - self.assertEqual(len(log.handlers), 1) + self.assertEqual(len(log.handlers), 2) self.assertIsInstance(log.handlers[0], logging.handlers.RotatingFileHandler) + self.assertFalse(log.handlers[1].enabled) async with get_logger(["start", "--verbose"]) as log: self.assertTrue(log.getChild("lbry").isEnabledFor(logging.DEBUG)) @@ -81,23 +82,30 @@ class CLILoggingTest(AsyncioTestCase): async def test_loggly(self): async with get_logger(["start"]) as log: # default share_usage_data=False - self.assertEqual(len(log.getChild("lbry").handlers), 2) # file and console + log = log.getChild("lbry") + self.assertIsInstance(log.handlers[0], logging.StreamHandler) + self.assertIsInstance(log.handlers[1], logging.StreamHandler) + self.assertIsInstance(log.handlers[2], HTTPSLogglyHandler) + self.assertFalse(log.handlers[2].enabled) async with get_logger(["start"], share_usage_data=True) as log: log = log.getChild("lbry") self.assertEqual(len(log.handlers), 3) self.assertIsInstance(log.handlers[2], HTTPSLogglyHandler) + self.assertTrue(log.handlers[2].enabled) async with get_logger(["start"], share_usage_data=False) as log: # explicit share_usage_data=False log = log.getChild("lbry") - self.assertEqual(len(log.handlers), 2) + self.assertEqual(len(log.handlers), 3) + self.assertIsInstance(log.handlers[2], HTTPSLogglyHandler) + self.assertFalse(log.handlers[2].enabled) async def test_quiet(self): async with get_logger(["start"]) as log: # default is loud log = log.getChild("lbry") - self.assertEqual(len(log.handlers), 2) + self.assertEqual(len(log.handlers), 3) self.assertIs(type(log.handlers[1]), logging.StreamHandler) async with get_logger(["start", "--quiet"]) as log: log = log.getChild("lbry") - self.assertEqual(len(log.handlers), 1) + self.assertEqual(len(log.handlers), 2) self.assertIsNot(type(log.handlers[0]), logging.StreamHandler) From 5394f1763cae10fe1fbc548d01746b70da7dff21 Mon Sep 17 00:00:00 2001 From: Victor Shyba Date: Thu, 20 Feb 2020 12:12:30 -0300 Subject: [PATCH 2/4] simplify external ip logic --- lbry/extras/daemon/analytics.py | 4 +--- lbry/extras/daemon/loggly_handler.py | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/lbry/extras/daemon/analytics.py b/lbry/extras/daemon/analytics.py index daa107791..828112396 100644 --- a/lbry/extras/daemon/analytics.py +++ b/lbry/extras/daemon/analytics.py @@ -127,16 +127,14 @@ class AnalyticsManager: async def start(self): if self.task is None: - self.external_ip = await utils.get_external_ip() if self.enabled else None self.task = asyncio.create_task(self.run()) async def run(self): while True: if self.enabled: + self.external_ip = await utils.get_external_ip() await self._send_heartbeat() await asyncio.sleep(1800) - if self.enabled: - self.external_ip = await utils.get_external_ip() def stop(self): if self.task is not None and not self.task.done(): diff --git a/lbry/extras/daemon/loggly_handler.py b/lbry/extras/daemon/loggly_handler.py index 3b709fb41..6d78e2f37 100644 --- a/lbry/extras/daemon/loggly_handler.py +++ b/lbry/extras/daemon/loggly_handler.py @@ -51,7 +51,7 @@ class HTTPSLogglyHandler(logging.Handler): @property def enabled(self): - return self._toggle is None or (self._toggle and self._toggle()) + return self._toggle and self._toggle() @staticmethod def get_full_message(record): From fc5d5faaedaf97f05913277b1c582f45ed6018b9 Mon Sep 17 00:00:00 2001 From: Victor Shyba Date: Thu, 20 Feb 2020 14:27:39 -0300 Subject: [PATCH 3/4] use conf directly isntead of lambda --- lbry/extras/cli.py | 2 +- lbry/extras/daemon/loggly_handler.py | 14 ++++++++------ tests/integration/other/test_other_commands.py | 2 +- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/lbry/extras/cli.py b/lbry/extras/cli.py index 5752286f8..e5de48616 100644 --- a/lbry/extras/cli.py +++ b/lbry/extras/cli.py @@ -256,7 +256,7 @@ def setup_logging(logger: logging.Logger, args: argparse.Namespace, conf: Config else: logger.getChild('lbry').setLevel(logging.DEBUG) - loggly_handler = get_loggly_handler(lambda: conf.share_usage_data) + loggly_handler = get_loggly_handler(conf) loggly_handler.setLevel(logging.ERROR) logger.getChild('lbry').addHandler(loggly_handler) diff --git a/lbry/extras/daemon/loggly_handler.py b/lbry/extras/daemon/loggly_handler.py index 6d78e2f37..3feb7f766 100644 --- a/lbry/extras/daemon/loggly_handler.py +++ b/lbry/extras/daemon/loggly_handler.py @@ -3,10 +3,12 @@ import json import logging.handlers import traceback +import typing from aiohttp.client_exceptions import ClientError import aiohttp from lbry import utils, __version__ - +if typing.TYPE_CHECKING: + from lbry.conf import Config LOGGLY_TOKEN = 'BQEzZmMzLJHgAGxkBF00LGD0YGuyATVgAmqxAQEuAQZ2BQH4' @@ -36,7 +38,7 @@ class JsonFormatter(logging.Formatter): class HTTPSLogglyHandler(logging.Handler): - def __init__(self, loggly_token: str, fqdn=False, localname=None, facility=None, cookies=None, feature_toggle=None): + def __init__(self, loggly_token: str, fqdn=False, localname=None, facility=None, cookies=None, config=None): super().__init__() self.fqdn = fqdn self.localname = localname @@ -47,11 +49,11 @@ class HTTPSLogglyHandler(logging.Handler): ) self._loop = asyncio.get_event_loop() self._session = aiohttp.ClientSession() - self._toggle = feature_toggle + self._config: typing.Optional['Config'] = config @property def enabled(self): - return self._toggle and self._toggle() + return self._config and self._config.share_usage_data @staticmethod def get_full_message(record): @@ -90,7 +92,7 @@ class HTTPSLogglyHandler(logging.Handler): pass -def get_loggly_handler(feature_toggle): - handler = HTTPSLogglyHandler(LOGGLY_TOKEN, feature_toggle=feature_toggle) +def get_loggly_handler(config): + handler = HTTPSLogglyHandler(LOGGLY_TOKEN, config=config) handler.setFormatter(JsonFormatter()) return handler diff --git a/tests/integration/other/test_other_commands.py b/tests/integration/other/test_other_commands.py index 6cdc3cf07..4ba5b9f10 100644 --- a/tests/integration/other/test_other_commands.py +++ b/tests/integration/other/test_other_commands.py @@ -29,7 +29,7 @@ class SettingsManagement(CommandTestCase): # test_privacy_settings (merged for reducing test time, unmerge when its fast) # tests that changing share_usage_data propagates to the relevant properties self.assertFalse(self.daemon.jsonrpc_settings_get()['share_usage_data']) - loggly = get_loggly_handler(lambda: self.daemon.conf.share_usage_data) + loggly = get_loggly_handler(self.daemon.conf) self.addCleanup(loggly.close) self.assertFalse(self.daemon.analytics_manager.enabled) self.assertFalse(loggly.enabled) From 7c160ff65e4532da245cb917fb6e1a72cff43cfa Mon Sep 17 00:00:00 2001 From: Victor Shyba Date: Thu, 20 Feb 2020 14:35:57 -0300 Subject: [PATCH 4/4] remove unused parameters --- lbry/extras/daemon/loggly_handler.py | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/lbry/extras/daemon/loggly_handler.py b/lbry/extras/daemon/loggly_handler.py index 3feb7f766..d21c37a97 100644 --- a/lbry/extras/daemon/loggly_handler.py +++ b/lbry/extras/daemon/loggly_handler.py @@ -38,22 +38,19 @@ class JsonFormatter(logging.Formatter): class HTTPSLogglyHandler(logging.Handler): - def __init__(self, loggly_token: str, fqdn=False, localname=None, facility=None, cookies=None, config=None): + def __init__(self, loggly_token: str, config: 'Config'): super().__init__() - self.fqdn = fqdn - self.localname = localname - self.facility = facility - self.cookies = cookies or {} + self.cookies = {} self.url = "https://logs-01.loggly.com/inputs/{token}/tag/{tag}".format( token=utils.deobfuscate(loggly_token), tag='lbrynet-' + __version__ ) self._loop = asyncio.get_event_loop() self._session = aiohttp.ClientSession() - self._config: typing.Optional['Config'] = config + self._config = config @property def enabled(self): - return self._config and self._config.share_usage_data + return self._config.share_usage_data @staticmethod def get_full_message(record):