From ed3dae4fbaa85aa0e16bf59b73f2eafb8448f43e Mon Sep 17 00:00:00 2001 From: Bilal Al-Shahwany Date: Wed, 1 Nov 2023 15:22:53 -0700 Subject: [PATCH] polishing --- splitio/client/config.py | 1 - splitio/client/factory.py | 44 +++++++------------------- splitio/engine/telemetry.py | 4 +-- splitio/models/telemetry.py | 40 ++--------------------- splitio/storage/__init__.py | 1 - splitio/storage/inmemmory.py | 20 ++---------- splitio/storage/pluggable.py | 16 ++-------- splitio/storage/redis.py | 14 ++------ splitio/util/storage_helper.py | 18 +---------- tests/engine/test_telemetry.py | 4 +-- tests/models/test_telemetry_model.py | 12 ++----- tests/storage/test_inmemory_storage.py | 4 +-- tests/storage/test_pluggable.py | 10 +++--- tests/storage/test_redis.py | 4 +-- tests/sync/test_telemetry.py | 2 +- 15 files changed, 36 insertions(+), 158 deletions(-) diff --git a/splitio/client/config.py b/splitio/client/config.py index 437df62e..92388edf 100644 --- a/splitio/client/config.py +++ b/splitio/client/config.py @@ -1,7 +1,6 @@ """Default settings for the Split.IO SDK Python client.""" import os.path import logging -import re from splitio.engine.impressions import ImpressionsMode from splitio.client.input_validator import validate_flag_sets diff --git a/splitio/client/factory.py b/splitio/client/factory.py index 1a69a193..67c57e68 100644 --- a/splitio/client/factory.py +++ b/splitio/client/factory.py @@ -69,9 +69,6 @@ _INSTANTIATED_FACTORIES_LOCK = threading.RLock() _MIN_DEFAULT_DATA_SAMPLING_ALLOWED = 0.1 # 10% _MAX_RETRY_SYNC_ALL = 3 -_FLAG_SETS_LOCK = threading.RLock() -_TOTAL_FLAG_SETS = 0 -_INVALID_FLAG_SETS = 0 class Status(Enum): @@ -315,7 +312,8 @@ def _wrap_impression_listener(listener, metadata): def _build_in_memory_factory(api_key, cfg, sdk_url=None, events_url=None, # pylint:disable=too-many-arguments,too-many-locals - auth_api_base_url=None, streaming_api_base_url=None, telemetry_api_base_url=None): + auth_api_base_url=None, streaming_api_base_url=None, telemetry_api_base_url=None, + total_flag_sets=0, invalid_flag_sets=0): """Build and return a split factory tailored to the supplied config.""" if not input_validator.validate_factory_instantiation(api_key): return None @@ -419,10 +417,7 @@ def _build_in_memory_factory(api_key, cfg, sdk_url=None, events_url=None, # pyl telemetry_evaluation_producer ) - telemetry_init_producer.record_config(cfg, extra_cfg) - total_flag_sets, invalid_flag_sets = _get_total_and_invalid_flag_sets() - telemetry_init_producer.record_flag_sets(total_flag_sets) - telemetry_init_producer.record_invalid_flag_sets(invalid_flag_sets) + telemetry_init_producer.record_config(cfg, extra_cfg, total_flag_sets, invalid_flag_sets) if preforked_initialization: synchronizer.sync_all(max_retry_attempts=_MAX_RETRY_SYNC_ALL) @@ -501,7 +496,7 @@ def _build_redis_factory(api_key, cfg): initialization_thread = threading.Thread(target=manager.start, name="SDKInitializer", daemon=True) initialization_thread.start() - telemetry_init_producer.record_config(cfg, {}) + telemetry_init_producer.record_config(cfg, {}, 0, 0) split_factory = SplitFactory( api_key, @@ -514,10 +509,7 @@ def _build_redis_factory(api_key, cfg): telemetry_init_producer=telemetry_init_producer ) redundant_factory_count, active_factory_count = _get_active_and_redundant_count() - total_flag_sets, invalid_flag_sets = _get_total_and_invalid_flag_sets() storages['telemetry'].record_active_and_redundant_factories(active_factory_count, redundant_factory_count) - storages['telemetry'].record_flag_sets(total_flag_sets) - storages['telemetry'].record_invalid_flag_sets(invalid_flag_sets) telemetry_submitter.synchronize_config() return split_factory @@ -582,7 +574,7 @@ def _build_pluggable_factory(api_key, cfg): initialization_thread = threading.Thread(target=manager.start, name="SDKInitializer", daemon=True) initialization_thread.start() - telemetry_init_producer.record_config(cfg, {}) + telemetry_init_producer.record_config(cfg, {}, 0, 0) split_factory = SplitFactory( api_key, @@ -595,10 +587,7 @@ def _build_pluggable_factory(api_key, cfg): telemetry_init_producer=telemetry_init_producer ) redundant_factory_count, active_factory_count = _get_active_and_redundant_count() - total_flag_sets, invalid_flag_sets = _get_total_and_invalid_flag_sets() storages['telemetry'].record_active_and_redundant_factories(active_factory_count, redundant_factory_count) - storages['telemetry'].record_flag_sets(total_flag_sets) - storages['telemetry'].record_invalid_flag_sets(invalid_flag_sets) telemetry_submitter.synchronize_config() return split_factory @@ -697,13 +686,11 @@ def get_factory(api_key, **kwargs): _INSTANTIATED_FACTORIES_LOCK.release() config_raw = kwargs.get('config', {}) + total_flag_sets = 0 + invalid_flag_sets = 0 if config_raw.get('flagSetsFilter') is not None and isinstance(config_raw.get('flagSetsFilter'), list): - global _TOTAL_FLAG_SETS - global _INVALID_FLAG_SETS - _FLAG_SETS_LOCK.acquire() - _TOTAL_FLAG_SETS = len(config_raw.get('flagSetsFilter')) - _INVALID_FLAG_SETS = _TOTAL_FLAG_SETS - len(input_validator.validate_flag_sets(config_raw.get('flagSetsFilter'), 'Telemetry Init')) - _FLAG_SETS_LOCK.release() + total_flag_sets = len(config_raw.get('flagSetsFilter')) + invalid_flag_sets = total_flag_sets - len(input_validator.validate_flag_sets(config_raw.get('flagSetsFilter'), 'Telemetry Init')) config = sanitize_config(api_key, config_raw) @@ -721,7 +708,9 @@ def get_factory(api_key, **kwargs): kwargs.get('events_api_base_url'), kwargs.get('auth_api_base_url'), kwargs.get('streaming_api_base_url'), - kwargs.get('telemetry_api_base_url')) + kwargs.get('telemetry_api_base_url'), + total_flag_sets, + invalid_flag_sets) return split_factory @@ -734,12 +723,3 @@ def _get_active_and_redundant_count(): active_factory_count += _INSTANTIATED_FACTORIES[item] _INSTANTIATED_FACTORIES_LOCK.release() return redundant_factory_count, active_factory_count - -def _get_total_and_invalid_flag_sets(): - total_flag_sets = 0 - invalid_flag_sets = 0 - _FLAG_SETS_LOCK.acquire() - total_flag_sets = _TOTAL_FLAG_SETS - invalid_flag_sets = _INVALID_FLAG_SETS - _FLAG_SETS_LOCK.release() - return total_flag_sets, invalid_flag_sets diff --git a/splitio/engine/telemetry.py b/splitio/engine/telemetry.py index 7471bc47..55afa320 100644 --- a/splitio/engine/telemetry.py +++ b/splitio/engine/telemetry.py @@ -36,9 +36,9 @@ def __init__(self, telemetry_storage): """Constructor.""" self._telemetry_storage = telemetry_storage - def record_config(self, config, extra_config): + def record_config(self, config, extra_config, total_flag_sets=0, invalid_flag_sets=0): """Record configurations.""" - self._telemetry_storage.record_config(config, extra_config) + self._telemetry_storage.record_config(config, extra_config, total_flag_sets, invalid_flag_sets) current_app, app_worker_id = self._get_app_worker_id() if current_app is not None: self.add_config_tag("initilization:" + current_app) diff --git a/splitio/models/telemetry.py b/splitio/models/telemetry.py index ff43ace3..e1685b3d 100644 --- a/splitio/models/telemetry.py +++ b/splitio/models/telemetry.py @@ -796,7 +796,7 @@ def _reset_all(self): self._flag_sets = 0 self._flag_sets_invalid = 0 - def record_config(self, config, extra_config): + def record_config(self, config, extra_config, total_flag_sets, invalid_flag_sets): """ Record configurations. @@ -829,32 +829,14 @@ def record_config(self, config, extra_config): self._impressions_mode = self._get_impressions_mode(config[_ConfigParams.IMPRESSIONS_MODE.value]) self._impression_listener = True if config[_ConfigParams.IMPRESSIONS_LISTENER.value] is not None else False self._http_proxy = self._check_if_proxy_detected() + self._flag_sets = total_flag_sets + self._flag_sets_invalid = invalid_flag_sets def record_active_and_redundant_factories(self, active_factory_count, redundant_factory_count): with self._lock: self._active_factory_count = active_factory_count self._redundant_factory_count = redundant_factory_count - def record_flag_sets(self, flag_sets): - """ - Record flag sets - - :param flag_sets: flag sets count - :type flag_sets: int - """ - with self._lock: - self._flag_sets = flag_sets - - def record_invalid_flag_sets(self, flag_sets): - """ - Record invalid flag sets - - :param flag_sets: flag sets count - :type flag_sets: int - """ - with self._lock: - self._flag_sets_invalid = flag_sets - def record_ready_time(self, ready_time): """ Record ready time. @@ -881,22 +863,6 @@ def record_not_ready_usage(self): with self._lock: self._not_ready += 1 - def get_flag_sets(self): - """ - Get flag sets - - """ - with self._lock: - return self._flag_sets - - def get_invalid_flag_sets(self): - """ - Get invalid flag sets - - """ - with self._lock: - return self._flag_sets_invalid - def get_bur_time_outs(self): """ Get block until ready timeout. diff --git a/splitio/storage/__init__.py b/splitio/storage/__init__.py index bb8c2f81..76b63070 100644 --- a/splitio/storage/__init__.py +++ b/splitio/storage/__init__.py @@ -1,6 +1,5 @@ """Base storage interfaces.""" import abc -import threading class SplitStorage(object, metaclass=abc.ABCMeta): """Split storage interface implemented as an abstract class.""" diff --git a/splitio/storage/inmemmory.py b/splitio/storage/inmemmory.py index a31cddd4..6d74bdad 100644 --- a/splitio/storage/inmemmory.py +++ b/splitio/storage/inmemmory.py @@ -642,9 +642,9 @@ def _reset_config_tags(self): with self._lock: self._config_tags = [] - def record_config(self, config, extra_config): + def record_config(self, config, extra_config, total_flag_sets, invalid_flag_sets): """Record configurations.""" - self._tel_config.record_config(config, extra_config) + self._tel_config.record_config(config, extra_config, total_flag_sets, invalid_flag_sets) def record_active_and_redundant_factories(self, active_factory_count, redundant_factory_count): """Record active and redundant factories.""" @@ -654,14 +654,6 @@ def record_ready_time(self, ready_time): """Record ready time.""" self._tel_config.record_ready_time(ready_time) - def record_flag_sets(self, flag_sets): - """Record flag sets.""" - self._tel_config.record_flag_sets(flag_sets) - - def record_invalid_flag_sets(self, flag_sets): - """Record invalid flag sets.""" - self._tel_config.record_invalid_flag_sets(flag_sets) - def add_tag(self, tag): """Record tag string.""" with self._lock: @@ -730,14 +722,6 @@ def record_update_from_sse(self, event): """Record update from sse.""" self._counters.record_update_from_sse(event) - def get_flag_sets(self): - """Get flag sets.""" - self._tel_config.get_flag_sets() - - def get_invalid_flag_sets(self): - """Get invalid flag sets.""" - self._tel_config.get_invalid_flag_sets() - def get_bur_time_outs(self): """Get block until ready timeout.""" return self._tel_config.get_bur_time_outs() diff --git a/splitio/storage/pluggable.py b/splitio/storage/pluggable.py index 257b9e1c..d1503af3 100644 --- a/splitio/storage/pluggable.py +++ b/splitio/storage/pluggable.py @@ -795,7 +795,7 @@ def add_config_tag(self, tag): if len(self._config_tags) < MAX_TAGS: self._config_tags.append(tag) - def record_config(self, config, extra_config): + def record_config(self, config, extra_config, total_flag_sets, invalid_flag_sets): """ initilize telemetry objects @@ -804,15 +804,7 @@ def record_config(self, config, extra_config): :param extra_config: any extra configs :type extra_config: Dict """ - self._tel_config.record_config(config, extra_config) - - def record_flag_sets(self, flag_sets): - """Record flag sets.""" - self._tel_config.record_flag_sets(flag_sets) - - def record_invalid_flag_sets(self, flag_sets): - """Record invalid flag sets.""" - self._tel_config.record_invalid_flag_sets(flag_sets) + self._tel_config.record_config(config, extra_config, total_flag_sets, invalid_flag_sets) def pop_config_tags(self): """Get and reset configs.""" @@ -833,9 +825,7 @@ def _format_config_stats(self): 'rF': config_stats['rF'], 'sT': config_stats['sT'], 'oM': config_stats['oM'], - 't': self.pop_config_tags(), - 'fsT': self._tel_config.get_flag_sets(), - 'fsI': self._tel_config.get_invalid_flag_sets() + 't': self.pop_config_tags() }) def record_active_and_redundant_factories(self, active_factory_count, redundant_factory_count): diff --git a/splitio/storage/redis.py b/splitio/storage/redis.py index 92b3f16f..4e50f643 100644 --- a/splitio/storage/redis.py +++ b/splitio/storage/redis.py @@ -662,22 +662,14 @@ def add_config_tag(self, tag): if len(self._config_tags) < MAX_TAGS: self._config_tags.append(tag) - def record_config(self, config, extra_config): + def record_config(self, config, extra_config, total_flag_sets, invalid_flag_sets): """ initilize telemetry objects :param congif: factory configuration parameters :type config: splitio.client.config """ - self._tel_config.record_config(config, extra_config) - - def record_flag_sets(self, flag_sets): - """Record flag sets.""" - self._tel_config.record_flag_sets(flag_sets) - - def record_invalid_flag_sets(self, flag_sets): - """Record invalid flag sets.""" - self._tel_config.record_invalid_flag_sets(flag_sets) + self._tel_config.record_config(config, extra_config, total_flag_sets, invalid_flag_sets) def pop_config_tags(self): """Get and reset tags.""" @@ -700,8 +692,6 @@ def _format_config_stats(self): 'rF': config_stats['rF'], 'sT': config_stats['sT'], 'oM': config_stats['oM'], - 'fsT': self._tel_config.get_flag_sets(), - 'fsI': self._tel_config.get_invalid_flag_sets(), 't': self.pop_config_tags() }) diff --git a/splitio/util/storage_helper.py b/splitio/util/storage_helper.py index bd270bc0..d281c438 100644 --- a/splitio/util/storage_helper.py +++ b/splitio/util/storage_helper.py @@ -68,20 +68,4 @@ def combine_valid_flag_sets(result_sets): for result_set in result_sets: if isinstance(result_set, set) and len(result_set) > 0: to_return.update(result_set) - return to_return - -def combine_valid_flag_sets(result_sets): - """ - Check each flag set in given array of sets, combine all flag sets in one unique set - - :param result_sets: Flag sets set - :type flag_sets: list(set) - - :return: flag sets set - :rtype: set - """ - to_return = set() - for result_set in result_sets: - if isinstance(result_set, set) and len(result_set) > 0: - to_return.update(result_set) - return to_return + return to_return \ No newline at end of file diff --git a/tests/engine/test_telemetry.py b/tests/engine/test_telemetry.py index 5cc4b022..45b05551 100644 --- a/tests/engine/test_telemetry.py +++ b/tests/engine/test_telemetry.py @@ -35,10 +35,8 @@ def test_record_config(self, mocker): 'metricsRefreshRate': 10, 'storageType': None } - telemetry_init_producer.record_config(config, {}) + telemetry_init_producer.record_config(config, {}, 5, 2) telemetry_init_producer.record_active_and_redundant_factories(1, 0) - telemetry_init_producer.record_flag_sets(5) - telemetry_init_producer.record_invalid_flag_sets(2) assert(telemetry_storage._tel_config.get_stats() == {'oM': 0, 'sT': telemetry_storage._tel_config._get_storage_type(config['operationMode'], config['storageType']), diff --git a/tests/models/test_telemetry_model.py b/tests/models/test_telemetry_model.py index dd46ae80..5ff98d72 100644 --- a/tests/models/test_telemetry_model.py +++ b/tests/models/test_telemetry_model.py @@ -316,7 +316,7 @@ def test_telemetry_config(self): 'storageType': None, 'flagSetsFilter': None } - telemetry_config.record_config(config, {}) + telemetry_config.record_config(config, {}, 5, 2) assert(telemetry_config.get_stats() == {'oM': 0, 'sT': telemetry_config._get_storage_type(config['operationMode'], config['storageType']), 'sE': config['streamingEnabled'], @@ -332,16 +332,13 @@ def test_telemetry_config(self): 'bT': 0, 'aF': 0, 'rF': 0, - 'fsT': 0, - 'fsI': 0} + 'fsT': 5, + 'fsI': 2} ) telemetry_config.record_ready_time(10) assert(telemetry_config._time_until_ready == 10) - telemetry_config.record_flag_sets(5) - assert(telemetry_config._flag_sets == 5) - assert(telemetry_config.get_bur_time_outs() == 0) [telemetry_config.record_bur_time_out() for i in range(2)] assert(telemetry_config.get_bur_time_outs() == 2) @@ -350,9 +347,6 @@ def test_telemetry_config(self): [telemetry_config.record_not_ready_usage() for i in range(5)] assert(telemetry_config.get_non_ready_usage() == 5) - telemetry_config.record_invalid_flag_sets(2) - assert(telemetry_config._flag_sets_invalid == 2) - os.environ["https_proxy"] = "some_host_ip" assert(telemetry_config._check_if_proxy_detected() == True) diff --git a/tests/storage/test_inmemory_storage.py b/tests/storage/test_inmemory_storage.py index 9344dd3f..2c44bd2d 100644 --- a/tests/storage/test_inmemory_storage.py +++ b/tests/storage/test_inmemory_storage.py @@ -641,10 +641,8 @@ def test_record_config(self): 'metricsRefreshRate': 10, 'storageType': None } - storage.record_config(config, {}) + storage.record_config(config, {}, 2, 1) storage.record_active_and_redundant_factories(1, 0) - storage.record_flag_sets(2) - storage.record_invalid_flag_sets(1) assert(storage._tel_config.get_stats() == {'oM': 0, 'sT': storage._tel_config._get_storage_type(config['operationMode'], config['storageType']), 'sE': config['streamingEnabled'], diff --git a/tests/storage/test_pluggable.py b/tests/storage/test_pluggable.py index ace92762..b5772b56 100644 --- a/tests/storage/test_pluggable.py +++ b/tests/storage/test_pluggable.py @@ -673,12 +673,12 @@ def test_record_config(self): pluggable_telemetry_storage = PluggableTelemetryStorage(self.mock_adapter, self.sdk_metadata, prefix=sprefix) self.config = {} self.extra_config = {} - def record_config_mock(config, extra_config): + def record_config_mock(config, extra_config, fs, ifs): self.config = config self.extra_config = extra_config pluggable_telemetry_storage.record_config = record_config_mock - pluggable_telemetry_storage.record_config({'item': 'value'}, {'item2': 'value2'}) + pluggable_telemetry_storage.record_config({'item': 'value'}, {'item2': 'value2'}, 0, 0) assert(self.config == {'item': 'value'}) assert(self.extra_config == {'item2': 'value2'}) @@ -764,10 +764,8 @@ def test_push_config_stats(self): 'eventsPushRate': 60, 'metricsRefreshRate': 10, 'storageType': None - }, {} + }, {}, 0, 0 ) pluggable_telemetry_storage.record_active_and_redundant_factories(2, 1) - pluggable_telemetry_storage.record_flag_sets(3) - pluggable_telemetry_storage.record_invalid_flag_sets(1) pluggable_telemetry_storage.push_config_stats() - assert(self.mock_adapter._keys[pluggable_telemetry_storage._telemetry_config_key + "::" + pluggable_telemetry_storage._sdk_metadata] == '{"aF": 2, "rF": 1, "sT": "memory", "oM": 0, "t": [], "fsT": 3, "fsI": 1}') + assert(self.mock_adapter._keys[pluggable_telemetry_storage._telemetry_config_key + "::" + pluggable_telemetry_storage._sdk_metadata] == '{"aF": 2, "rF": 1, "sT": "memory", "oM": 0, "t": []}') diff --git a/tests/storage/test_redis.py b/tests/storage/test_redis.py index 8969f5d9..1c54a8aa 100644 --- a/tests/storage/test_redis.py +++ b/tests/storage/test_redis.py @@ -413,7 +413,7 @@ def test_init(self, mocker): @mock.patch('splitio.models.telemetry.TelemetryConfig.record_config') def test_record_config(self, mocker): redis_telemetry = RedisTelemetryStorage(mocker.Mock(), mocker.Mock()) - redis_telemetry.record_config(mocker.Mock(), mocker.Mock()) + redis_telemetry.record_config(mocker.Mock(), mocker.Mock(), 0, 0) assert(mocker.called) @mock.patch('splitio.storage.adapters.redis.RedisAdapter.hset') @@ -432,8 +432,6 @@ def test_format_config_stats(self, mocker): 'rF': stats['rF'], 'sT': stats['sT'], 'oM': stats['oM'], - 'fsT': redis_telemetry._tel_config.get_flag_sets(), - 'fsI': redis_telemetry._tel_config.get_invalid_flag_sets(), 't': redis_telemetry.pop_config_tags(), })) diff --git a/tests/sync/test_telemetry.py b/tests/sync/test_telemetry.py index 9d901713..9ce82cc7 100644 --- a/tests/sync/test_telemetry.py +++ b/tests/sync/test_telemetry.py @@ -111,7 +111,7 @@ def test_synchronize_telemetry(self, mocker): 'activeFactoryCount': 1, 'notReady': 0, 'timeUntilReady': 1 - }, {} + }, {}, 0, 0 ) self.formatted_config = "" def record_init(*args, **kwargs):