Merge pull request #469 from splitio/flagsets-telemetry-init

chillaq · web-flow · commit 9dc6f0e03c88 · 2023-09-22T09:31:52.000-07:00
Added flagsets total and invalid count for telemetry config
diff --git a/splitio/client/client.py b/splitio/client/client.py
@@ -404,9 +404,9 @@ def _get_treatments_by_flag_sets(self, key, flag_sets, method, attributes=None):
         :return: Dictionary with the result of all the feature flags provided
         :rtype: dict
         """
-        feature_flags_names = self._get_feature_flag_names_by_flag_sets(flag_sets, method)
+        feature_flags_names = self._get_feature_flag_names_by_flag_sets(flag_sets, method.value)
         if feature_flags_names == []:
-            _LOGGER.warning("%s: No valid Flag set or no feature flags found for evaluating treatments" % (method))
+            _LOGGER.warning("%s: No valid Flag set or no feature flags found for evaluating treatments" % (method.value))
             return {}
 
         if 'config' in method.value:
diff --git a/splitio/client/factory.py b/splitio/client/factory.py
@@ -69,6 +69,9 @@
 _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):
@@ -417,6 +420,9 @@ def _build_in_memory_factory(api_key, cfg, sdk_url=None, events_url=None,  # pyl
     )
 
     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)
 
     if preforked_initialization:
         synchronizer.sync_all(max_retry_attempts=_MAX_RETRY_SYNC_ALL)
@@ -508,7 +514,10 @@ 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
@@ -586,7 +595,10 @@ 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
@@ -684,7 +696,16 @@ def get_factory(api_key, **kwargs):
     _INSTANTIATED_FACTORIES.update([api_key])
     _INSTANTIATED_FACTORIES_LOCK.release()
 
-    config = sanitize_config(api_key, kwargs.get('config', {}))
+    config_raw = kwargs.get('config', {})
+    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()
+
+    config = sanitize_config(api_key, config_raw)
 
     if config['operationMode'] == 'localhost':
         split_factory =  _build_localhost_factory(config)
@@ -712,4 +733,13 @@ def _get_active_and_redundant_count():
         redundant_factory_count += _INSTANTIATED_FACTORIES[item] - 1
         active_factory_count += _INSTANTIATED_FACTORIES[item]
     _INSTANTIATED_FACTORIES_LOCK.release()
-    return redundant_factory_count, active_factory_count
+    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/client/input_validator.py b/splitio/client/input_validator.py
@@ -597,4 +597,4 @@ def validate_flag_sets(flag_sets, method_name):
 
         sanitized_flag_sets.add(flag_set)
 
-    return sorted(list(sanitized_flag_sets))
+    return list(sanitized_flag_sets)
diff --git a/splitio/engine/telemetry.py b/splitio/engine/telemetry.py
@@ -52,6 +52,10 @@ def record_flag_sets(self, flag_sets):
         """Record flag sets."""
         self._telemetry_storage.record_flag_sets(flag_sets)
 
+    def record_invalid_flag_sets(self, flag_sets):
+        """Record invalid flag sets."""
+        self._telemetry_storage.record_invalid_flag_sets(flag_sets)
+
     def record_bur_time_out(self):
         """Record block until ready timeout."""
         self._telemetry_storage.record_bur_time_out()
diff --git a/splitio/models/telemetry.py b/splitio/models/telemetry.py
@@ -794,6 +794,7 @@ def _reset_all(self):
             self._active_factory_count = 0
             self._redundant_factory_count = 0
             self._flag_sets = 0
+            self._flag_sets_invalid = 0
 
     def record_config(self, config, extra_config):
         """
@@ -844,6 +845,16 @@ def record_flag_sets(self, flag_sets):
         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.
@@ -878,6 +889,14 @@ def get_flag_sets(self):
         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.
@@ -930,7 +949,8 @@ def get_stats(self):
                 'hp': self._http_proxy,
                 'aF': self._active_factory_count,
                 'rF': self._redundant_factory_count,
-                'fS': self._flag_sets
+                'fsT': self._flag_sets,
+                'fsI': self._flag_sets_invalid
             }
 
     def _get_operation_mode(self, op_mode):
diff --git a/splitio/storage/__init__.py b/splitio/storage/__init__.py
@@ -2,7 +2,6 @@
 import abc
 import threading
 
-
 class SplitStorage(object, metaclass=abc.ABCMeta):
     """Split storage interface implemented as an abstract class."""
 
@@ -324,6 +323,7 @@ def __init__(self, flag_sets=[]):
         """Constructor."""
         self.flag_sets = set(flag_sets)
         self.should_filter = any(flag_sets)
+        self.sorted_flag_sets = sorted(flag_sets)
 
     def set_exist(self, flag_set):
         """
diff --git a/splitio/storage/inmemmory.py b/splitio/storage/inmemmory.py
@@ -658,6 +658,10 @@ 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,6 +734,10 @@ 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
@@ -806,6 +806,14 @@ def record_config(self, config, extra_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)
+
     def pop_config_tags(self):
         """Get and reset configs."""
         with self._lock:
@@ -825,7 +833,9 @@ def _format_config_stats(self):
             'rF': config_stats['rF'],
             'sT': config_stats['sT'],
             'oM': config_stats['oM'],
-            't': self.pop_config_tags()
+            't': self.pop_config_tags(),
+            'fsT': self._tel_config.get_flag_sets(),
+            'fsI': self._tel_config.get_invalid_flag_sets()
         })
 
     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
@@ -671,6 +671,14 @@ def record_config(self, config, extra_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)
+
     def pop_config_tags(self):
         """Get and reset tags."""
         with self._lock:
@@ -692,6 +700,8 @@ 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/sync/split.py b/splitio/sync/split.py
@@ -123,7 +123,7 @@ def _get_config_sets(self):
         """
         if self._feature_flag_storage.flag_set_filter.flag_sets == set({}):
             return None
-        return ','.join(self._feature_flag_storage.flag_set_filter.flag_sets)
+        return ','.join(self._feature_flag_storage.flag_set_filter.sorted_flag_sets)
 
     def synchronize_splits(self, till=None):
         """
diff --git a/splitio/util/storage_helper.py b/splitio/util/storage_helper.py
@@ -70,27 +70,6 @@ def combine_valid_flag_sets(result_sets):
             to_return.update(result_set)
     return to_return
 
-def _check_flag_sets(feature_flag_storage, feature_flag):
-    """
-    Check each flag set in given array, return it if exist in a given config flag set array, if config array is empty return all
-
-    :param flag_sets: Flag sets array
-    :type flag_sets: list(str)
-    :param config_flag_sets: Config flag sets array
-    :type config_flag_sets: list(str)
-
-    :return: array of flag sets
-    :rtype: list(str)
-    """
-    sets_to_fetch = []
-    for flag_set in flag_sets:
-        if not flag_set_filter.set_exist(flag_set) and flag_set_filter.should_filter:
-            _LOGGER.warning("Flag set %s is not part of the configured flag set list, ignoring the request." % (flag_set))
-            continue
-        sets_to_fetch.append(flag_set)
-
-    return sets_to_fetch
-
 def combine_valid_flag_sets(result_sets):
     """
     Check each flag set in given array of sets, combine all flag sets in one unique set
diff --git a/tests/client/test_factory.py b/tests/client/test_factory.py
@@ -29,6 +29,26 @@
 class SplitFactoryTests(object):
     """Split factory test cases."""
 
+    def test_flag_sets_counts(self):
+        factory = get_factory("none", config={
+            'flagSetsFilter': ['set1', 'set2', 'set3']
+        })
+
+        assert factory._telemetry_init_producer._telemetry_storage._tel_config._flag_sets == 3
+        assert factory._telemetry_init_producer._telemetry_storage._tel_config._flag_sets_invalid == 0
+
+        factory = get_factory("none", config={
+            'flagSetsFilter': ['s#et1', 'set2', 'set3']
+        })
+        assert factory._telemetry_init_producer._telemetry_storage._tel_config._flag_sets == 3
+        assert factory._telemetry_init_producer._telemetry_storage._tel_config._flag_sets_invalid == 1
+
+        factory = get_factory("none", config={
+            'flagSetsFilter': ['s#et1', 22, 'set3']
+        })
+        assert factory._telemetry_init_producer._telemetry_storage._tel_config._flag_sets == 3
+        assert factory._telemetry_init_producer._telemetry_storage._tel_config._flag_sets_invalid == 2
+
     def test_inmemory_client_creation_streaming_false(self, mocker):
         """Test that a client with in-memory storage is created correctly."""
 
diff --git a/tests/client/test_input_validator.py b/tests/client/test_input_validator.py
@@ -1296,19 +1296,19 @@ def test_validate_pluggable_adapter(self):
     def test_sanitize_flag_sets(self):
         """Test sanitization for flag sets."""
         flag_sets = input_validator.validate_flag_sets([' set1', 'set2 ', 'set3'], 'm')
-        assert flag_sets == ['set1', 'set2', 'set3']
+        assert sorted(flag_sets) == ['set1', 'set2', 'set3']
 
         flag_sets = input_validator.validate_flag_sets(['1set', '_set2'], 'm')
         assert flag_sets == ['1set']
 
         flag_sets = input_validator.validate_flag_sets(['Set1', 'SET2'], 'm')
-        assert flag_sets == ['set1', 'set2']
+        assert sorted(flag_sets) == ['set1', 'set2']
 
         flag_sets = input_validator.validate_flag_sets(['se\t1', 's/et2', 's*et3', 's!et4', 'se@t5', 'se#t5', 'se$t5', 'se^t5', 'se%t5', 'se&t5'], 'm')
         assert flag_sets == []
 
         flag_sets = input_validator.validate_flag_sets(['set4', 'set1', 'set3', 'set1'], 'm')
-        assert flag_sets == ['set1', 'set3', 'set4']
+        assert sorted(flag_sets) == ['set1', 'set3', 'set4']
 
         flag_sets = input_validator.validate_flag_sets(['w' * 50, 's' * 51], 'm')
         assert flag_sets == ['w' * 50]
diff --git a/tests/engine/test_telemetry.py b/tests/engine/test_telemetry.py
@@ -37,7 +37,8 @@ def test_record_config(self, mocker):
         }
         telemetry_init_producer.record_config(config, {})
         telemetry_init_producer.record_active_and_redundant_factories(1, 0)
-        telemetry_init_producer.record_flag_sets(2)
+        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']),
@@ -54,7 +55,8 @@ def test_record_config(self, mocker):
             'nR': 0,
             'aF': 1,
             'rF': 0,
-            'fS': 2}
+            'fsT': 5,
+            'fsI': 2}
             )
 
     def test_record_ready_time(self, mocker):
diff --git a/tests/models/test_telemetry_model.py b/tests/models/test_telemetry_model.py
@@ -316,7 +316,8 @@ def test_telemetry_config(self):
             'bT': 0,
             'aF': 0,
             'rF': 0,
-            'fS': 0}
+            'fsT': 0,
+            'fsI': 0}
             )
 
         telemetry_config.record_ready_time(10)
@@ -331,6 +332,9 @@ 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_flag_sets.py b/tests/storage/test_flag_sets.py
@@ -58,3 +58,6 @@ def test_flag_set_filter(self):
         assert not flag_set_filter.intersect(set({'set4'}))
         assert not flag_set_filter.set_exist('set4')
         assert flag_set_filter.set_exist('set1')
+
+        flag_set_filter = FlagSetsFilter(['set5', 'set2', 'set6', 'set1'])
+        assert flag_set_filter.sorted_flag_sets == ['set1', 'set2', 'set5', 'set6']
diff --git a/tests/storage/test_inmemory_storage.py b/tests/storage/test_inmemory_storage.py
diff --git a/tests/storage/test_pluggable.py b/tests/storage/test_pluggable.py
diff --git a/tests/storage/test_redis.py b/tests/storage/test_redis.py
diff --git a/tests/sync/test_splits_synchronizer.py b/tests/sync/test_splits_synchronizer.py
diff --git a/tests/sync/test_synchronizer.py b/tests/sync/test_synchronizer.py
diff --git a/tests/tasks/test_split_sync.py b/tests/tasks/test_split_sync.py

Original file line number	Diff line number	Diff line change
`@@ -597,4 +597,4 @@ def validate_flag_sets(flag_sets, method_name):`
`597`	`597`
`598`	`598`	`sanitized_flag_sets.add(flag_set)`
`599`	`599`
`600`		`- return sorted(list(sanitized_flag_sets))`
	`600`	`+ return list(sanitized_flag_sets)`