From c5c0b81c6303a84cae2318d1024109eeb2981ee3 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Mon, 19 Mar 2018 19:43:52 -0700 Subject: [PATCH 01/48] change EventSerializer to UserFilter since we'll be scrubbing events one at a time --- ldclient/event_consumer.py | 12 ++- ldclient/event_serializer.py | 48 ---------- ldclient/user_filter.py | 40 +++++++++ testing/test_event_serializer.py | 148 ------------------------------- testing/test_user_filter.py | 118 ++++++++++++++++++++++++ 5 files changed, 167 insertions(+), 199 deletions(-) delete mode 100644 ldclient/event_serializer.py create mode 100644 ldclient/user_filter.py delete mode 100644 testing/test_event_serializer.py create mode 100644 testing/test_user_filter.py diff --git a/ldclient/event_consumer.py b/ldclient/event_consumer.py index 4d8f671b..a1a6cd2e 100644 --- a/ldclient/event_consumer.py +++ b/ldclient/event_consumer.py @@ -6,7 +6,9 @@ import requests from requests.packages.urllib3.exceptions import ProtocolError -from ldclient.event_serializer import EventSerializer +import six + +from ldclient.user_filter import UserFilter from ldclient.interfaces import EventConsumer from ldclient.util import _headers from ldclient.util import log @@ -19,7 +21,7 @@ def __init__(self, event_queue, config): self.daemon = True self._config = config self._queue = event_queue - self._serializer = EventSerializer(config) + self._user_filter = UserFilter(config) self._running = True def run(self): @@ -42,7 +44,8 @@ def send_batch(self, events): def do_send(should_retry): # noinspection PyBroadException try: - json_body = self._serializer.serialize_events(events) + filtered_events = [ self._filter_event(e) for e in events ] + json_body = jsonpickle.encode(filtered_events, unpicklable=False) log.debug('Sending events payload: ' + json_body) hdrs = _headers(self._config.sdk_key) uri = self._config.events_uri @@ -77,6 +80,9 @@ def do_send(should_retry): for _ in events: self._queue.task_done() + def _filter_event(self, e): + return dict((key, self._user_filter.filter_user_props(value) if key == 'user' else value) for (key, value) in six.iteritems(e)) + def send(self): events = self.next() diff --git a/ldclient/event_serializer.py b/ldclient/event_serializer.py deleted file mode 100644 index c833e80b..00000000 --- a/ldclient/event_serializer.py +++ /dev/null @@ -1,48 +0,0 @@ -import jsonpickle -import six - - -class EventSerializer: - IGNORE_ATTRS = frozenset(['key', 'custom', 'anonymous']) - ALLOWED_TOP_LEVEL_ATTRS = frozenset(['key', 'secondary', 'ip', 'country', 'email', - 'firstName', 'lastName', 'avatar', 'name', 'anonymous', 'custom']) - - def __init__(self, config): - self._private_attribute_names = config.private_attribute_names - self._all_attributes_private = config.all_attributes_private - - def serialize_events(self, events): - body = [events] if isinstance(events, dict) else events - filtered = [ self._filter_event(e) for e in body ] - return jsonpickle.encode(filtered, unpicklable=False) - - def _is_private_attr(self, name, user_private_attrs): - if name in EventSerializer.IGNORE_ATTRS: - return False - elif self._all_attributes_private: - return True - else: - return (name in self._private_attribute_names) or (name in user_private_attrs) - - def _filter_event(self, e): - def filter_user_props(user_props): - all_private_attrs = set() - user_private_attrs = user_props.get('privateAttributeNames', []) - - def filter_private_attrs(attrs, allowed_attrs = frozenset()): - for key, value in six.iteritems(attrs): - if (not allowed_attrs) or (key in allowed_attrs): - if self._is_private_attr(key, user_private_attrs): - all_private_attrs.add(key) - else: - yield key, value - - ret = dict(filter_private_attrs(user_props, EventSerializer.ALLOWED_TOP_LEVEL_ATTRS)) - if 'custom' in user_props: - ret['custom'] = dict(filter_private_attrs(user_props['custom'])) - - if all_private_attrs: - ret['privateAttrs'] = sorted(list(all_private_attrs)) # note, only sorting to make tests reliable - return ret - - return dict((key, filter_user_props(value) if key == 'user' else value) for (key, value) in six.iteritems(e)) diff --git a/ldclient/user_filter.py b/ldclient/user_filter.py new file mode 100644 index 00000000..d48ab23f --- /dev/null +++ b/ldclient/user_filter.py @@ -0,0 +1,40 @@ +import jsonpickle +import six + + +class UserFilter: + IGNORE_ATTRS = frozenset(['key', 'custom', 'anonymous']) + ALLOWED_TOP_LEVEL_ATTRS = frozenset(['key', 'secondary', 'ip', 'country', 'email', + 'firstName', 'lastName', 'avatar', 'name', 'anonymous', 'custom']) + + def __init__(self, config): + self._private_attribute_names = config.private_attribute_names + self._all_attributes_private = config.all_attributes_private + + def _is_private_attr(self, name, user_private_attrs): + if name in UserFilter.IGNORE_ATTRS: + return False + elif self._all_attributes_private: + return True + else: + return (name in self._private_attribute_names) or (name in user_private_attrs) + + def filter_user_props(self, user_props): + all_private_attrs = set() + user_private_attrs = user_props.get('privateAttributeNames', []) + + def filter_private_attrs(attrs, allowed_attrs = frozenset()): + for key, value in six.iteritems(attrs): + if (not allowed_attrs) or (key in allowed_attrs): + if self._is_private_attr(key, user_private_attrs): + all_private_attrs.add(key) + else: + yield key, value + + ret = dict(filter_private_attrs(user_props, UserFilter.ALLOWED_TOP_LEVEL_ATTRS)) + if 'custom' in user_props: + ret['custom'] = dict(filter_private_attrs(user_props['custom'])) + + if all_private_attrs: + ret['privateAttrs'] = sorted(list(all_private_attrs)) # note, only sorting to make tests reliable + return ret diff --git a/testing/test_event_serializer.py b/testing/test_event_serializer.py deleted file mode 100644 index fd84ecac..00000000 --- a/testing/test_event_serializer.py +++ /dev/null @@ -1,148 +0,0 @@ -from builtins import object -import json -from ldclient.client import Config -from ldclient.event_serializer import EventSerializer - - -base_config = Config() -config_with_all_attrs_private = Config(all_attributes_private = True) -config_with_some_attrs_private = Config(private_attribute_names=[u'firstName', u'bizzle']) - -# users to serialize - -user = { - u'key': u'abc', - u'firstName': u'Sue', - u'custom': { - u'bizzle': u'def', - u'dizzle': u'ghi' - } -} - -user_specifying_own_private_attr = { - u'key': u'abc', - u'firstName': u'Sue', - u'custom': { - u'bizzle': u'def', - u'dizzle': u'ghi' - }, - u'privateAttributeNames': [ u'dizzle', u'unused' ] -} - -user_with_unknown_top_level_attrs = { - u'key': u'abc', - u'firstName': u'Sue', - u'species': u'human', - u'hatSize': 6, - u'custom': { - u'bizzle': u'def', - u'dizzle': u'ghi' - } -} - -anon_user = { - u'key': u'abc', - u'anonymous': True, - u'custom': { - u'bizzle': u'def', - u'dizzle': u'ghi' - } -} - -# expected results from serializing user - -user_with_all_attrs_hidden = { - u'key': u'abc', - u'custom': { }, - u'privateAttrs': [ u'bizzle', u'dizzle', u'firstName' ] -} - -user_with_some_attrs_hidden = { - u'key': u'abc', - u'custom': { - u'dizzle': u'ghi' - }, - u'privateAttrs': [ u'bizzle', u'firstName' ] -} - -user_with_own_specified_attr_hidden = { - u'key': u'abc', - u'firstName': u'Sue', - u'custom': { - u'bizzle': u'def' - }, - u'privateAttrs': [ u'dizzle' ] -} - -anon_user_with_all_attrs_hidden = { - u'key': u'abc', - u'anonymous': True, - u'custom': { }, - u'privateAttrs': [ u'bizzle', u'dizzle' ] -} - -def make_event(u, key = u'xyz'): - return { - u'creationDate': 1000000, - u'key': key, - u'kind': u'thing', - u'user': u - } - - -def test_all_user_attrs_serialized(): - es = EventSerializer(base_config) - event = make_event(user) - j = es.serialize_events(event) - assert json.loads(j) == [event] - -def test_all_user_attrs_private(): - es = EventSerializer(config_with_all_attrs_private) - event = make_event(user) - filtered_event = make_event(user_with_all_attrs_hidden) - j = es.serialize_events(event) - assert json.loads(j) == [filtered_event] - -def test_some_user_attrs_private(): - es = EventSerializer(config_with_some_attrs_private) - event = make_event(user) - filtered_event = make_event(user_with_some_attrs_hidden) - j = es.serialize_events(event) - assert json.loads(j) == [filtered_event] - -def test_per_user_private_attr(): - es = EventSerializer(base_config) - event = make_event(user_specifying_own_private_attr) - filtered_event = make_event(user_with_own_specified_attr_hidden) - j = es.serialize_events(event) - assert json.loads(j) == [filtered_event] - -def test_per_user_private_attr_plus_global_private_attrs(): - es = EventSerializer(config_with_some_attrs_private) - event = make_event(user_specifying_own_private_attr) - filtered_event = make_event(user_with_all_attrs_hidden) - j = es.serialize_events(event) - assert json.loads(j) == [filtered_event] - -def test_all_events_serialized(): - es = EventSerializer(config_with_all_attrs_private) - event0 = make_event(user, 'key0') - event1 = make_event(user, 'key1') - filtered0 = make_event(user_with_all_attrs_hidden, 'key0') - filtered1 = make_event(user_with_all_attrs_hidden, 'key1') - j = es.serialize_events([event0, event1]) - assert json.loads(j) == [filtered0, filtered1] - -def test_unknown_top_level_attrs_stripped(): - es = EventSerializer(base_config) - event = make_event(user_with_unknown_top_level_attrs) - filtered_event = make_event(user) - j = es.serialize_events(event) - assert json.loads(j) == [filtered_event] - -def test_leave_anonymous_attr_as_is(): - es = EventSerializer(config_with_all_attrs_private) - event = make_event(anon_user) - filtered_event = make_event(anon_user_with_all_attrs_hidden) - j = es.serialize_events(event) - assert json.loads(j) == [filtered_event] diff --git a/testing/test_user_filter.py b/testing/test_user_filter.py new file mode 100644 index 00000000..15550541 --- /dev/null +++ b/testing/test_user_filter.py @@ -0,0 +1,118 @@ +from builtins import object +import json +from ldclient.client import Config +from ldclient.user_filter import UserFilter + + +base_config = Config() +config_with_all_attrs_private = Config(all_attributes_private = True) +config_with_some_attrs_private = Config(private_attribute_names=[u'firstName', u'bizzle']) + +# users to serialize + +user = { + u'key': u'abc', + u'firstName': u'Sue', + u'custom': { + u'bizzle': u'def', + u'dizzle': u'ghi' + } +} + +user_specifying_own_private_attr = { + u'key': u'abc', + u'firstName': u'Sue', + u'custom': { + u'bizzle': u'def', + u'dizzle': u'ghi' + }, + u'privateAttributeNames': [ u'dizzle', u'unused' ] +} + +user_with_unknown_top_level_attrs = { + u'key': u'abc', + u'firstName': u'Sue', + u'species': u'human', + u'hatSize': 6, + u'custom': { + u'bizzle': u'def', + u'dizzle': u'ghi' + } +} + +anon_user = { + u'key': u'abc', + u'anonymous': True, + u'custom': { + u'bizzle': u'def', + u'dizzle': u'ghi' + } +} + +# expected results from serializing user + +user_with_all_attrs_hidden = { + u'key': u'abc', + u'custom': { }, + u'privateAttrs': [ u'bizzle', u'dizzle', u'firstName' ] +} + +user_with_some_attrs_hidden = { + u'key': u'abc', + u'custom': { + u'dizzle': u'ghi' + }, + u'privateAttrs': [ u'bizzle', u'firstName' ] +} + +user_with_own_specified_attr_hidden = { + u'key': u'abc', + u'firstName': u'Sue', + u'custom': { + u'bizzle': u'def' + }, + u'privateAttrs': [ u'dizzle' ] +} + +anon_user_with_all_attrs_hidden = { + u'key': u'abc', + u'anonymous': True, + u'custom': { }, + u'privateAttrs': [ u'bizzle', u'dizzle' ] +} + + +def test_all_user_attrs_serialized(): + uf = UserFilter(base_config) + j = uf.filter_user_props(user) + assert j == user + +def test_all_user_attrs_private(): + uf = UserFilter(config_with_all_attrs_private) + j = uf.filter_user_props(user) + assert j == user_with_all_attrs_hidden + +def test_some_user_attrs_private(): + uf = UserFilter(config_with_some_attrs_private) + j = uf.filter_user_props(user) + assert j == user_with_some_attrs_hidden + +def test_per_user_private_attr(): + uf = UserFilter(base_config) + j = uf.filter_user_props(user_specifying_own_private_attr) + assert j == user_with_own_specified_attr_hidden + +def test_per_user_private_attr_plus_global_private_attrs(): + uf = UserFilter(config_with_some_attrs_private) + j = uf.filter_user_props(user_specifying_own_private_attr) + assert j == user_with_all_attrs_hidden + +def test_unknown_top_level_attrs_stripped(): + uf = UserFilter(base_config) + j = uf.filter_user_props(user_with_unknown_top_level_attrs) + assert j == user + +def test_leave_anonymous_attr_as_is(): + uf = UserFilter(config_with_all_attrs_private) + j = uf.filter_user_props(anon_user) + assert j == anon_user_with_all_attrs_hidden From b9d73a1be8a4cce6725b8727f0903bf8e6a8ab2d Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Mon, 19 Mar 2018 19:44:42 -0700 Subject: [PATCH 02/48] preserve flag eval variation index for events --- ldclient/client.py | 5 +++-- ldclient/flag.py | 20 +++++++++++--------- testing/test_flag.py | 34 +++++++++++++++++----------------- 3 files changed, 31 insertions(+), 28 deletions(-) diff --git a/ldclient/client.py b/ldclient/client.py index 14a87e04..2ce5bad0 100644 --- a/ldclient/client.py +++ b/ldclient/client.py @@ -191,14 +191,15 @@ def _evaluate(self, flag, user): return evaluate(flag, user, self._store) def _evaluate_and_send_events(self, flag, user, default): - value, events = self._evaluate(flag, user) + variation, value, events = evaluate(flag, user, self._store) for event in events or []: self._send_event(event) if value is None: value = default self._send_event({'kind': 'feature', 'key': flag.get('key'), - 'user': user, 'value': value, 'default': default, 'version': flag.get('version')}) + 'user': user, 'variation': variation, 'value': value, + 'default': default, 'version': flag.get('version')}) return value def all_flags(self, user): diff --git a/ldclient/flag.py b/ldclient/flag.py index 06787de9..9775c5ae 100644 --- a/ldclient/flag.py +++ b/ldclient/flag.py @@ -18,16 +18,19 @@ def evaluate(flag, user, store): prereq_events = [] if flag.get('on', False): - value, prereq_events = _evaluate(flag, user, store) + variation, value, prereq_events = _evaluate(flag, user, store) if value is not None: - return value, prereq_events + return variation, value, prereq_events - return _get_off_variation(flag), prereq_events + off_var = flag.get('offVariation') + off_value = None if off_var is None else _get_variation(flag, off_var) + return off_var, off_value, prereq_events def _evaluate(flag, user, store, prereq_events=None): events = prereq_events or [] failed_prereq = None + prereq_var = None prereq_value = None for prereq in flag.get('prerequisites') or []: prereq_flag = store.get(FEATURES, prereq.get('key'), lambda x: x) @@ -36,22 +39,21 @@ def _evaluate(flag, user, store, prereq_events=None): failed_prereq = prereq break if prereq_flag.get('on', False) is True: - prereq_value, events = _evaluate(prereq_flag, user, store, events) - variation = _get_variation(prereq_flag, prereq.get('variation')) - if prereq_value is None or not prereq_value == variation: + prereq_var, prereq_value, events = _evaluate(prereq_flag, user, store, events) + if prereq_var is None or not prereq_var == prereq.get('variation'): failed_prereq = prereq else: failed_prereq = prereq - event = {'kind': 'feature', 'key': prereq.get('key'), 'user': user, + event = {'kind': 'feature', 'key': prereq.get('key'), 'user': user, 'variation': prereq_var, 'value': prereq_value, 'version': prereq_flag.get('version'), 'prereqOf': flag.get('key')} events.append(event) if failed_prereq is not None: - return None, events + return None, None, events index = _evaluate_index(flag, user, store) - return _get_variation(flag, index), events + return index, _get_variation(flag, index), events def _evaluate_index(feature, user, store): diff --git a/testing/test_flag.py b/testing/test_flag.py index 8b9740aa..63a42637 100644 --- a/testing/test_flag.py +++ b/testing/test_flag.py @@ -16,7 +16,7 @@ def test_flag_returns_off_variation_if_flag_is_off(): 'variations': ['a', 'b', 'c'] } user = { 'key': 'x' } - assert evaluate(flag, user, empty_store) == ('b', []) + assert evaluate(flag, user, empty_store) == (1, 'b', []) def test_flag_returns_none_if_flag_is_off_and_off_variation_is_unspecified(): flag = { @@ -26,7 +26,7 @@ def test_flag_returns_none_if_flag_is_off_and_off_variation_is_unspecified(): 'variations': ['a', 'b', 'c'] } user = { 'key': 'x' } - assert evaluate(flag, user, empty_store) == (None, []) + assert evaluate(flag, user, empty_store) == (None, None, []) def test_flag_returns_off_variation_if_prerequisite_not_found(): flag = { @@ -38,7 +38,7 @@ def test_flag_returns_off_variation_if_prerequisite_not_found(): 'variations': ['a', 'b', 'c'] } user = { 'key': 'x' } - assert evaluate(flag, user, empty_store) == ('b', []) + assert evaluate(flag, user, empty_store) == (1, 'b', []) def test_flag_returns_off_variation_and_event_if_prerequisite_is_not_met(): store = InMemoryFeatureStore() @@ -60,9 +60,9 @@ def test_flag_returns_off_variation_and_event_if_prerequisite_is_not_met(): } store.upsert(FEATURES, flag1) user = { 'key': 'x' } - events_should_be = [{'kind': 'feature', 'key': 'feature1', 'value': 'd', 'version': 2, - 'user': user, 'prereqOf': 'feature0'}] - assert evaluate(flag, user, store) == ('b', events_should_be) + events_should_be = [{'kind': 'feature', 'key': 'feature1', 'variation': 0, 'value': 'd', + 'version': 2, 'user': user, 'prereqOf': 'feature0'}] + assert evaluate(flag, user, store) == (1, 'b', events_should_be) def test_flag_returns_fallthrough_and_event_if_prereq_is_met_and_there_are_no_rules(): store = InMemoryFeatureStore() @@ -84,9 +84,9 @@ def test_flag_returns_fallthrough_and_event_if_prereq_is_met_and_there_are_no_ru } store.upsert(FEATURES, flag1) user = { 'key': 'x' } - events_should_be = [{'kind': 'feature', 'key': 'feature1', 'value': 'e', 'version': 2, - 'user': user, 'prereqOf': 'feature0'}] - assert evaluate(flag, user, store) == ('a', events_should_be) + events_should_be = [{'kind': 'feature', 'key': 'feature1', 'variation': 1, 'value': 'e', + 'version': 2, 'user': user, 'prereqOf': 'feature0'}] + assert evaluate(flag, user, store) == (0, 'a', events_should_be) def test_flag_matches_user_from_targets(): flag = { @@ -98,7 +98,7 @@ def test_flag_matches_user_from_targets(): 'variations': ['a', 'b', 'c'] } user = { 'key': 'userkey' } - assert evaluate(flag, user, empty_store) == ('c', []) + assert evaluate(flag, user, empty_store) == (2, 'c', []) def test_flag_matches_user_from_rules(): flag = { @@ -121,7 +121,7 @@ def test_flag_matches_user_from_rules(): 'variations': ['a', 'b', 'c'] } user = { 'key': 'userkey' } - assert evaluate(flag, user, empty_store) == ('c', []) + assert evaluate(flag, user, empty_store) == (2, 'c', []) def test_segment_match_clause_retrieves_segment_from_store(): store = InMemoryFeatureStore() @@ -152,7 +152,7 @@ def test_segment_match_clause_retrieves_segment_from_store(): ] } - assert evaluate(flag, user, store) == (True, []) + assert evaluate(flag, user, store) == (1, True, []) def test_segment_match_clause_falls_through_with_no_errors_if_segment_not_found(): user = { "key": "foo" } @@ -175,7 +175,7 @@ def test_segment_match_clause_falls_through_with_no_errors_if_segment_not_found( ] } - assert evaluate(flag, user, empty_store) == (False, []) + assert evaluate(flag, user, empty_store) == (0, False, []) def test_clause_matches_builtin_attribute(): clause = { @@ -185,7 +185,7 @@ def test_clause_matches_builtin_attribute(): } user = { 'key': 'x', 'name': 'Bob' } flag = _make_bool_flag_from_clause(clause) - assert evaluate(flag, user, empty_store) == (True, []) + assert evaluate(flag, user, empty_store) == (1, True, []) def test_clause_matches_custom_attribute(): clause = { @@ -195,7 +195,7 @@ def test_clause_matches_custom_attribute(): } user = { 'key': 'x', 'name': 'Bob', 'custom': { 'legs': 4 } } flag = _make_bool_flag_from_clause(clause) - assert evaluate(flag, user, empty_store) == (True, []) + assert evaluate(flag, user, empty_store) == (1, True, []) def test_clause_returns_false_for_missing_attribute(): clause = { @@ -205,7 +205,7 @@ def test_clause_returns_false_for_missing_attribute(): } user = { 'key': 'x', 'name': 'Bob' } flag = _make_bool_flag_from_clause(clause) - assert evaluate(flag, user, empty_store) == (False, []) + assert evaluate(flag, user, empty_store) == (0, False, []) def test_clause_can_be_negated(): clause = { @@ -216,7 +216,7 @@ def test_clause_can_be_negated(): } user = { 'key': 'x', 'name': 'Bob' } flag = _make_bool_flag_from_clause(clause) - assert evaluate(flag, user, empty_store) == (False, []) + assert evaluate(flag, user, empty_store) == (0, False, []) def _make_bool_flag_from_clause(clause): From 3543019dccd85a035be832049623b355f7d2c862 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Mon, 19 Mar 2018 19:48:28 -0700 Subject: [PATCH 03/48] set variation to None when value is default --- ldclient/client.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ldclient/client.py b/ldclient/client.py index 2ce5bad0..17b0d654 100644 --- a/ldclient/client.py +++ b/ldclient/client.py @@ -151,8 +151,8 @@ def variation(self, key, user, default): return default def send_event(value, version=None): - self._send_event({'kind': 'feature', 'key': key, - 'user': user, 'value': value, 'default': default, 'version': version}) + self._send_event({'kind': 'feature', 'key': key, 'user': user, 'variation': None, + 'value': value, 'default': default, 'version': version}) if not self.is_initialized(): if self._store.initialized: From 3c8e35307a9985aa2db0d0f320c0f385a6f8f9bc Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Mon, 19 Mar 2018 19:54:50 -0700 Subject: [PATCH 04/48] copy new flag properties into event --- ldclient/client.py | 7 +++++-- ldclient/flag.py | 4 +++- testing/test_flag.py | 10 ++++++---- 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/ldclient/client.py b/ldclient/client.py index 17b0d654..8309652d 100644 --- a/ldclient/client.py +++ b/ldclient/client.py @@ -152,7 +152,8 @@ def variation(self, key, user, default): def send_event(value, version=None): self._send_event({'kind': 'feature', 'key': key, 'user': user, 'variation': None, - 'value': value, 'default': default, 'version': version}) + 'value': value, 'default': default, 'version': version, + 'trackEvents': False, 'debugEventsUntilDate': None}) if not self.is_initialized(): if self._store.initialized: @@ -199,7 +200,9 @@ def _evaluate_and_send_events(self, flag, user, default): value = default self._send_event({'kind': 'feature', 'key': flag.get('key'), 'user': user, 'variation': variation, 'value': value, - 'default': default, 'version': flag.get('version')}) + 'default': default, 'version': flag.get('version'), + 'trackEvents': flag.get('trackEvents'), + 'debugEventsUntilDate': flag.get('debugEventsUntilDate')}) return value def all_flags(self, user): diff --git a/ldclient/flag.py b/ldclient/flag.py index 9775c5ae..404154d7 100644 --- a/ldclient/flag.py +++ b/ldclient/flag.py @@ -46,7 +46,9 @@ def _evaluate(flag, user, store, prereq_events=None): failed_prereq = prereq event = {'kind': 'feature', 'key': prereq.get('key'), 'user': user, 'variation': prereq_var, - 'value': prereq_value, 'version': prereq_flag.get('version'), 'prereqOf': flag.get('key')} + 'value': prereq_value, 'version': prereq_flag.get('version'), 'prereqOf': flag.get('key'), + 'trackEvents': prereq_flag.get('trackEvents'), + 'debugEventsUntilDate': prereq_flag.get('debugEventsUntilDate')} events.append(event) if failed_prereq is not None: diff --git a/testing/test_flag.py b/testing/test_flag.py index 63a42637..ef3ef69e 100644 --- a/testing/test_flag.py +++ b/testing/test_flag.py @@ -56,12 +56,13 @@ def test_flag_returns_off_variation_and_event_if_prerequisite_is_not_met(): 'on': True, 'fallthrough': { 'variation': 0 }, 'variations': ['d', 'e'], - 'version': 2 + 'version': 2, + 'trackEvents': False } store.upsert(FEATURES, flag1) user = { 'key': 'x' } events_should_be = [{'kind': 'feature', 'key': 'feature1', 'variation': 0, 'value': 'd', - 'version': 2, 'user': user, 'prereqOf': 'feature0'}] + 'version': 2, 'user': user, 'prereqOf': 'feature0', 'trackEvents': False, 'debugEventsUntilDate': None}] assert evaluate(flag, user, store) == (1, 'b', events_should_be) def test_flag_returns_fallthrough_and_event_if_prereq_is_met_and_there_are_no_rules(): @@ -80,12 +81,13 @@ def test_flag_returns_fallthrough_and_event_if_prereq_is_met_and_there_are_no_ru 'on': True, 'fallthrough': { 'variation': 1 }, 'variations': ['d', 'e'], - 'version': 2 + 'version': 2, + 'trackEvents': False } store.upsert(FEATURES, flag1) user = { 'key': 'x' } events_should_be = [{'kind': 'feature', 'key': 'feature1', 'variation': 1, 'value': 'e', - 'version': 2, 'user': user, 'prereqOf': 'feature0'}] + 'version': 2, 'user': user, 'prereqOf': 'feature0', 'trackEvents': False, 'debugEventsUntilDate': None}] assert evaluate(flag, user, store) == (0, 'a', events_should_be) def test_flag_matches_user_from_targets(): From ae65f95df314fb16311a2a68c8bddd772a08ca07 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Mon, 19 Mar 2018 20:23:16 -0700 Subject: [PATCH 05/48] add new config properties --- ldclient/config.py | 32 ++++++++++++++++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-) diff --git a/ldclient/config.py b/ldclient/config.py index 8abd96a8..28285bc0 100644 --- a/ldclient/config.py +++ b/ldclient/config.py @@ -29,7 +29,10 @@ def __init__(self, event_consumer_class=None, private_attribute_names=(), all_attributes_private=False, - offline=False): + offline=False, + user_keys_capacity=1000, + user_keys_flush_interval=300, + inline_users_in_events=False): """ :param string sdk_key: The SDK key for your LaunchDarkly account. :param string base_uri: The base URL for the LaunchDarkly server. Most users should use the default @@ -66,6 +69,13 @@ def __init__(self, private, not just the attributes specified in `private_attribute_names`. :param feature_store: A FeatureStore implementation :type feature_store: FeatureStore + :param int user_keys_capacity: The number of user keys that the event processor can remember at any + one time, so that duplicate user details will not be sent in analytics events. + :param float user_keys_flush_interval: The interval in seconds at which the event processor will + reset its set of known user keys. + :param bool inline_users_in_events: Whether to include full user details in every analytics event. + By default, events will only include the user key, except for one "index" event that provides the + full details for the user. :param feature_requester_class: A factory for a FeatureRequester implementation taking the sdk key and config :type feature_requester_class: (str, Config, FeatureStore) -> FeatureRequester :param event_consumer_class: A factory for an EventConsumer implementation taking the event queue, sdk key, and config @@ -100,6 +110,9 @@ def __init__(self, self.__private_attribute_names = private_attribute_names self.__all_attributes_private = all_attributes_private self.__offline = offline + self.__user_keys_capacity = user_keys_capacity + self.__user_keys_flush_interval = user_keys_flush_interval + self.__inline_users_in_events = inline_users_in_events @classmethod def default(cls): @@ -126,7 +139,10 @@ def copy_with_new_sdk_key(self, new_sdk_key): event_consumer_class=self.__event_consumer_class, private_attribute_names=self.__private_attribute_names, all_attributes_private=self.__all_attributes_private, - offline=self.__offline) + offline=self.__offline, + user_keys_capacity=self.__user_keys_capacity, + user_keys_flush_interval=self.__user_keys_flush_interval, + inline_users_in_events=self.__inline_users_in_events) def get_default(self, key, default): return default if key not in self.__defaults else self.__defaults[key] @@ -223,6 +239,18 @@ def all_attributes_private(self): def offline(self): return self.__offline + @property + def user_keys_capacity(self): + return self.__user_keys_capacity + + @property + def user_keys_flush_interval(self): + return self.__user_keys_flush_interval + + @property + def inline_users_in_events(self): + return self.__inline_users_in_events + def _validate(self): if self.offline is False and self.sdk_key is None or self.sdk_key is '': log.warn("Missing or blank sdk_key.") From 2870207f3cc375b7bd4a124eea216cee79487d30 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Mon, 19 Mar 2018 21:04:30 -0700 Subject: [PATCH 06/48] implement event summarizer --- ldclient/event_summarizer.py | 86 +++++++++++++++++++++++++ testing/test_event_summarizer.py | 105 +++++++++++++++++++++++++++++++ 2 files changed, 191 insertions(+) create mode 100644 ldclient/event_summarizer.py create mode 100644 testing/test_event_summarizer.py diff --git a/ldclient/event_summarizer.py b/ldclient/event_summarizer.py new file mode 100644 index 00000000..8320b9c2 --- /dev/null +++ b/ldclient/event_summarizer.py @@ -0,0 +1,86 @@ +import pylru + +class EventSummarizer(object): + def __init__(self, config): + self.user_keys = pylru.lrucache(config.user_keys_capacity) + self.start_date = 0 + self.end_date = 0 + self.counters = dict() + + """ + Add to the set of users we've noticed, and return true if the user was already known to us. + """ + def notice_user(self, user): + if user is None or 'key' not in user: + return False + key = user['key'] + if key in self.user_keys: + self.user_keys[key] # refresh cache item + return True + self.user_keys[key] = True + return False + + """ + Reset the set of users we've seen. + """ + def reset_users(self): + self.user_keys.clear() + + """ + Add this event to our counters, if it is a type of event we need to count. + """ + def summarize_event(self, event): + if event['kind'] == 'feature': + counter_key = (event['key'], event['variation'], event['version']) + counter_val = self.counters.get(counter_key) + if counter_val is None: + counter_val = { 'count': 1, 'value': event['value'], 'default': event['default'] } + self.counters[counter_key] = counter_val + else: + counter_val['count'] = counter_val['count'] + 1 + date = event['creationDate'] + if self.start_date == 0 or date < self.start_date: + self.start_date = date + if date > self.end_date: + self.end_date = date + + """ + Return a snapshot of the current summarized event data, and reset this state. + """ + def snapshot(self): + ret = { + 'start_date': self.start_date, + 'end_date': self.end_date, + 'counters': self.counters + } + self.start_date = 0 + self.end_date = 0 + self.counters = dict() + return ret + + """ + Transform the summary data into the format used for event sending. + """ + def output(self, snapshot_data): + counters = snapshot_data['counters'] + flags_out = dict() + for ckey, cval in counters.items(): + flag_key, variation, version = ckey + flag_data = flags_out.get(flag_key) + if flag_data is None: + flag_data = { 'default': cval['default'], 'counters': [] } + flags_out[flag_key] = flag_data + counter = { + 'count': cval['count'], + 'value': cval['value'] + } + if version is None: + counter['unknown'] = True + else: + counter['version'] = version + flag_data['counters'].append(counter) + return { + 'start_date': snapshot_data['start_date'], + 'end_date': snapshot_data['end_date'], + 'features': flags_out + } diff --git a/testing/test_event_summarizer.py b/testing/test_event_summarizer.py new file mode 100644 index 00000000..04c339a3 --- /dev/null +++ b/testing/test_event_summarizer.py @@ -0,0 +1,105 @@ +import pytest + +from ldclient.config import Config +from ldclient.event_summarizer import EventSummarizer + + +user = { 'key': 'user1' } + +def test_notice_user_returns_false_for_never_seen_user(): + es = EventSummarizer(Config()) + assert es.notice_user(user) == False + +def test_notice_user_returns_true_for_previously_seen_user(): + es = EventSummarizer(Config()) + es.notice_user(user) + assert es.notice_user({ 'key': user['key'] }) == True + +def test_oldest_user_forgotten_if_capacity_exceeded(): + es = EventSummarizer(Config(user_keys_capacity = 2)) + user1 = { 'key': 'user1' } + user2 = { 'key': 'user2' } + user3 = { 'key': 'user3' } + es.notice_user(user1) + es.notice_user(user2) + es.notice_user(user3) + assert es.notice_user(user3) == True + assert es.notice_user(user2) == True + assert es.notice_user(user1) == False + +def test_summarize_event_does_nothing_for_identify_event(): + es = EventSummarizer(Config()) + snapshot = es.snapshot() + es.summarize_event({ 'kind': 'identify', 'creationDate': 1000, 'user': user }) + + assert es.snapshot() == snapshot + +def test_summarize_event_does_nothing_for_custom_event(): + es = EventSummarizer(Config()) + snapshot = es.snapshot() + es.summarize_event({ 'kind': 'custom', 'creationDate': 1000, 'key': 'eventkey', 'user': user }) + + assert es.snapshot() == snapshot + +def test_summarize_event_sets_start_and_end_dates(): + es = EventSummarizer(Config()) + event1 = { 'kind': 'feature', 'creationDate': 2000, 'key': 'flag', 'user': user, + 'version': 1, 'variation': 0, 'value': '', 'default': None } + event2 = { 'kind': 'feature', 'creationDate': 1000, 'key': 'flag', 'user': user, + 'version': 1, 'variation': 0, 'value': '', 'default': None } + event3 = { 'kind': 'feature', 'creationDate': 1500, 'key': 'flag', 'user': user, + 'version': 1, 'variation': 0, 'value': '', 'default': None } + es.summarize_event(event1) + es.summarize_event(event2) + es.summarize_event(event3) + data = es.output(es.snapshot()) + + assert data['start_date'] == 1000 + assert data['end_date'] == 2000 + +def test_summarize_event_increments_counters(): + es = EventSummarizer(Config()) + event1 = { 'kind': 'feature', 'creationDate': 1000, 'key': 'flag1', 'user': user, + 'version': 11, 'variation': 1, 'value': 'value1', 'default': 'default1' } + event2 = { 'kind': 'feature', 'creationDate': 1000, 'key': 'flag1', 'user': user, + 'version': 11, 'variation': 2, 'value': 'value2', 'default': 'default1' } + event3 = { 'kind': 'feature', 'creationDate': 1000, 'key': 'flag2', 'user': user, + 'version': 22, 'variation': 1, 'value': 'value99', 'default': 'default2' } + event4 = { 'kind': 'feature', 'creationDate': 1000, 'key': 'flag1', 'user': user, + 'version': 11, 'variation': 1, 'value': 'value1', 'default': 'default1' } + event5 = { 'kind': 'feature', 'creationDate': 1000, 'key': 'badkey', 'user': user, + 'version': None, 'variation': None, 'value': 'default3', 'default': 'default3' } + es.summarize_event(event1) + es.summarize_event(event2) + es.summarize_event(event3) + es.summarize_event(event4) + es.summarize_event(event5) + data = es.output(es.snapshot()) + + data['features']['flag1']['counters'].sort(key = lambda c: c['value']) + expected = { + 'start_date': 1000, + 'end_date': 1000, + 'features': { + 'flag1': { + 'default': 'default1', + 'counters': [ + { 'version': 11, 'value': 'value1', 'count': 2 }, + { 'version': 11, 'value': 'value2', 'count': 1 } + ] + }, + 'flag2': { + 'default': 'default2', + 'counters': [ + { 'version': 22, 'value': 'value99', 'count': 1 } + ] + }, + 'badkey': { + 'default': 'default3', + 'counters': [ + { 'unknown': True, 'value': 'default3', 'count': 1} + ] + } + } + } + assert data == expected From ab48ef87ee92f1f404f6aac6a2caaa664015136c Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Mon, 19 Mar 2018 21:09:56 -0700 Subject: [PATCH 07/48] add pylru dependency --- requirements.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/requirements.txt b/requirements.txt index ebdbadf1..c74c7469 100644 --- a/requirements.txt +++ b/requirements.txt @@ -6,3 +6,4 @@ six>=1.10.0 pyRFC3339>=1.0 jsonpickle==0.9.3 semver>=2.7.9 +pylru>=1.0.9 From 57f5511b58075aa3c7cb9f0ae035d5caec2f88f7 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Wed, 21 Mar 2018 13:16:24 -0700 Subject: [PATCH 08/48] rename EventConsumer to EventProcessor; don't expose its internal queue; fix tests to avoid making real connections --- ldclient/client.py | 30 ++- ldclient/config.py | 18 +- .../{event_consumer.py => event_processor.py} | 83 +++++++- ldclient/interfaces.py | 12 +- testing/test_ldclient.py | 195 ++++++++---------- 5 files changed, 190 insertions(+), 148 deletions(-) rename ldclient/{event_consumer.py => event_processor.py} (58%) diff --git a/ldclient/client.py b/ldclient/client.py index 8309652d..771fb155 100644 --- a/ldclient/client.py +++ b/ldclient/client.py @@ -9,6 +9,7 @@ from builtins import object from ldclient.config import Config as Config +from ldclient.event_processor import NullEventProcessor from ldclient.feature_requester import FeatureRequesterImpl from ldclient.flag import evaluate from ldclient.polling import PollingUpdateProcessor @@ -43,21 +44,22 @@ def __init__(self, sdk_key=None, config=None, start_wait=5): self._config._validate() self._session = CacheControl(requests.Session()) - self._queue = queue.Queue(self._config.events_max_pending) - self._event_consumer = None + self._event_processor = None self._lock = Lock() self._store = self._config.feature_store """ :type: FeatureStore """ + if self._config.offline or not self._config.send_events: + self._event_processor = NullEventProcessor() + else: + self._event_processor = self._config.event_processor_class(self._config) + self._event_processor.start() + if self._config.offline: log.info("Started LaunchDarkly Client in offline mode") return - if self._config.send_events: - self._event_consumer = self._config.event_consumer_class(self._queue, self._config) - self._event_consumer.start() - if self._config.use_ldd: log.info("Started LaunchDarkly Client in LDD mode") return @@ -75,6 +77,7 @@ def __init__(self, sdk_key=None, config=None, start_wait=5): self._update_processor = self._config.update_processor_class( self._config, self._feature_requester, self._store, update_processor_ready) else: + raise "*** NOT HERE ***" if self._config.stream: self._update_processor = StreamingUpdateProcessor( self._config, self._feature_requester, self._store, update_processor_ready) @@ -102,19 +105,14 @@ def close(self): log.info("Closing LaunchDarkly client..") if self.is_offline(): return - if self._event_consumer and self._event_consumer.is_alive(): - self._event_consumer.stop() + if self._event_processor and self._event_processor.is_alive(): + self._event_processor.stop() if self._update_processor and self._update_processor.is_alive(): self._update_processor.stop() def _send_event(self, event): - if self._config.offline or not self._config.send_events: - return event['creationDate'] = int(time.time() * 1000) - if self._queue.full(): - log.warning("Event queue is full-- dropped an event") - else: - self._queue.put(event) + self._event_processor.send_event(event) def track(self, event_name, user, data=None): self._sanitize_user(user) @@ -135,9 +133,9 @@ def is_initialized(self): return self.is_offline() or self._config.use_ldd or self._update_processor.initialized() def flush(self): - if self._config.offline or not self._config.send_events: + if self._config.offline: return - return self._event_consumer.flush() + return self._event_processor.flush() def toggle(self, key, user, default): log.warn("Deprecated method: toggle() called. Use variation() instead.") diff --git a/ldclient/config.py b/ldclient/config.py index 28285bc0..12e965f5 100644 --- a/ldclient/config.py +++ b/ldclient/config.py @@ -1,4 +1,4 @@ -from ldclient.event_consumer import EventConsumerImpl +from ldclient.event_processor import DefaultEventProcessor from ldclient.feature_store import InMemoryFeatureStore from ldclient.util import log @@ -26,7 +26,7 @@ def __init__(self, use_ldd=False, feature_store=InMemoryFeatureStore(), feature_requester_class=None, - event_consumer_class=None, + event_processor_class=None, private_attribute_names=(), all_attributes_private=False, offline=False, @@ -78,8 +78,8 @@ def __init__(self, full details for the user. :param feature_requester_class: A factory for a FeatureRequester implementation taking the sdk key and config :type feature_requester_class: (str, Config, FeatureStore) -> FeatureRequester - :param event_consumer_class: A factory for an EventConsumer implementation taking the event queue, sdk key, and config - :type event_consumer_class: (queue.Queue, str, Config) -> EventConsumer + :param event_processor_class: A factory for an EventProcessor implementation taking the config + :type event_processor_class: (Config) -> EventProcessor :param update_processor_class: A factory for an UpdateProcessor implementation taking the sdk key, config, and FeatureStore implementation """ @@ -96,7 +96,7 @@ def __init__(self, self.__poll_interval = max(poll_interval, 30) self.__use_ldd = use_ldd self.__feature_store = InMemoryFeatureStore() if not feature_store else feature_store - self.__event_consumer_class = EventConsumerImpl if not event_consumer_class else event_consumer_class + self.__event_processor_class = DefaultEventProcessor if not event_processor_class else event_processor_class self.__feature_requester_class = feature_requester_class self.__connect_timeout = connect_timeout self.__read_timeout = read_timeout @@ -136,7 +136,7 @@ def copy_with_new_sdk_key(self, new_sdk_key): use_ldd=self.__use_ldd, feature_store=self.__feature_store, feature_requester_class=self.__feature_requester_class, - event_consumer_class=self.__event_consumer_class, + event_processor_class=self.__event_processor_class, private_attribute_names=self.__private_attribute_names, all_attributes_private=self.__all_attributes_private, offline=self.__offline, @@ -192,8 +192,8 @@ def feature_store(self): return self.__feature_store @property - def event_consumer_class(self): - return self.__event_consumer_class + def event_processor_class(self): + return self.__event_processor_class @property def feature_requester_class(self): @@ -250,7 +250,7 @@ def user_keys_flush_interval(self): @property def inline_users_in_events(self): return self.__inline_users_in_events - + def _validate(self): if self.offline is False and self.sdk_key is None or self.sdk_key is '': log.warn("Missing or blank sdk_key.") diff --git a/ldclient/event_consumer.py b/ldclient/event_processor.py similarity index 58% rename from ldclient/event_consumer.py rename to ldclient/event_processor.py index a1a6cd2e..3586d5dd 100644 --- a/ldclient/event_consumer.py +++ b/ldclient/event_processor.py @@ -3,24 +3,50 @@ import errno from threading import Thread +# noinspection PyBroadException +try: + import queue +except: + # noinspection PyUnresolvedReferences,PyPep8Naming + import Queue as queue + import requests from requests.packages.urllib3.exceptions import ProtocolError import six from ldclient.user_filter import UserFilter -from ldclient.interfaces import EventConsumer +from ldclient.interfaces import EventProcessor from ldclient.util import _headers from ldclient.util import log -class EventConsumerImpl(Thread, EventConsumer): - def __init__(self, event_queue, config): +class NullEventProcessor(Thread, EventProcessor): + def __init(self, config): + pass + + def start(self): + pass + + def stop(self): + pass + + def is_alive(self): + return False + + def send_event(self, event): + pass + + def flush(self): + pass + +class DefaultEventProcessor(Thread, EventProcessor): + def __init__(self, config): Thread.__init__(self) self._session = requests.Session() self.daemon = True self._config = config - self._queue = event_queue + self._queue = queue.Queue(config.events_max_pending) self._user_filter = UserFilter(config) self._running = True @@ -37,6 +63,12 @@ def run(self): def stop(self): self._running = False + def send_event(self, event): + if self._queue.full(): + log.warning("Event queue is full-- dropped an event") + else: + self._queue.put(event) + def flush(self): self._queue.join() @@ -44,8 +76,8 @@ def send_batch(self, events): def do_send(should_retry): # noinspection PyBroadException try: - filtered_events = [ self._filter_event(e) for e in events ] - json_body = jsonpickle.encode(filtered_events, unpicklable=False) + output_events = [ self._make_output_event(e) for e in events ] + json_body = jsonpickle.encode(output_events, unpicklable=False) log.debug('Sending events payload: ' + json_body) hdrs = _headers(self._config.sdk_key) uri = self._config.events_uri @@ -80,8 +112,43 @@ def do_send(should_retry): for _ in events: self._queue.task_done() - def _filter_event(self, e): - return dict((key, self._user_filter.filter_user_props(value) if key == 'user' else value) for (key, value) in six.iteritems(e)) + def _make_output_event(self, e): + kind = e['kind'] + if kind == 'feature': + is_debug = (not e['trackEvents']) and (e.get('debugEventsUntilDate') is not None) + out = { + 'kind': 'debug' if is_debug else 'feature', + 'creationDate': e['creationDate'], + 'key': e['key'], + 'version': e.get('version'), + 'value': e.get('value'), + 'default': e.get('default'), + 'prereqOf': e.get('prereqOf') + } + if self._config.inline_users_in_events: + out['user'] = self._user_filter.filter_user_props(e['user']) + else: + out['userKey'] = e['user'].get('key') + return out + elif kind == 'identify': + return { + 'kind': 'identify', + 'creationDate': e['creationDate'], + 'user': self._user_filter.filter_user_props(e['user']) + } + elif kind == 'custom': + out = { + 'kind': 'custom', + 'key': e['key'], + 'data': e.get('data') + } + if self._config.inline_users_in_events: + out['user'] = self._user_filter.filter_user_props(e['user']) + else: + out['userKey'] = e['user'].get('key') + return out + else: + return e def send(self): events = self.next() diff --git a/ldclient/interfaces.py b/ldclient/interfaces.py index af1caa86..dd5065fa 100644 --- a/ldclient/interfaces.py +++ b/ldclient/interfaces.py @@ -113,16 +113,22 @@ def initialized(self): """ -class EventConsumer(BackgroundOperation): +class EventProcessor(BackgroundOperation): """ - Consumes events from the client and sends them to LaunchDarkly + Buffers analytics events and sends them to LaunchDarkly """ __metaclass__ = ABCMeta + @abstractmethod + def send_event(self, event): + """ + Processes an event to be sent at some point. + """ + @abstractmethod def flush(self): """ - Flushes any outstanding events immediately. + Sends any outstanding events immediately. """ diff --git a/testing/test_ldclient.py b/testing/test_ldclient.py index b6585362..1047ec7c 100644 --- a/testing/test_ldclient.py +++ b/testing/test_ldclient.py @@ -1,7 +1,8 @@ from builtins import object from ldclient.client import LDClient, Config +from ldclient.event_processor import NullEventProcessor from ldclient.feature_store import InMemoryFeatureStore -from ldclient.interfaces import FeatureRequester, FeatureStore +from ldclient.interfaces import FeatureRequester, FeatureStore, UpdateProcessor import pytest from testing.sync_util import wait_until @@ -54,34 +55,11 @@ def get(self, key): return None -client = LDClient(config=Config(base_uri="http://localhost:3000", feature_store=MockFeatureStore())) -offline_client = LDClient(config= - Config(sdk_key="secret", base_uri="http://localhost:3000", feature_store=MockFeatureStore(), - offline=True)) -no_send_events_client = LDClient(config= - Config(sdk_key="secret", base_uri="http://localhost:3000", feature_store=MockFeatureStore(), - send_events=False)) - -user = { - u'key': u'xyz', - u'custom': { - u'bizzle': u'def' - } -} - -numeric_key_user = {} - -sanitized_numeric_key_user = { - u'key': '33', - u'custom': { - u'bizzle': u'def' - } -} - - -class MockConsumer(object): +class MockEventProcessor(object): def __init__(self, *_): self._running = False + self._events = [] + mock_event_processor = self def stop(self): self._running = False @@ -92,6 +70,9 @@ def start(self): def is_alive(self): return self._running + def send_event(self, event): + self._events.append(event) + def flush(self): pass @@ -104,12 +85,44 @@ def get_all(self): pass -def mock_consumer(): - return MockConsumer() +class MockUpdateProcessor(UpdateProcessor): + def __init__(self, config, requester, store, ready): + ready.set() + def start(self): + pass -def noop_consumer(): - return + def stop(self): + pass + + def is_alive(self): + return True + + +client = LDClient(config=Config(base_uri="http://localhost:3000", feature_store=MockFeatureStore(), + event_processor_class = MockEventProcessor, update_processor_class = MockUpdateProcessor)) +offline_client = LDClient(config= + Config(sdk_key="secret", base_uri="http://localhost:3000", feature_store=MockFeatureStore(), + offline=True)) +no_send_events_client = LDClient(config= + Config(sdk_key="secret", base_uri="http://localhost:3000", feature_store=MockFeatureStore(), + update_processor_class = MockUpdateProcessor, send_events=False)) + +user = { + u'key': u'xyz', + u'custom': { + u'bizzle': u'def' + } +} + +numeric_key_user = {} + +sanitized_numeric_key_user = { + u'key': '33', + u'custom': { + u'bizzle': u'def' + } +} def setup_function(function): @@ -120,13 +133,10 @@ def setup_function(function): u'bizzle': u'def' } } - client._queue = queue.Queue(10) - client._event_consumer = mock_consumer() -def wait_for_event(c, cb): - e = c._queue.get(False) - return cb(e) +def get_first_event(c): + return c._event_processor._events.pop(0) def test_ctor_both_sdk_keys_set(): @@ -135,6 +145,14 @@ def test_ctor_both_sdk_keys_set(): LDClient(sdk_key="sdk key b", config=config) +def test_client_has_null_event_processor_if_offline(): + assert isinstance(offline_client._event_processor, NullEventProcessor) + + +def test_client_has_null_event_processor_if_send_events_off(): + assert isinstance(no_send_events_client._event_processor, NullEventProcessor) + + def test_toggle_offline(): assert offline_client.variation('feature.key', user, default=None) is None @@ -144,99 +162,65 @@ def test_sanitize_user(): assert numeric_key_user == sanitized_numeric_key_user -def test_toggle_event_offline(): - offline_client.variation('feature.key', user, default=None) - assert offline_client._queue.empty() - - -def test_toggle_event_with_send_events_off(): - no_send_events_client.variation('feature.key', user, default=None) - assert no_send_events_client._queue.empty() - - def test_identify(): client.identify(user) - def expected_event(e): - return e['kind'] == 'identify' and e['key'] == u'xyz' and e['user'] == user - - assert expected_event(client._queue.get(False)) + e = get_first_event(client) + assert e['kind'] == 'identify' and e['key'] == u'xyz' and e['user'] == user def test_identify_numeric_key_user(): client.identify(numeric_key_user) - def expected_event(e): - return e['kind'] == 'identify' and e['key'] == '33' and e['user'] == sanitized_numeric_key_user - - assert expected_event(client._queue.get(False)) - - -def test_identify_offline(): - offline_client.identify(numeric_key_user) - assert offline_client._queue.empty() - - -def test_identify_with_send_events_off(): - no_send_events_client.identify(numeric_key_user) - assert no_send_events_client._queue.empty() + e = get_first_event(client) + assert e['kind'] == 'identify' and e['key'] == '33' and e['user'] == sanitized_numeric_key_user def test_track(): client.track('my_event', user, 42) - def expected_event(e): - return e['kind'] == 'custom' and e['key'] == 'my_event' and e['user'] == user and e['data'] == 42 - - assert expected_event(client._queue.get(False)) + e = get_first_event(client) + assert e['kind'] == 'custom' and e['key'] == 'my_event' and e['user'] == user and e['data'] == 42 def test_track_numeric_key_user(): client.track('my_event', numeric_key_user, 42) - def expected_event(e): - return e['kind'] == 'custom' and e['key'] == 'my_event' and e['user'] == sanitized_numeric_key_user \ - and e['data'] == 42 - - assert expected_event(client._queue.get(False)) - - -def test_track_offline(): - offline_client.track('my_event', user, 42) - assert offline_client._queue.empty() - - -def test_track_with_send_events_off(): - no_send_events_client.track('my_event', user, 42) - assert no_send_events_client._queue.empty() + e = get_first_event(client) + assert e['kind'] == 'custom' and e['key'] == 'my_event' and e['user'] == sanitized_numeric_key_user \ + and e['data'] == 42 def test_defaults(): - client = LDClient(config=Config(base_uri="http://localhost:3000", - defaults={"foo": "bar"}, - offline=True)) - assert "bar" == client.variation('foo', user, default=None) + my_client = LDClient(config=Config(base_uri="http://localhost:3000", + defaults={"foo": "bar"}, + offline=True)) + assert "bar" == my_client.variation('foo', user, default=None) def test_defaults_and_online(): expected = "bar" my_client = LDClient(config=Config(base_uri="http://localhost:3000", defaults={"foo": expected}, - event_consumer_class=MockConsumer, + event_processor_class=MockEventProcessor, feature_requester_class=MockFeatureRequester, + update_processor_class=MockUpdateProcessor, feature_store=InMemoryFeatureStore())) actual = my_client.variation('foo', user, default="originalDefault") assert actual == expected - assert wait_for_event(my_client, lambda e: e['kind'] == 'feature' and e['key'] == u'foo' and e['user'] == user) + e = get_first_event(my_client) + assert e['kind'] == 'feature' and e['key'] == u'foo' and e['user'] == user def test_defaults_and_online_no_default(): - client = LDClient(config=Config(base_uri="http://localhost:3000", - defaults={"foo": "bar"}, - event_consumer_class=MockConsumer, - feature_requester_class=MockFeatureRequester)) - assert "jim" == client.variation('baz', user, default="jim") - assert wait_for_event(client, lambda e: e['kind'] == 'feature' and e['key'] == u'baz' and e['user'] == user) + my_client = LDClient(config=Config(base_uri="http://localhost:3000", + defaults={"foo": "bar"}, + event_processor_class=MockEventProcessor, + feature_requester_class=MockFeatureRequester, + update_processor_class=MockUpdateProcessor)) + assert "jim" == my_client.variation('baz', user, default="jim") + e = get_first_event(my_client) + assert e['kind'] == 'feature' and e['key'] == u'baz' and e['user'] == user def test_exception_in_retrieval(): @@ -251,9 +235,11 @@ def get_all(self): defaults={"foo": "bar"}, feature_store=InMemoryFeatureStore(), feature_requester_class=ExceptionFeatureRequester, - event_consumer_class=MockConsumer)) + event_processor_class=MockEventProcessor, + update_processor_class=MockUpdateProcessor)) assert "bar" == client.variation('foo', user, default="jim") - assert wait_for_event(client, lambda e: e['kind'] == 'feature' and e['key'] == u'foo' and e['user'] == user) + e = get_first_event(client) + assert e['kind'] == 'feature' and e['key'] == u'foo' and e['user'] == user def test_no_defaults(): @@ -263,18 +249,3 @@ def test_no_defaults(): def test_secure_mode_hash(): user = {'key': 'Message'} assert offline_client.secure_mode_hash(user) == "aa747c502a898200f9e4fa21bac68136f886a0e27aec70ba06daf2e2a5cb5597" - - -def drain(queue): - while not queue.empty(): - queue.get() - queue.task_done() - return - - -def test_flush_empties_queue(): - client.track('my_event', user, 42) - client.track('my_event', user, 33) - drain(client._queue) - client.flush() - assert client._queue.empty() From 2e9ef50b357a6dea4f9ac3b2b4fc745ba200fb35 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Wed, 21 Mar 2018 13:23:32 -0700 Subject: [PATCH 09/48] don't need to create a FeatureRequestor if we're using a custom update processor --- ldclient/client.py | 19 +++++---- testing/test_ldclient.py | 86 ++++++++++++++-------------------------- 2 files changed, 38 insertions(+), 67 deletions(-) diff --git a/ldclient/client.py b/ldclient/client.py index 771fb155..88a836d3 100644 --- a/ldclient/client.py +++ b/ldclient/client.py @@ -64,28 +64,27 @@ def __init__(self, sdk_key=None, config=None, start_wait=5): log.info("Started LaunchDarkly Client in LDD mode") return - if self._config.feature_requester_class: - self._feature_requester = self._config.feature_requester_class(self._config) - else: - self._feature_requester = FeatureRequesterImpl(self._config) - """ :type: FeatureRequester """ - update_processor_ready = threading.Event() if self._config.update_processor_class: log.info("Using user-specified update processor: " + str(self._config.update_processor_class)) self._update_processor = self._config.update_processor_class( - self._config, self._feature_requester, self._store, update_processor_ready) + self._config, self._store, update_processor_ready) else: - raise "*** NOT HERE ***" + if self._config.feature_requester_class: + _feature_requester = self._config.feature_requester_class(self._config) + else: + _feature_requester = FeatureRequesterImpl(self._config) + """ :type: FeatureRequester """ + if self._config.stream: self._update_processor = StreamingUpdateProcessor( - self._config, self._feature_requester, self._store, update_processor_ready) + self._config, _feature_requester, self._store, update_processor_ready) else: log.info("Disabling streaming API") log.warn("You should only disable the streaming API if instructed to do so by LaunchDarkly support") self._update_processor = PollingUpdateProcessor( - self._config, self._feature_requester, self._store, update_processor_ready) + self._config, _feature_requester, self._store, update_processor_ready) """ :type: UpdateProcessor """ self._update_processor.start() diff --git a/testing/test_ldclient.py b/testing/test_ldclient.py index 1047ec7c..94a3efef 100644 --- a/testing/test_ldclient.py +++ b/testing/test_ldclient.py @@ -3,6 +3,7 @@ from ldclient.event_processor import NullEventProcessor from ldclient.feature_store import InMemoryFeatureStore from ldclient.interfaces import FeatureRequester, FeatureStore, UpdateProcessor +from ldclient.versioned_data_kind import FEATURES import pytest from testing.sync_util import wait_until @@ -12,49 +13,6 @@ import Queue as queue -class MockFeatureStore(FeatureStore): - def delete(self, key, version): - pass - - @property - def initialized(self): - pass - - def init(self, features): - pass - - def all(self): - pass - - def upsert(self, key, feature): - pass - - def __init__(self, *_): - pass - - def get(self, key): - if key == "feature.key": - return { - u'key': u'feature.key', - u'salt': u'abc', - u'on': True, - u'variations': [ - { - u'value': True, - u'weight': 100, - u'targets': [] - }, - { - u'value': False, - u'weight': 0, - u'targets': [] - } - ] - } - else: - return None - - class MockEventProcessor(object): def __init__(self, *_): self._running = False @@ -77,16 +35,8 @@ def flush(self): pass -class MockFeatureRequester(FeatureRequester): - def __init__(self, *_): - pass - - def get_all(self): - pass - - class MockUpdateProcessor(UpdateProcessor): - def __init__(self, config, requester, store, ready): + def __init__(self, config, store, ready): ready.set() def start(self): @@ -99,13 +49,37 @@ def is_alive(self): return True -client = LDClient(config=Config(base_uri="http://localhost:3000", feature_store=MockFeatureStore(), +feature = { + u'key': u'feature.key', + u'salt': u'abc', + u'on': True, + u'variations': [ + { + u'value': True, + u'weight': 100, + u'targets': [] + }, + { + u'value': False, + u'weight': 0, + u'targets': [] + } + ] +} +feature_store = InMemoryFeatureStore() +feature_store.init({ + FEATURES: { + 'feature.key': feature + } +}) + +client = LDClient(config=Config(base_uri="http://localhost:3000", event_processor_class = MockEventProcessor, update_processor_class = MockUpdateProcessor)) offline_client = LDClient(config= - Config(sdk_key="secret", base_uri="http://localhost:3000", feature_store=MockFeatureStore(), + Config(sdk_key="secret", base_uri="http://localhost:3000", offline=True)) no_send_events_client = LDClient(config= - Config(sdk_key="secret", base_uri="http://localhost:3000", feature_store=MockFeatureStore(), + Config(sdk_key="secret", base_uri="http://localhost:3000", update_processor_class = MockUpdateProcessor, send_events=False)) user = { @@ -203,7 +177,6 @@ def test_defaults_and_online(): my_client = LDClient(config=Config(base_uri="http://localhost:3000", defaults={"foo": expected}, event_processor_class=MockEventProcessor, - feature_requester_class=MockFeatureRequester, update_processor_class=MockUpdateProcessor, feature_store=InMemoryFeatureStore())) actual = my_client.variation('foo', user, default="originalDefault") @@ -216,7 +189,6 @@ def test_defaults_and_online_no_default(): my_client = LDClient(config=Config(base_uri="http://localhost:3000", defaults={"foo": "bar"}, event_processor_class=MockEventProcessor, - feature_requester_class=MockFeatureRequester, update_processor_class=MockUpdateProcessor)) assert "jim" == my_client.variation('baz', user, default="jim") e = get_first_event(my_client) From d32adaa11e03bdac745d17e340a0d938da5b1a53 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Wed, 21 Mar 2018 17:38:45 -0700 Subject: [PATCH 10/48] implement summary events in DefaultEventProcessor --- ldclient/client.py | 2 - ldclient/event_processor.py | 227 +++++++++++++------- ldclient/event_summarizer.py | 4 +- testing/test_event_processor.py | 356 +++++++++++++++++++++++++++++++ testing/test_event_summarizer.py | 8 +- 5 files changed, 510 insertions(+), 87 deletions(-) create mode 100644 testing/test_event_processor.py diff --git a/ldclient/client.py b/ldclient/client.py index 88a836d3..b6801199 100644 --- a/ldclient/client.py +++ b/ldclient/client.py @@ -3,7 +3,6 @@ import hashlib import hmac import threading -import time import requests from builtins import object @@ -110,7 +109,6 @@ def close(self): self._update_processor.stop() def _send_event(self, event): - event['creationDate'] = int(time.time() * 1000) self._event_processor.send_event(event) def track(self, event_name, user, data=None): diff --git a/ldclient/event_processor.py b/ldclient/event_processor.py index 3586d5dd..6e89c103 100644 --- a/ldclient/event_processor.py +++ b/ldclient/event_processor.py @@ -1,7 +1,10 @@ from __future__ import absolute_import +from email.utils import parsedate import errno -from threading import Thread +import jsonpickle +from threading import Event, Thread, Timer +import time # noinspection PyBroadException try: @@ -15,6 +18,7 @@ import six +from ldclient.event_summarizer import EventSummarizer from ldclient.user_filter import UserFilter from ldclient.interfaces import EventProcessor from ldclient.util import _headers @@ -40,77 +44,175 @@ def send_event(self, event): def flush(self): pass + class DefaultEventProcessor(Thread, EventProcessor): - def __init__(self, config): + def __init__(self, config, session=None): Thread.__init__(self) - self._session = requests.Session() + self._session = requests.Session() if session is None else session self.daemon = True self._config = config self._queue = queue.Queue(config.events_max_pending) + self._events = [] self._user_filter = UserFilter(config) + self._summarizer = EventSummarizer(config) + self._last_known_past_time = 0 self._running = True + self._set_flush_timer() + self._set_users_flush_timer() def run(self): log.info("Starting event consumer") self._running = True while self._running: try: - self.send() + self._process_next() + log.error("*** processed ***") except Exception: - log.warning( - 'Unhandled exception in event consumer') + log.error('Unhandled exception in event consumer', exc_info=True) def stop(self): + log.error("*** GOT STOP ***") + self.flush() + self._session.close() self._running = False + self._flush_timer.cancel() + self._users_flush_timer.cancel() + # Post a non-message so we won't keep blocking on the queue + self._queue.put(('stop', None)) def send_event(self, event): - if self._queue.full(): - log.warning("Event queue is full-- dropped an event") - else: - self._queue.put(event) + event['creationDate'] = self._now() + self._queue.put(('event', event)) def flush(self): - self._queue.join() + # Put a flush message on the queue and wait until it's been processed. + reply = Event() + self._queue.put(('flush', reply)) + reply.wait() + + def _now(self): + return int(time.time() * 1000) + + def _set_flush_timer(self): + self._flush_timer = Timer(5, self._flush_async) + self._flush_timer.start() + + def _set_users_flush_timer(self): + self._users_flush_timer = Timer(self._config.user_keys_flush_interval, self._flush_users) + self._users_flush_timer.start() + + def _flush_async(self): + self._queue.put(('flush', None)) + self._set_flush_timer() + + def _flush_users(self): + self._queue.put(('flush_users', None)) + self._set_users_flush_timer() + + def _process_next(self): + item = self._queue.get(block=True) + if item[0] == 'event': + self._process_event(item[1]) + elif item[0] == 'flush': + self._dispatch_flush(item[1]) + elif item[0] == 'flush_users': + self._summarizer.reset_users() + + def _process_event(self, event): + # For each user we haven't seen before, we add an index event - unless this is already + # an identify event for that user. + user = event.get('user') + if not self._config.inline_users_in_events and user and not self._summarizer.notice_user(user): + if event['kind'] != 'identify': + ie = { 'kind': 'index', 'creationDate': event['creationDate'], 'user': user } + self._store_event(ie) + + # Always record the event in the summarizer. + self._summarizer.summarize_event(event) + + if self._should_track_full_event(event): + # Queue the event as-is; we'll transform it into an output event when we're flushing + # (to avoid doing that work on our main thread). + self._store_event(event) + + def _store_event(self, event): + if len(self._events) >= self._config.events_max_pending: + log.warning("Event queue is full-- dropped an event") + else: + self._events.append(event) + + def _should_track_full_event(self, event): + if event['kind'] == 'feature': + if event.get('trackEvents'): + return True + debug_until = event.get('debugEventsUntilDate') + if debug_until is not None: + last_past = self._last_known_past_time + if debug_until > last_past and debug_until > self._now(): + return True + return False + else: + return True + + def _dispatch_flush(self, reply): + events = self._events + self._events = [] + snapshot = self._summarizer.snapshot() + if len(events) > 0 or len(snapshot['counters']) > 0: + flusher = Thread(target = self._flush_task, args=(events, snapshot, reply)) + flusher.start() + else: + if reply is not None: + reply.set() + + def _flush_task(self, events, snapshot, reply): + output_events = [ self._make_output_event(e) for e in events ] + if len(snapshot['counters']) > 0: + summary = self._summarizer.output(snapshot) + summary['kind'] = 'summary' + output_events.append(summary) + try: + self._do_send(output_events, True) + finally: + if reply is not None: + reply.set() - def send_batch(self, events): - def do_send(should_retry): - # noinspection PyBroadException - try: - output_events = [ self._make_output_event(e) for e in events ] - json_body = jsonpickle.encode(output_events, unpicklable=False) - log.debug('Sending events payload: ' + json_body) - hdrs = _headers(self._config.sdk_key) - uri = self._config.events_uri - r = self._session.post(uri, - headers=hdrs, - timeout=(self._config.connect_timeout, self._config.read_timeout), - data=json_body) - if r.status_code == 401: - log.error('Received 401 error, no further events will be posted since SDK key is invalid') - self.stop() - return - r.raise_for_status() - except ProtocolError as e: - if e.args is not None and len(e.args) > 1 and e.args[1] is not None: - inner = e.args[1] - if inner.errno is not None and inner.errno == errno.ECONNRESET and should_retry: - log.warning( - 'ProtocolError exception caught while sending events. Retrying.') - do_send(False) - else: + def _do_send(self, output_events, should_retry): + # noinspection PyBroadException + try: + json_body = jsonpickle.encode(output_events, unpicklable=False) + log.debug('Sending events payload: ' + json_body) + hdrs = _headers(self._config.sdk_key) + uri = self._config.events_uri + r = self._session.post(uri, + headers=hdrs, + timeout=(self._config.connect_timeout, self._config.read_timeout), + data=json_body) + server_date_str = r.headers.get('Date') + if server_date_str is not None: + server_date = parsedate(server_date_str) + if server_date is not None: + self._last_known_past_time = server_date + if r.status_code == 401: + log.error('Received 401 error, no further events will be posted since SDK key is invalid') + self.stop() + return + r.raise_for_status() + except ProtocolError as e: + if e.args is not None and len(e.args) > 1 and e.args[1] is not None: + inner = e.args[1] + if inner.errno is not None and inner.errno == errno.ECONNRESET and should_retry: log.warning( - 'Unhandled exception in event consumer. Analytics events were not processed.', - exc_info=True) - except: + 'ProtocolError exception caught while sending events. Retrying.') + self.do_send(output_events, False) + else: log.warning( 'Unhandled exception in event consumer. Analytics events were not processed.', exc_info=True) - - try: - do_send(True) - finally: - for _ in events: - self._queue.task_done() + except: + log.warning( + 'Unhandled exception in event consumer. Analytics events were not processed.', + exc_info=True) def _make_output_event(self, e): kind = e['kind'] @@ -149,36 +251,3 @@ def _make_output_event(self, e): return out else: return e - - def send(self): - events = self.next() - - if len(events) == 0: - return - else: - self.send_batch(events) - - def next(self): - q = self._queue - items = [] - - item = self.next_item() - if item is None: - return items - - items.append(item) - while len(items) < self._config.events_upload_max_batch_size and not q.empty(): - item = self.next_item() - if item: - items.append(item) - - return items - - def next_item(self): - q = self._queue - # noinspection PyBroadException - try: - item = q.get(block=True, timeout=5) - return item - except Exception: - return None diff --git a/ldclient/event_summarizer.py b/ldclient/event_summarizer.py index 8320b9c2..ce7a4c87 100644 --- a/ldclient/event_summarizer.py +++ b/ldclient/event_summarizer.py @@ -80,7 +80,7 @@ def output(self, snapshot_data): counter['version'] = version flag_data['counters'].append(counter) return { - 'start_date': snapshot_data['start_date'], - 'end_date': snapshot_data['end_date'], + 'startDate': snapshot_data['start_date'], + 'endDate': snapshot_data['end_date'], 'features': flags_out } diff --git a/testing/test_event_processor.py b/testing/test_event_processor.py new file mode 100644 index 00000000..761cc5ae --- /dev/null +++ b/testing/test_event_processor.py @@ -0,0 +1,356 @@ +from email.utils import formatdate +import json +import pytest +from requests.structures import CaseInsensitiveDict +import time + +from ldclient.config import Config +from ldclient.event_processor import DefaultEventProcessor + + +default_config = Config() +user = { + 'key': 'userkey', + 'name': 'Red' +} +filtered_user = { + 'key': 'userkey', + 'privateAttrs': [ 'name' ] +} + +ep = None +mock_session = None + + +class MockResponse(object): + def __init__(self, status, headers): + self._status = status + self._headers = headers + + @property + def status_code(self): + return self._status + + @property + def headers(self): + return self._headers + + def raise_for_status(self): + pass + +class MockSession(object): + def __init__(self): + self._request_data = None + self._request_headers = None + self._server_time = None + + def post(self, uri, headers, timeout, data): + self._request_headers = headers + self._request_data = data + resp_hdr = CaseInsensitiveDict() + if self._server_time is not None: + resp_hdr['Date'] = formatdate(self._server_time / 1000, localtime=False, usegmt=True) + return MockResponse(200, resp_hdr) + + def close(self): + pass + + @property + def request_data(self): + return self._request_data + + @property + def request_headers(self): + return self._request_headers + + def set_server_time(self, timestamp): + self._server_time = timestamp + + +def setup_function(): + global mock_session + mock_session = MockSession() + +def teardown_function(): + if ep is not None: + ep.stop() + +def setup_processor(config): + global ep + ep = DefaultEventProcessor(config, mock_session) + ep.start() + + +def test_identify_event_is_queued(): + setup_processor(Config()) + + e = { 'kind': 'identify', 'user': user } + ep.send_event(e) + + output = flush_and_get_events() + assert len(output) == 1 + assert output == [{ + 'kind': 'identify', + 'creationDate': e['creationDate'], + 'user': user + }] + +def test_user_is_filtered_in_identify_event(): + setup_processor(Config(all_attributes_private = True)) + + e = { 'kind': 'identify', 'user': user } + ep.send_event(e) + + output = flush_and_get_events() + assert len(output) == 1 + assert output == [{ + 'kind': 'identify', + 'creationDate': e['creationDate'], + 'user': filtered_user + }] + +def test_individual_feature_event_is_queued_with_index_event(): + setup_processor(Config()) + + e = { + 'kind': 'feature', 'key': 'flagkey', 'version': 11, 'user': user, + 'variation': 1, 'value': 'value', 'default': 'default', 'trackEvents': True + } + ep.send_event(e) + + output = flush_and_get_events() + assert len(output) == 3 + check_index_event(output[0], e) + check_feature_event(output[1], e, False, None) + check_summary_event(output[2]) + +def test_feature_event_can_contain_inline_user(): + setup_processor(Config(inline_users_in_events = True)) + + e = { + 'kind': 'feature', 'key': 'flagkey', 'version': 11, 'user': user, + 'variation': 1, 'value': 'value', 'default': 'default', 'trackEvents': True + } + ep.send_event(e) + + output = flush_and_get_events() + assert len(output) == 2 + check_feature_event(output[0], e, False, user) + check_summary_event(output[1]) + +def test_user_is_filtered_in_feature_event(): + setup_processor(Config(inline_users_in_events = True, all_attributes_private = True)) + + e = { + 'kind': 'feature', 'key': 'flagkey', 'version': 11, 'user': user, + 'variation': 1, 'value': 'value', 'default': 'default', 'trackEvents': True + } + ep.send_event(e) + + output = flush_and_get_events() + assert len(output) == 2 + check_feature_event(output[0], e, False, filtered_user) + check_summary_event(output[1]) + +def test_event_kind_is_debug_if_flag_is_temporarily_in_debug_mode(): + setup_processor(Config()) + + future_time = now() + 100000 + e = { + 'kind': 'feature', 'key': 'flagkey', 'version': 11, 'user': user, + 'variation': 1, 'value': 'value', 'default': 'default', + 'trackEvents': False, 'debugEventsUntilDate': future_time + } + ep.send_event(e) + + output = flush_and_get_events() + assert len(output) == 3 + check_index_event(output[0], e) + check_feature_event(output[1], e, True, None) + check_summary_event(output[2]) + +def test_debug_mode_expires_based_on_client_time_if_client_time_is_later_than_server_time(): + setup_processor(Config()) + + # Pick a server time that is somewhat behind the client time + server_time = now() - 20000 + + # Send and flush an event we don't care about, just to set the last server time + mock_session.set_server_time(server_time) + ep.send_event({ 'kind': 'identify', 'user': { 'key': 'otherUser' }}) + flush_and_get_events() + + # Now send an event with debug mode on, with a "debug until" time that is further in + # the future than the server time, but in the past compared to the client. + debug_until = server_time + 1000 + e = { + 'kind': 'feature', 'key': 'flagkey', 'version': 11, 'user': user, + 'variation': 1, 'value': 'value', 'default': 'default', + 'trackEvents': False, 'debugEventsUntilDate': debug_until + } + ep.send_event(e) + + # Should get a summary event only, not a full feature event + output = flush_and_get_events() + assert len(output) == 2 + check_index_event(output[0], e) + check_summary_event(output[1]) + +def test_debug_mode_expires_based_on_server_time_if_server_time_is_later_than_client_time(): + setup_processor(Config()) + + # Pick a server time that is somewhat ahead of the client time + server_time = now() + 20000 + + # Send and flush an event we don't care about, just to set the last server time + mock_session.set_server_time(server_time) + ep.send_event({ 'kind': 'identify', 'user': { 'key': 'otherUser' }}) + flush_and_get_events() + + # Now send an event with debug mode on, with a "debug until" time that is further in + # the future than the client time, but in the past compared to the server. + debug_until = server_time - 1000 + e = { + 'kind': 'feature', 'key': 'flagkey', 'version': 11, 'user': user, + 'variation': 1, 'value': 'value', 'default': 'default', + 'trackEvents': False, 'debugEventsUntilDate': debug_until + } + ep.send_event(e) + + # Should get a summary event only, not a full feature event + output = flush_and_get_events() + assert len(output) == 2 + check_index_event(output[0], e) + check_summary_event(output[1]) + +def test_two_feature_events_for_same_user_generate_only_one_index_event(): + setup_processor(Config()) + + e1 = { + 'kind': 'feature', 'key': 'flagkey', 'version': 11, 'user': user, + 'variation': 1, 'value': 'value1', 'default': 'default', 'trackEvents': False + } + e2 = { + 'kind': 'feature', 'key': 'flagkey', 'version': 11, 'user': user, + 'variation': 2, 'value': 'value2', 'default': 'default', 'trackEvents': False + } + ep.send_event(e1) + ep.send_event(e2) + + output = flush_and_get_events() + assert len(output) == 2 + check_index_event(output[0], e1) + check_summary_event(output[1]) + +def test_nontracked_events_are_summarized(): + setup_processor(Config()) + + e1 = { + 'kind': 'feature', 'key': 'flagkey1', 'version': 11, 'user': user, + 'variation': 1, 'value': 'value1', 'default': 'default1', 'trackEvents': False + } + e2 = { + 'kind': 'feature', 'key': 'flagkey2', 'version': 22, 'user': user, + 'variation': 2, 'value': 'value2', 'default': 'default2', 'trackEvents': False + } + ep.send_event(e1) + ep.send_event(e2) + + output = flush_and_get_events() + assert len(output) == 2 + check_index_event(output[0], e1) + se = output[1] + assert se['kind'] == 'summary' + assert se['startDate'] == e1['creationDate'] + assert se['endDate'] == e2['creationDate'] + assert se['features'] == { + 'flagkey1': { + 'default': 'default1', + 'counters': [ { 'version': 11, 'value': 'value1', 'count': 1 } ] + }, + 'flagkey2': { + 'default': 'default2', + 'counters': [ { 'version': 22, 'value': 'value2', 'count': 1 } ] + } + } + +def test_custom_event_is_queued_with_user(): + setup_processor(Config()) + + e = { 'kind': 'custom', 'key': 'eventkey', 'user': user, 'data': { 'thing': 'stuff '} } + ep.send_event(e) + + output = flush_and_get_events() + assert len(output) == 2 + check_index_event(output[0], e) + check_custom_event(output[1], e, None) + +def test_custom_event_can_contain_inline_user(): + setup_processor(Config(inline_users_in_events = True)) + + e = { 'kind': 'custom', 'key': 'eventkey', 'user': user, 'data': { 'thing': 'stuff '} } + ep.send_event(e) + + output = flush_and_get_events() + assert len(output) == 1 + check_custom_event(output[0], e, user) + +def test_user_is_filtered_in_custom_event(): + setup_processor(Config(inline_users_in_events = True, all_attributes_private = True)) + + e = { 'kind': 'custom', 'key': 'eventkey', 'user': user, 'data': { 'thing': 'stuff '} } + ep.send_event(e) + + output = flush_and_get_events() + assert len(output) == 1 + check_custom_event(output[0], e, filtered_user) + +def test_nothing_is_sent_if_there_are_no_events(): + setup_processor(Config()) + ep.flush() + assert mock_session.request_data is None + +def test_sdk_key_is_sent(): + setup_processor(Config(sdk_key = 'SDK_KEY')) + + ep.send_event({ 'kind': 'identify', 'user': user }) + ep.flush() + + assert mock_session.request_headers.get('Authorization') is 'SDK_KEY' + + +def flush_and_get_events(): + ep.flush() + return json.loads(mock_session.request_data) + +def check_index_event(data, source): + assert data['kind'] == 'index' + assert data['creationDate'] == source['creationDate'] + assert data['user'] == source['user'] + +def check_feature_event(data, source, debug, inline_user): + assert data['kind'] == ('debug' if debug else 'feature') + assert data['creationDate'] == source['creationDate'] + assert data['key'] == source['key'] + assert data.get('version') == source.get('version') + assert data.get('value') == source.get('value') + assert data.get('default') == source.get('default') + if inline_user is None: + assert data['userKey'] == source['user']['key'] + else: + assert data['user'] == inline_user + +def check_custom_event(data, source, inline_user): + assert data['kind'] == 'custom' + assert data['key'] == source['key'] + assert data['data'] == source['data'] + if inline_user is None: + assert data['userKey'] == source['user']['key'] + else: + assert data['user'] == inline_user + +def check_summary_event(data): + assert data['kind'] == 'summary' + +def now(): + return int(time.time() * 1000) diff --git a/testing/test_event_summarizer.py b/testing/test_event_summarizer.py index 04c339a3..e8db480d 100644 --- a/testing/test_event_summarizer.py +++ b/testing/test_event_summarizer.py @@ -54,8 +54,8 @@ def test_summarize_event_sets_start_and_end_dates(): es.summarize_event(event3) data = es.output(es.snapshot()) - assert data['start_date'] == 1000 - assert data['end_date'] == 2000 + assert data['startDate'] == 1000 + assert data['endDate'] == 2000 def test_summarize_event_increments_counters(): es = EventSummarizer(Config()) @@ -78,8 +78,8 @@ def test_summarize_event_increments_counters(): data['features']['flag1']['counters'].sort(key = lambda c: c['value']) expected = { - 'start_date': 1000, - 'end_date': 1000, + 'startDate': 1000, + 'endDate': 1000, 'features': { 'flag1': { 'default': 'default1', From ca7baf6dcb5d606e5f2d303a820a5d48c70d5901 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Wed, 21 Mar 2018 17:41:30 -0700 Subject: [PATCH 11/48] rm debugging --- ldclient/event_processor.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/ldclient/event_processor.py b/ldclient/event_processor.py index 6e89c103..890967fa 100644 --- a/ldclient/event_processor.py +++ b/ldclient/event_processor.py @@ -66,12 +66,10 @@ def run(self): while self._running: try: self._process_next() - log.error("*** processed ***") except Exception: log.error('Unhandled exception in event consumer', exc_info=True) def stop(self): - log.error("*** GOT STOP ***") self.flush() self._session.close() self._running = False From e0ff3295f4e28100d47e98af1ba5da3fb42c27d0 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Wed, 21 Mar 2018 17:55:35 -0700 Subject: [PATCH 12/48] add flush_interval to Config, remove obsolete property --- ldclient/config.py | 16 +++++++++------- ldclient/event_processor.py | 2 +- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/ldclient/config.py b/ldclient/config.py index 12e965f5..b730fb09 100644 --- a/ldclient/config.py +++ b/ldclient/config.py @@ -13,8 +13,8 @@ def __init__(self, events_uri='https://events.launchdarkly.com', connect_timeout=10, read_timeout=15, - events_upload_max_batch_size=100, events_max_pending=10000, + flush_interval=5, stream_uri='https://stream.launchdarkly.com', stream=True, verify_ssl=True, @@ -46,6 +46,8 @@ def __init__(self, :param int events_max_pending: The capacity of the events buffer. The client buffers up to this many events in memory before flushing. If the capacity is exceeded before the buffer is flushed, events will be discarded. + : param float flush_interval: The number of seconds in between flushes of the events buffer. Decreasing + the flush interval means that the event buffer is less likely to reach capacity. :param string stream_uri: The URL for the LaunchDarkly streaming events server. Most users should use the default value. :param bool stream: Whether or not the streaming API should be used to receive flag updates. By @@ -100,8 +102,8 @@ def __init__(self, self.__feature_requester_class = feature_requester_class self.__connect_timeout = connect_timeout self.__read_timeout = read_timeout - self.__events_upload_max_batch_size = events_upload_max_batch_size self.__events_max_pending = events_max_pending + self.__flush_interval = flush_interval self.__verify_ssl = verify_ssl self.__defaults = defaults if offline is True: @@ -124,8 +126,8 @@ def copy_with_new_sdk_key(self, new_sdk_key): events_uri=self.__events_uri, connect_timeout=self.__connect_timeout, read_timeout=self.__read_timeout, - events_upload_max_batch_size=self.__events_upload_max_batch_size, events_max_pending=self.__events_max_pending, + flush_interval=self.__flush_interval, stream_uri=self.__stream_uri, stream=self.__stream, verify_ssl=self.__verify_ssl, @@ -215,14 +217,14 @@ def events_enabled(self): def send_events(self): return self.__send_events - @property - def events_upload_max_batch_size(self): - return self.__events_upload_max_batch_size - @property def events_max_pending(self): return self.__events_max_pending + @property + def flush_interval(self): + return self.__flush_interval + @property def verify_ssl(self): return self.__verify_ssl diff --git a/ldclient/event_processor.py b/ldclient/event_processor.py index 890967fa..3b9d7eb6 100644 --- a/ldclient/event_processor.py +++ b/ldclient/event_processor.py @@ -92,7 +92,7 @@ def _now(self): return int(time.time() * 1000) def _set_flush_timer(self): - self._flush_timer = Timer(5, self._flush_async) + self._flush_timer = Timer(self._config.flush_interval, self._flush_async) self._flush_timer.start() def _set_users_flush_timer(self): From 3708ef27c367679531df78500320c897e8bcff58 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Wed, 21 Mar 2018 17:59:13 -0700 Subject: [PATCH 13/48] consumer->processor --- ldclient/event_processor.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ldclient/event_processor.py b/ldclient/event_processor.py index 3b9d7eb6..dd129886 100644 --- a/ldclient/event_processor.py +++ b/ldclient/event_processor.py @@ -61,7 +61,7 @@ def __init__(self, config, session=None): self._set_users_flush_timer() def run(self): - log.info("Starting event consumer") + log.info("Starting event processor") self._running = True while self._running: try: From 3b07e8767a394a6ca93c3f60d96e52bbd4c2c225 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Wed, 21 Mar 2018 17:59:36 -0700 Subject: [PATCH 14/48] consumer->processor --- ldclient/event_processor.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ldclient/event_processor.py b/ldclient/event_processor.py index dd129886..8b547de0 100644 --- a/ldclient/event_processor.py +++ b/ldclient/event_processor.py @@ -67,7 +67,7 @@ def run(self): try: self._process_next() except Exception: - log.error('Unhandled exception in event consumer', exc_info=True) + log.error('Unhandled exception in event processor', exc_info=True) def stop(self): self.flush() @@ -205,11 +205,11 @@ def _do_send(self, output_events, should_retry): self.do_send(output_events, False) else: log.warning( - 'Unhandled exception in event consumer. Analytics events were not processed.', + 'Unhandled exception in event processor. Analytics events were not processed.', exc_info=True) except: log.warning( - 'Unhandled exception in event consumer. Analytics events were not processed.', + 'Unhandled exception in event processor. Analytics events were not processed.', exc_info=True) def _make_output_event(self, e): From 22319786b06758a64c555c87790e86b30dce14bb Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Fri, 23 Mar 2018 16:58:56 -0700 Subject: [PATCH 15/48] use timer thread objects to avoid race condition --- ldclient/event_processor.py | 21 +++++++-------------- ldclient/repeating_timer.py | 15 +++++++++++++++ 2 files changed, 22 insertions(+), 14 deletions(-) create mode 100644 ldclient/repeating_timer.py diff --git a/ldclient/event_processor.py b/ldclient/event_processor.py index 8b547de0..e00ffaa7 100644 --- a/ldclient/event_processor.py +++ b/ldclient/event_processor.py @@ -21,6 +21,7 @@ from ldclient.event_summarizer import EventSummarizer from ldclient.user_filter import UserFilter from ldclient.interfaces import EventProcessor +from ldclient.repeating_timer import RepeatingTimer from ldclient.util import _headers from ldclient.util import log @@ -57,8 +58,10 @@ def __init__(self, config, session=None): self._summarizer = EventSummarizer(config) self._last_known_past_time = 0 self._running = True - self._set_flush_timer() - self._set_users_flush_timer() + self._flush_timer = RepeatingTimer(self._config.flush_interval, self._flush_async) + self._flush_timer.start() + self._users_flush_timer = RepeatingTimer(self._config.user_keys_flush_interval, self._flush_users) + self._users_flush_timer.start() def run(self): log.info("Starting event processor") @@ -73,8 +76,8 @@ def stop(self): self.flush() self._session.close() self._running = False - self._flush_timer.cancel() - self._users_flush_timer.cancel() + self._flush_timer.stop() + self._users_flush_timer.stop() # Post a non-message so we won't keep blocking on the queue self._queue.put(('stop', None)) @@ -91,21 +94,11 @@ def flush(self): def _now(self): return int(time.time() * 1000) - def _set_flush_timer(self): - self._flush_timer = Timer(self._config.flush_interval, self._flush_async) - self._flush_timer.start() - - def _set_users_flush_timer(self): - self._users_flush_timer = Timer(self._config.user_keys_flush_interval, self._flush_users) - self._users_flush_timer.start() - def _flush_async(self): self._queue.put(('flush', None)) - self._set_flush_timer() def _flush_users(self): self._queue.put(('flush_users', None)) - self._set_users_flush_timer() def _process_next(self): item = self._queue.get(block=True) diff --git a/ldclient/repeating_timer.py b/ldclient/repeating_timer.py new file mode 100644 index 00000000..efe20e91 --- /dev/null +++ b/ldclient/repeating_timer.py @@ -0,0 +1,15 @@ +from threading import Event, Thread + +class RepeatingTimer(Thread): + def __init__(self, interval, callable): + Thread.__init__(self) + self._interval = interval + self._action = callable + self._stop = Event() + + def run(self): + while not self._stop.wait(self._interval): + self._action() + + def stop(self): + self._stop.set() From 971d7fe8453b2a2bb8e43d78f063b7a145d7b33a Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Fri, 23 Mar 2018 17:00:03 -0700 Subject: [PATCH 16/48] rm spurious underscore --- ldclient/client.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ldclient/client.py b/ldclient/client.py index b6801199..6ab8451c 100644 --- a/ldclient/client.py +++ b/ldclient/client.py @@ -71,19 +71,19 @@ def __init__(self, sdk_key=None, config=None, start_wait=5): self._config, self._store, update_processor_ready) else: if self._config.feature_requester_class: - _feature_requester = self._config.feature_requester_class(self._config) + feature_requester = self._config.feature_requester_class(self._config) else: - _feature_requester = FeatureRequesterImpl(self._config) + feature_requester = FeatureRequesterImpl(self._config) """ :type: FeatureRequester """ if self._config.stream: self._update_processor = StreamingUpdateProcessor( - self._config, _feature_requester, self._store, update_processor_ready) + self._config, feature_requester, self._store, update_processor_ready) else: log.info("Disabling streaming API") log.warn("You should only disable the streaming API if instructed to do so by LaunchDarkly support") self._update_processor = PollingUpdateProcessor( - self._config, _feature_requester, self._store, update_processor_ready) + self._config, feature_requester, self._store, update_processor_ready) """ :type: UpdateProcessor """ self._update_processor.start() From 8a45a53ec848302ad2b4dfe5dc2afb0620c4a4c5 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Fri, 23 Mar 2018 17:04:44 -0700 Subject: [PATCH 17/48] use namedtuple --- ldclient/event_processor.py | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/ldclient/event_processor.py b/ldclient/event_processor.py index e00ffaa7..ebf28749 100644 --- a/ldclient/event_processor.py +++ b/ldclient/event_processor.py @@ -1,9 +1,10 @@ from __future__ import absolute_import +from collections import namedtuple from email.utils import parsedate import errno import jsonpickle -from threading import Event, Thread, Timer +from threading import Event, Thread import time # noinspection PyBroadException @@ -46,6 +47,9 @@ def flush(self): pass +EventProcessorMessage = namedtuple('EventProcessorMessage', ['type', 'param']) + + class DefaultEventProcessor(Thread, EventProcessor): def __init__(self, config, session=None): Thread.__init__(self) @@ -79,34 +83,34 @@ def stop(self): self._flush_timer.stop() self._users_flush_timer.stop() # Post a non-message so we won't keep blocking on the queue - self._queue.put(('stop', None)) + self._queue.put(EventProcessorMessage('stop', None)) def send_event(self, event): event['creationDate'] = self._now() - self._queue.put(('event', event)) + self._queue.put(EventProcessorMessage('event', event)) def flush(self): # Put a flush message on the queue and wait until it's been processed. reply = Event() - self._queue.put(('flush', reply)) + self._queue.put(EventProcessorMessage('flush', reply)) reply.wait() def _now(self): return int(time.time() * 1000) def _flush_async(self): - self._queue.put(('flush', None)) + self._queue.put(EventProcessorMessage('flush', None)) def _flush_users(self): - self._queue.put(('flush_users', None)) + self._queue.put(EventProcessorMessage('flush_users', None)) def _process_next(self): - item = self._queue.get(block=True) - if item[0] == 'event': - self._process_event(item[1]) - elif item[0] == 'flush': - self._dispatch_flush(item[1]) - elif item[0] == 'flush_users': + message = self._queue.get(block=True) + if message.type == 'event': + self._process_event(message.param) + elif message.type == 'flush': + self._dispatch_flush(message.param) + elif message.type == 'flush_users': self._summarizer.reset_users() def _process_event(self, event): From d0284eda33a895ebbce6073053e2aedb8477c41a Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Sat, 24 Mar 2018 14:02:33 -0700 Subject: [PATCH 18/48] make timer threads daemons --- ldclient/repeating_timer.py | 1 + 1 file changed, 1 insertion(+) diff --git a/ldclient/repeating_timer.py b/ldclient/repeating_timer.py index efe20e91..a1e393ea 100644 --- a/ldclient/repeating_timer.py +++ b/ldclient/repeating_timer.py @@ -3,6 +3,7 @@ class RepeatingTimer(Thread): def __init__(self, interval, callable): Thread.__init__(self) + self.daemon = True self._interval = interval self._action = callable self._stop = Event() From 9b3632f92659ab74e0541e2012da057abff83b81 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Sat, 24 Mar 2018 14:02:45 -0700 Subject: [PATCH 19/48] break up DefaultEventProcessor a bit and don't inherit from Thread --- ldclient/event_processor.py | 99 ++++++++++++++++++++++--------------- 1 file changed, 60 insertions(+), 39 deletions(-) diff --git a/ldclient/event_processor.py b/ldclient/event_processor.py index ebf28749..f261a00b 100644 --- a/ldclient/event_processor.py +++ b/ldclient/event_processor.py @@ -27,7 +27,7 @@ from ldclient.util import log -class NullEventProcessor(Thread, EventProcessor): +class NullEventProcessor(EventProcessor): def __init(self, config): pass @@ -50,59 +50,42 @@ def flush(self): EventProcessorMessage = namedtuple('EventProcessorMessage', ['type', 'param']) -class DefaultEventProcessor(Thread, EventProcessor): - def __init__(self, config, session=None): - Thread.__init__(self) - self._session = requests.Session() if session is None else session - self.daemon = True +class EventConsumer(object): + def __init__(self, queue, config, session): + self._queue = queue self._config = config - self._queue = queue.Queue(config.events_max_pending) + self._session = requests.Session() if session is None else session + self._main_thread = Thread(target=self._run_main_loop) + self._main_thread.daemon = True + self._running = False self._events = [] self._user_filter = UserFilter(config) self._summarizer = EventSummarizer(config) self._last_known_past_time = 0 - self._running = True - self._flush_timer = RepeatingTimer(self._config.flush_interval, self._flush_async) - self._flush_timer.start() - self._users_flush_timer = RepeatingTimer(self._config.user_keys_flush_interval, self._flush_users) - self._users_flush_timer.start() - def run(self): - log.info("Starting event processor") - self._running = True - while self._running: - try: - self._process_next() - except Exception: - log.error('Unhandled exception in event processor', exc_info=True) + def start(self): + self._main_thread.start() def stop(self): - self.flush() self._session.close() self._running = False - self._flush_timer.stop() - self._users_flush_timer.stop() # Post a non-message so we won't keep blocking on the queue self._queue.put(EventProcessorMessage('stop', None)) - def send_event(self, event): - event['creationDate'] = self._now() - self._queue.put(EventProcessorMessage('event', event)) - - def flush(self): - # Put a flush message on the queue and wait until it's been processed. - reply = Event() - self._queue.put(EventProcessorMessage('flush', reply)) - reply.wait() + def is_alive(self): + return self._main_thread.is_alive() - def _now(self): + def now(self): return int(time.time() * 1000) - def _flush_async(self): - self._queue.put(EventProcessorMessage('flush', None)) - - def _flush_users(self): - self._queue.put(EventProcessorMessage('flush_users', None)) + def _run_main_loop(self): + log.info("Starting event processor") + self._running = True + while self._running: + try: + self._process_next() + except Exception: + log.error('Unhandled exception in event processor', exc_info=True) def _process_next(self): message = self._queue.get(block=True) @@ -143,7 +126,7 @@ def _should_track_full_event(self, event): debug_until = event.get('debugEventsUntilDate') if debug_until is not None: last_past = self._last_known_past_time - if debug_until > last_past and debug_until > self._now(): + if debug_until > last_past and debug_until > self.now(): return True return False else: @@ -246,3 +229,41 @@ def _make_output_event(self, e): return out else: return e + + +class DefaultEventProcessor(EventProcessor): + def __init__(self, config, session=None): + self._queue = queue.Queue(config.events_max_pending) + self._consumer = EventConsumer(self._queue, config, session) + self._flush_timer = RepeatingTimer(config.flush_interval, self._flush_async) + self._users_flush_timer = RepeatingTimer(config.user_keys_flush_interval, self._flush_users) + + def start(self): + self._consumer.start() + self._flush_timer.start() + self._users_flush_timer.start() + + def stop(self): + self._flush_timer.stop() + self._users_flush_timer.stop() + self.flush() + self._consumer.stop() + + def is_alive(self): + return self._consumer.is_alive() + + def send_event(self, event): + event['creationDate'] = self._consumer.now() + self._queue.put(EventProcessorMessage('event', event)) + + def flush(self): + # Put a flush message on the queue and wait until it's been processed. + reply = Event() + self._queue.put(EventProcessorMessage('flush', reply)) + reply.wait() + + def _flush_async(self): + self._queue.put(EventProcessorMessage('flush', None)) + + def _flush_users(self): + self._queue.put(EventProcessorMessage('flush_users', None)) From c73e0b99b40d932da3b38d401cf2a121783bb673 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Sat, 24 Mar 2018 14:08:19 -0700 Subject: [PATCH 20/48] break out the output event generation logic --- ldclient/event_processor.py | 86 ++++++++++++++++++++----------------- 1 file changed, 46 insertions(+), 40 deletions(-) diff --git a/ldclient/event_processor.py b/ldclient/event_processor.py index f261a00b..ab25a6d9 100644 --- a/ldclient/event_processor.py +++ b/ldclient/event_processor.py @@ -50,6 +50,50 @@ def flush(self): EventProcessorMessage = namedtuple('EventProcessorMessage', ['type', 'param']) +class EventOutputTransformer(object): + def __init__(self, config): + self._config = config + self._user_filter = UserFilter(config) + + def make_output_event(self, e): + kind = e['kind'] + if kind == 'feature': + is_debug = (not e['trackEvents']) and (e.get('debugEventsUntilDate') is not None) + out = { + 'kind': 'debug' if is_debug else 'feature', + 'creationDate': e['creationDate'], + 'key': e['key'], + 'version': e.get('version'), + 'value': e.get('value'), + 'default': e.get('default'), + 'prereqOf': e.get('prereqOf') + } + if self._config.inline_users_in_events: + out['user'] = self._user_filter.filter_user_props(e['user']) + else: + out['userKey'] = e['user'].get('key') + return out + elif kind == 'identify': + return { + 'kind': 'identify', + 'creationDate': e['creationDate'], + 'user': self._user_filter.filter_user_props(e['user']) + } + elif kind == 'custom': + out = { + 'kind': 'custom', + 'key': e['key'], + 'data': e.get('data') + } + if self._config.inline_users_in_events: + out['user'] = self._user_filter.filter_user_props(e['user']) + else: + out['userKey'] = e['user'].get('key') + return out + else: + return e + + class EventConsumer(object): def __init__(self, queue, config, session): self._queue = queue @@ -59,8 +103,8 @@ def __init__(self, queue, config, session): self._main_thread.daemon = True self._running = False self._events = [] - self._user_filter = UserFilter(config) self._summarizer = EventSummarizer(config) + self._output_transformer = EventOutputTransformer(config) self._last_known_past_time = 0 def start(self): @@ -144,7 +188,7 @@ def _dispatch_flush(self, reply): reply.set() def _flush_task(self, events, snapshot, reply): - output_events = [ self._make_output_event(e) for e in events ] + output_events = [ self._output_transformer.make_output_event(e) for e in events ] if len(snapshot['counters']) > 0: summary = self._summarizer.output(snapshot) summary['kind'] = 'summary' @@ -192,44 +236,6 @@ def _do_send(self, output_events, should_retry): 'Unhandled exception in event processor. Analytics events were not processed.', exc_info=True) - def _make_output_event(self, e): - kind = e['kind'] - if kind == 'feature': - is_debug = (not e['trackEvents']) and (e.get('debugEventsUntilDate') is not None) - out = { - 'kind': 'debug' if is_debug else 'feature', - 'creationDate': e['creationDate'], - 'key': e['key'], - 'version': e.get('version'), - 'value': e.get('value'), - 'default': e.get('default'), - 'prereqOf': e.get('prereqOf') - } - if self._config.inline_users_in_events: - out['user'] = self._user_filter.filter_user_props(e['user']) - else: - out['userKey'] = e['user'].get('key') - return out - elif kind == 'identify': - return { - 'kind': 'identify', - 'creationDate': e['creationDate'], - 'user': self._user_filter.filter_user_props(e['user']) - } - elif kind == 'custom': - out = { - 'kind': 'custom', - 'key': e['key'], - 'data': e.get('data') - } - if self._config.inline_users_in_events: - out['user'] = self._user_filter.filter_user_props(e['user']) - else: - out['userKey'] = e['user'].get('key') - return out - else: - return e - class DefaultEventProcessor(EventProcessor): def __init__(self, config, session=None): From 5a32403263e97311e8ec50c1afae92412af6a436 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Sat, 24 Mar 2018 14:39:16 -0700 Subject: [PATCH 21/48] break up the event processor code some more --- ldclient/event_processor.py | 158 ++++++++++++++++++++----------- ldclient/event_summarizer.py | 38 ++------ testing/test_event_summarizer.py | 38 ++------ 3 files changed, 118 insertions(+), 116 deletions(-) diff --git a/ldclient/event_processor.py b/ldclient/event_processor.py index ab25a6d9..1e28d4c2 100644 --- a/ldclient/event_processor.py +++ b/ldclient/event_processor.py @@ -55,6 +55,9 @@ def __init__(self, config): self._config = config self._user_filter = UserFilter(config) + """ + Transform an event into the format used for the event payload. + """ def make_output_event(self, e): kind = e['kind'] if kind == 'feature': @@ -93,6 +96,93 @@ def make_output_event(self, e): else: return e + """ + Transform summarizer data into the format used for the event payload. + """ + def make_summary_event(self, summary): + flags_out = dict() + for ckey, cval in summary.counters.items(): + flag_key, variation, version = ckey + flag_data = flags_out.get(flag_key) + if flag_data is None: + flag_data = { 'default': cval['default'], 'counters': [] } + flags_out[flag_key] = flag_data + counter = { + 'count': cval['count'], + 'value': cval['value'] + } + if version is None: + counter['unknown'] = True + else: + counter['version'] = version + flag_data['counters'].append(counter) + return { + 'kind': 'summary', + 'startDate': summary.start_date, + 'endDate': summary.end_date, + 'features': flags_out + } + + +class EventPayloadSendTask(object): + def __init__(self, session, config, events, summary, response_listener, reply_event): + self._session = session + self._config = config + self._events = events + self._summary = summary + self._response_listener = response_listener + self._reply_event = reply_event + + def start(self): + if len(self._events) > 0 or len(self._summary.counters) > 0: + Thread(target = self._run).start() + else: + self._completed() + + def _completed(self): + if self._reply_event is not None: + self._reply_event.set() + + def _run(self): + transformer = EventOutputTransformer(self._config) + output_events = [ transformer.make_output_event(e) for e in self._events ] + if len(self._summary.counters) > 0: + output_events.append(transformer.make_summary_event(self._summary)) + try: + self._do_send(output_events, True) + finally: + self._completed() + + def _do_send(self, output_events, should_retry): + # noinspection PyBroadException + try: + json_body = jsonpickle.encode(output_events, unpicklable=False) + log.debug('Sending events payload: ' + json_body) + hdrs = _headers(self._config.sdk_key) + uri = self._config.events_uri + r = self._session.post(uri, + headers=hdrs, + timeout=(self._config.connect_timeout, self._config.read_timeout), + data=json_body) + if self._response_listener is not None: + self._response_listener(r) + r.raise_for_status() + except ProtocolError as e: + if e.args is not None and len(e.args) > 1 and e.args[1] is not None: + inner = e.args[1] + if inner.errno is not None and inner.errno == errno.ECONNRESET and should_retry: + log.warning( + 'ProtocolError exception caught while sending events. Retrying.') + self.do_send(output_events, False) + else: + log.warning( + 'Unhandled exception in event processor. Analytics events were not processed.', + exc_info=True) + except: + log.warning( + 'Unhandled exception in event processor. Analytics events were not processed.', + exc_info=True) + class EventConsumer(object): def __init__(self, queue, config, session): @@ -180,61 +270,19 @@ def _dispatch_flush(self, reply): events = self._events self._events = [] snapshot = self._summarizer.snapshot() - if len(events) > 0 or len(snapshot['counters']) > 0: - flusher = Thread(target = self._flush_task, args=(events, snapshot, reply)) - flusher.start() - else: - if reply is not None: - reply.set() - - def _flush_task(self, events, snapshot, reply): - output_events = [ self._output_transformer.make_output_event(e) for e in events ] - if len(snapshot['counters']) > 0: - summary = self._summarizer.output(snapshot) - summary['kind'] = 'summary' - output_events.append(summary) - try: - self._do_send(output_events, True) - finally: - if reply is not None: - reply.set() - - def _do_send(self, output_events, should_retry): - # noinspection PyBroadException - try: - json_body = jsonpickle.encode(output_events, unpicklable=False) - log.debug('Sending events payload: ' + json_body) - hdrs = _headers(self._config.sdk_key) - uri = self._config.events_uri - r = self._session.post(uri, - headers=hdrs, - timeout=(self._config.connect_timeout, self._config.read_timeout), - data=json_body) - server_date_str = r.headers.get('Date') - if server_date_str is not None: - server_date = parsedate(server_date_str) - if server_date is not None: - self._last_known_past_time = server_date - if r.status_code == 401: - log.error('Received 401 error, no further events will be posted since SDK key is invalid') - self.stop() - return - r.raise_for_status() - except ProtocolError as e: - if e.args is not None and len(e.args) > 1 and e.args[1] is not None: - inner = e.args[1] - if inner.errno is not None and inner.errno == errno.ECONNRESET and should_retry: - log.warning( - 'ProtocolError exception caught while sending events. Retrying.') - self.do_send(output_events, False) - else: - log.warning( - 'Unhandled exception in event processor. Analytics events were not processed.', - exc_info=True) - except: - log.warning( - 'Unhandled exception in event processor. Analytics events were not processed.', - exc_info=True) + task = EventPayloadSendTask(self._session, self._config, events, snapshot, self._handle_response, reply) + task.start() + + def _handle_response(self, r): + server_date_str = r.headers.get('Date') + if server_date_str is not None: + server_date = parsedate(server_date_str) + if server_date is not None: + self._last_known_past_time = server_date + if r.status_code == 401: + log.error('Received 401 error, no further events will be posted since SDK key is invalid') + self.stop() + return class DefaultEventProcessor(EventProcessor): diff --git a/ldclient/event_summarizer.py b/ldclient/event_summarizer.py index ce7a4c87..5032a13d 100644 --- a/ldclient/event_summarizer.py +++ b/ldclient/event_summarizer.py @@ -1,5 +1,10 @@ +from collections import namedtuple import pylru + +EventSummary = namedtuple('EventSummary', ['start_date', 'end_date', 'counters']) + + class EventSummarizer(object): def __init__(self, config): self.user_keys = pylru.lrucache(config.user_keys_capacity) @@ -48,39 +53,8 @@ def summarize_event(self, event): Return a snapshot of the current summarized event data, and reset this state. """ def snapshot(self): - ret = { - 'start_date': self.start_date, - 'end_date': self.end_date, - 'counters': self.counters - } + ret = EventSummary(start_date = self.start_date, end_date = self.end_date, counters = self.counters) self.start_date = 0 self.end_date = 0 self.counters = dict() return ret - - """ - Transform the summary data into the format used for event sending. - """ - def output(self, snapshot_data): - counters = snapshot_data['counters'] - flags_out = dict() - for ckey, cval in counters.items(): - flag_key, variation, version = ckey - flag_data = flags_out.get(flag_key) - if flag_data is None: - flag_data = { 'default': cval['default'], 'counters': [] } - flags_out[flag_key] = flag_data - counter = { - 'count': cval['count'], - 'value': cval['value'] - } - if version is None: - counter['unknown'] = True - else: - counter['version'] = version - flag_data['counters'].append(counter) - return { - 'startDate': snapshot_data['start_date'], - 'endDate': snapshot_data['end_date'], - 'features': flags_out - } diff --git a/testing/test_event_summarizer.py b/testing/test_event_summarizer.py index e8db480d..460b3477 100644 --- a/testing/test_event_summarizer.py +++ b/testing/test_event_summarizer.py @@ -52,10 +52,10 @@ def test_summarize_event_sets_start_and_end_dates(): es.summarize_event(event1) es.summarize_event(event2) es.summarize_event(event3) - data = es.output(es.snapshot()) + data = es.snapshot() - assert data['startDate'] == 1000 - assert data['endDate'] == 2000 + assert data.start_date == 1000 + assert data.end_date == 2000 def test_summarize_event_increments_counters(): es = EventSummarizer(Config()) @@ -74,32 +74,12 @@ def test_summarize_event_increments_counters(): es.summarize_event(event3) es.summarize_event(event4) es.summarize_event(event5) - data = es.output(es.snapshot()) + data = es.snapshot() - data['features']['flag1']['counters'].sort(key = lambda c: c['value']) expected = { - 'startDate': 1000, - 'endDate': 1000, - 'features': { - 'flag1': { - 'default': 'default1', - 'counters': [ - { 'version': 11, 'value': 'value1', 'count': 2 }, - { 'version': 11, 'value': 'value2', 'count': 1 } - ] - }, - 'flag2': { - 'default': 'default2', - 'counters': [ - { 'version': 22, 'value': 'value99', 'count': 1 } - ] - }, - 'badkey': { - 'default': 'default3', - 'counters': [ - { 'unknown': True, 'value': 'default3', 'count': 1} - ] - } - } + ('flag1', 1, 11): { 'count': 2, 'value': 'value1', 'default': 'default1' }, + ('flag1', 2, 11): { 'count': 1, 'value': 'value2', 'default': 'default1' }, + ('flag2', 1, 22): { 'count': 1, 'value': 'value99', 'default': 'default2' }, + ('badkey', None, None): { 'count': 1, 'value': 'default3', 'default': 'default3' } } - assert data == expected + assert data.counters == expected From 3b1b159662b7e09d02f6b18af21fdfc4a6a17ca7 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Sat, 24 Mar 2018 14:42:50 -0700 Subject: [PATCH 22/48] fix user filtering for index event --- ldclient/event_processor.py | 6 ++++++ testing/test_event_processor.py | 33 ++++++++++++++++++++++++--------- 2 files changed, 30 insertions(+), 9 deletions(-) diff --git a/ldclient/event_processor.py b/ldclient/event_processor.py index 1e28d4c2..93c6b476 100644 --- a/ldclient/event_processor.py +++ b/ldclient/event_processor.py @@ -93,6 +93,12 @@ def make_output_event(self, e): else: out['userKey'] = e['user'].get('key') return out + elif kind == 'index': + return { + 'kind': 'index', + 'creationDate': e['creationDate'], + 'user': self._user_filter.filter_user_props(e['user']) + } else: return e diff --git a/testing/test_event_processor.py b/testing/test_event_processor.py index 761cc5ae..5a1cac76 100644 --- a/testing/test_event_processor.py +++ b/testing/test_event_processor.py @@ -120,7 +120,22 @@ def test_individual_feature_event_is_queued_with_index_event(): output = flush_and_get_events() assert len(output) == 3 - check_index_event(output[0], e) + check_index_event(output[0], e, user) + check_feature_event(output[1], e, False, None) + check_summary_event(output[2]) + +def test_user_is_filtered_in_index_event(): + setup_processor(Config(all_attributes_private = True)) + + e = { + 'kind': 'feature', 'key': 'flagkey', 'version': 11, 'user': user, + 'variation': 1, 'value': 'value', 'default': 'default', 'trackEvents': True + } + ep.send_event(e) + + output = flush_and_get_events() + assert len(output) == 3 + check_index_event(output[0], e, filtered_user) check_feature_event(output[1], e, False, None) check_summary_event(output[2]) @@ -165,7 +180,7 @@ def test_event_kind_is_debug_if_flag_is_temporarily_in_debug_mode(): output = flush_and_get_events() assert len(output) == 3 - check_index_event(output[0], e) + check_index_event(output[0], e, user) check_feature_event(output[1], e, True, None) check_summary_event(output[2]) @@ -193,7 +208,7 @@ def test_debug_mode_expires_based_on_client_time_if_client_time_is_later_than_se # Should get a summary event only, not a full feature event output = flush_and_get_events() assert len(output) == 2 - check_index_event(output[0], e) + check_index_event(output[0], e, user) check_summary_event(output[1]) def test_debug_mode_expires_based_on_server_time_if_server_time_is_later_than_client_time(): @@ -220,7 +235,7 @@ def test_debug_mode_expires_based_on_server_time_if_server_time_is_later_than_cl # Should get a summary event only, not a full feature event output = flush_and_get_events() assert len(output) == 2 - check_index_event(output[0], e) + check_index_event(output[0], e, user) check_summary_event(output[1]) def test_two_feature_events_for_same_user_generate_only_one_index_event(): @@ -239,7 +254,7 @@ def test_two_feature_events_for_same_user_generate_only_one_index_event(): output = flush_and_get_events() assert len(output) == 2 - check_index_event(output[0], e1) + check_index_event(output[0], e1, user) check_summary_event(output[1]) def test_nontracked_events_are_summarized(): @@ -258,7 +273,7 @@ def test_nontracked_events_are_summarized(): output = flush_and_get_events() assert len(output) == 2 - check_index_event(output[0], e1) + check_index_event(output[0], e1, user) se = output[1] assert se['kind'] == 'summary' assert se['startDate'] == e1['creationDate'] @@ -282,7 +297,7 @@ def test_custom_event_is_queued_with_user(): output = flush_and_get_events() assert len(output) == 2 - check_index_event(output[0], e) + check_index_event(output[0], e, user) check_custom_event(output[1], e, None) def test_custom_event_can_contain_inline_user(): @@ -323,10 +338,10 @@ def flush_and_get_events(): ep.flush() return json.loads(mock_session.request_data) -def check_index_event(data, source): +def check_index_event(data, source, user): assert data['kind'] == 'index' assert data['creationDate'] == source['creationDate'] - assert data['user'] == source['user'] + assert data['user'] == user def check_feature_event(data, source, debug, inline_user): assert data['kind'] == ('debug' if debug else 'feature') From a745739f90a72f29efaa9a6261814cdf94a91e13 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Sat, 24 Mar 2018 14:43:26 -0700 Subject: [PATCH 23/48] fix creation date in custom event --- ldclient/event_processor.py | 1 + testing/test_event_processor.py | 1 + 2 files changed, 2 insertions(+) diff --git a/ldclient/event_processor.py b/ldclient/event_processor.py index 93c6b476..9ac9289a 100644 --- a/ldclient/event_processor.py +++ b/ldclient/event_processor.py @@ -85,6 +85,7 @@ def make_output_event(self, e): elif kind == 'custom': out = { 'kind': 'custom', + 'creationDate': e['creationDate'], 'key': e['key'], 'data': e.get('data') } diff --git a/testing/test_event_processor.py b/testing/test_event_processor.py index 5a1cac76..f2ec4545 100644 --- a/testing/test_event_processor.py +++ b/testing/test_event_processor.py @@ -357,6 +357,7 @@ def check_feature_event(data, source, debug, inline_user): def check_custom_event(data, source, inline_user): assert data['kind'] == 'custom' + assert data['creationDate'] == source['creationDate'] assert data['key'] == source['key'] assert data['data'] == source['data'] if inline_user is None: From a67c7b32deee0663a3bd79b3781fae04fe14f8ce Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Mon, 26 Mar 2018 11:45:46 -0700 Subject: [PATCH 24/48] don't create a task object unless we're actually going to send some events --- ldclient/event_processor.py | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/ldclient/event_processor.py b/ldclient/event_processor.py index 9ac9289a..080b80c3 100644 --- a/ldclient/event_processor.py +++ b/ldclient/event_processor.py @@ -141,24 +141,18 @@ def __init__(self, session, config, events, summary, response_listener, reply_ev self._reply_event = reply_event def start(self): - if len(self._events) > 0 or len(self._summary.counters) > 0: - Thread(target = self._run).start() - else: - self._completed() - - def _completed(self): - if self._reply_event is not None: - self._reply_event.set() + Thread(target = self._run).start() def _run(self): - transformer = EventOutputTransformer(self._config) - output_events = [ transformer.make_output_event(e) for e in self._events ] - if len(self._summary.counters) > 0: - output_events.append(transformer.make_summary_event(self._summary)) try: - self._do_send(output_events, True) + transformer = EventOutputTransformer(self._config) + output_events = [ transformer.make_output_event(e) for e in self._events ] + if len(self._summary.counters) > 0: + output_events.append(transformer.make_summary_event(self._summary)) + self._do_send(output_events, True) finally: - self._completed() + if self._reply_event is not None: + self._reply_event.set() def _do_send(self, output_events, should_retry): # noinspection PyBroadException @@ -277,8 +271,12 @@ def _dispatch_flush(self, reply): events = self._events self._events = [] snapshot = self._summarizer.snapshot() - task = EventPayloadSendTask(self._session, self._config, events, snapshot, self._handle_response, reply) - task.start() + if len(events) > 0 or len(snapshot.counters) > 0: + task = EventPayloadSendTask(self._session, self._config, events, snapshot, self._handle_response, reply) + task.start() + else: + if reply is not None: + reply.set() def _handle_response(self, r): server_date_str = r.headers.get('Date') From bd147c3955439f49197fe3af6d8349a07737a93b Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Mon, 26 Mar 2018 12:06:46 -0700 Subject: [PATCH 25/48] misc fixes --- ldclient/event_processor.py | 6 +++++- testing/test_event_processor.py | 3 ++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/ldclient/event_processor.py b/ldclient/event_processor.py index 080b80c3..dfb9a819 100644 --- a/ldclient/event_processor.py +++ b/ldclient/event_processor.py @@ -149,7 +149,11 @@ def _run(self): output_events = [ transformer.make_output_event(e) for e in self._events ] if len(self._summary.counters) > 0: output_events.append(transformer.make_summary_event(self._summary)) - self._do_send(output_events, True) + self._do_send(output_events, True) + except: + log.warning( + 'Unhandled exception in event processor. Analytics events were not processed.', + exc_info=True) finally: if self._reply_event is not None: self._reply_event.set() diff --git a/testing/test_event_processor.py b/testing/test_event_processor.py index f2ec4545..46c645a4 100644 --- a/testing/test_event_processor.py +++ b/testing/test_event_processor.py @@ -7,6 +7,7 @@ from ldclient.config import Config from ldclient.event_processor import DefaultEventProcessor +from ldclient.util import log default_config = Config() user = { @@ -336,7 +337,7 @@ def test_sdk_key_is_sent(): def flush_and_get_events(): ep.flush() - return json.loads(mock_session.request_data) + return None if mock_session.request_data is None else json.loads(mock_session.request_data) def check_index_event(data, source, user): assert data['kind'] == 'index' From e8b29986c7192104df9205f380ffbfe3fa585db8 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Mon, 26 Mar 2018 12:10:52 -0700 Subject: [PATCH 26/48] don't use stop() internally; make it safe to call stop() more than once --- ldclient/event_processor.py | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/ldclient/event_processor.py b/ldclient/event_processor.py index dfb9a819..0ca69ce0 100644 --- a/ldclient/event_processor.py +++ b/ldclient/event_processor.py @@ -197,6 +197,7 @@ def __init__(self, queue, config, session): self._main_thread = Thread(target=self._run_main_loop) self._main_thread.daemon = True self._running = False + self._disabled = False self._events = [] self._summarizer = EventSummarizer(config) self._output_transformer = EventOutputTransformer(config) @@ -206,13 +207,13 @@ def start(self): self._main_thread.start() def stop(self): - self._session.close() - self._running = False - # Post a non-message so we won't keep blocking on the queue - self._queue.put(EventProcessorMessage('stop', None)) + if self._running: + self._running = False + # Post a non-message so we won't keep blocking on the queue + self._queue.put(EventProcessorMessage('stop', None)) def is_alive(self): - return self._main_thread.is_alive() + return self._running def now(self): return int(time.time() * 1000) @@ -225,6 +226,7 @@ def _run_main_loop(self): self._process_next() except Exception: log.error('Unhandled exception in event processor', exc_info=True) + self._session.close() def _process_next(self): message = self._queue.get(block=True) @@ -236,6 +238,9 @@ def _process_next(self): self._summarizer.reset_users() def _process_event(self, event): + if self._disabled: + return + # For each user we haven't seen before, we add an index event - unless this is already # an identify event for that user. user = event.get('user') @@ -272,6 +277,11 @@ def _should_track_full_event(self, event): return True def _dispatch_flush(self, reply): + if self._disabled: + if reply is not None: + reply.set() + return + events = self._events self._events = [] snapshot = self._summarizer.snapshot() @@ -290,7 +300,7 @@ def _handle_response(self, r): self._last_known_past_time = server_date if r.status_code == 401: log.error('Received 401 error, no further events will be posted since SDK key is invalid') - self.stop() + self._disabled = true return From 4a53a0ca76bd42989ff256e6a0c763c3a1fc4cbb Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Mon, 26 Mar 2018 13:17:43 -0700 Subject: [PATCH 27/48] break out user deduplicator into a separate class --- ldclient/event_processor.py | 8 +++--- ldclient/event_summarizer.py | 23 +---------------- ldclient/user_deduplicator.py | 25 +++++++++++++++++++ testing/test_event_summarizer.py | 30 +++------------------- testing/test_user_deduplicator.py | 41 +++++++++++++++++++++++++++++++ 5 files changed, 76 insertions(+), 51 deletions(-) create mode 100644 ldclient/user_deduplicator.py create mode 100644 testing/test_user_deduplicator.py diff --git a/ldclient/event_processor.py b/ldclient/event_processor.py index 0ca69ce0..5155d243 100644 --- a/ldclient/event_processor.py +++ b/ldclient/event_processor.py @@ -20,6 +20,7 @@ import six from ldclient.event_summarizer import EventSummarizer +from ldclient.user_deduplicator import UserDeduplicator from ldclient.user_filter import UserFilter from ldclient.interfaces import EventProcessor from ldclient.repeating_timer import RepeatingTimer @@ -199,7 +200,8 @@ def __init__(self, queue, config, session): self._running = False self._disabled = False self._events = [] - self._summarizer = EventSummarizer(config) + self._summarizer = EventSummarizer() + self._user_deduplicator = UserDeduplicator(config) self._output_transformer = EventOutputTransformer(config) self._last_known_past_time = 0 @@ -235,7 +237,7 @@ def _process_next(self): elif message.type == 'flush': self._dispatch_flush(message.param) elif message.type == 'flush_users': - self._summarizer.reset_users() + self._user_deduplicator.reset_users() def _process_event(self, event): if self._disabled: @@ -244,7 +246,7 @@ def _process_event(self, event): # For each user we haven't seen before, we add an index event - unless this is already # an identify event for that user. user = event.get('user') - if not self._config.inline_users_in_events and user and not self._summarizer.notice_user(user): + if not self._config.inline_users_in_events and user and not self._user_deduplicator.notice_user(user): if event['kind'] != 'identify': ie = { 'kind': 'index', 'creationDate': event['creationDate'], 'user': user } self._store_event(ie) diff --git a/ldclient/event_summarizer.py b/ldclient/event_summarizer.py index 5032a13d..b67cf549 100644 --- a/ldclient/event_summarizer.py +++ b/ldclient/event_summarizer.py @@ -1,36 +1,15 @@ from collections import namedtuple -import pylru EventSummary = namedtuple('EventSummary', ['start_date', 'end_date', 'counters']) class EventSummarizer(object): - def __init__(self, config): - self.user_keys = pylru.lrucache(config.user_keys_capacity) + def __init__(self): self.start_date = 0 self.end_date = 0 self.counters = dict() - """ - Add to the set of users we've noticed, and return true if the user was already known to us. - """ - def notice_user(self, user): - if user is None or 'key' not in user: - return False - key = user['key'] - if key in self.user_keys: - self.user_keys[key] # refresh cache item - return True - self.user_keys[key] = True - return False - - """ - Reset the set of users we've seen. - """ - def reset_users(self): - self.user_keys.clear() - """ Add this event to our counters, if it is a type of event we need to count. """ diff --git a/ldclient/user_deduplicator.py b/ldclient/user_deduplicator.py new file mode 100644 index 00000000..0cb96af3 --- /dev/null +++ b/ldclient/user_deduplicator.py @@ -0,0 +1,25 @@ +import pylru + + +class UserDeduplicator(object): + def __init__(self, config): + self.user_keys = pylru.lrucache(config.user_keys_capacity) + + """ + Add to the set of users we've noticed, and return true if the user was already known to us. + """ + def notice_user(self, user): + if user is None or 'key' not in user: + return False + key = user['key'] + if key in self.user_keys: + self.user_keys[key] # refresh cache item + return True + self.user_keys[key] = True + return False + + """ + Reset the set of users we've seen. + """ + def reset_users(self): + self.user_keys.clear() diff --git a/testing/test_event_summarizer.py b/testing/test_event_summarizer.py index 460b3477..ae411aaf 100644 --- a/testing/test_event_summarizer.py +++ b/testing/test_event_summarizer.py @@ -1,48 +1,26 @@ import pytest -from ldclient.config import Config from ldclient.event_summarizer import EventSummarizer user = { 'key': 'user1' } -def test_notice_user_returns_false_for_never_seen_user(): - es = EventSummarizer(Config()) - assert es.notice_user(user) == False - -def test_notice_user_returns_true_for_previously_seen_user(): - es = EventSummarizer(Config()) - es.notice_user(user) - assert es.notice_user({ 'key': user['key'] }) == True - -def test_oldest_user_forgotten_if_capacity_exceeded(): - es = EventSummarizer(Config(user_keys_capacity = 2)) - user1 = { 'key': 'user1' } - user2 = { 'key': 'user2' } - user3 = { 'key': 'user3' } - es.notice_user(user1) - es.notice_user(user2) - es.notice_user(user3) - assert es.notice_user(user3) == True - assert es.notice_user(user2) == True - assert es.notice_user(user1) == False - def test_summarize_event_does_nothing_for_identify_event(): - es = EventSummarizer(Config()) + es = EventSummarizer() snapshot = es.snapshot() es.summarize_event({ 'kind': 'identify', 'creationDate': 1000, 'user': user }) assert es.snapshot() == snapshot def test_summarize_event_does_nothing_for_custom_event(): - es = EventSummarizer(Config()) + es = EventSummarizer() snapshot = es.snapshot() es.summarize_event({ 'kind': 'custom', 'creationDate': 1000, 'key': 'eventkey', 'user': user }) assert es.snapshot() == snapshot def test_summarize_event_sets_start_and_end_dates(): - es = EventSummarizer(Config()) + es = EventSummarizer() event1 = { 'kind': 'feature', 'creationDate': 2000, 'key': 'flag', 'user': user, 'version': 1, 'variation': 0, 'value': '', 'default': None } event2 = { 'kind': 'feature', 'creationDate': 1000, 'key': 'flag', 'user': user, @@ -58,7 +36,7 @@ def test_summarize_event_sets_start_and_end_dates(): assert data.end_date == 2000 def test_summarize_event_increments_counters(): - es = EventSummarizer(Config()) + es = EventSummarizer() event1 = { 'kind': 'feature', 'creationDate': 1000, 'key': 'flag1', 'user': user, 'version': 11, 'variation': 1, 'value': 'value1', 'default': 'default1' } event2 = { 'kind': 'feature', 'creationDate': 1000, 'key': 'flag1', 'user': user, diff --git a/testing/test_user_deduplicator.py b/testing/test_user_deduplicator.py new file mode 100644 index 00000000..1649832b --- /dev/null +++ b/testing/test_user_deduplicator.py @@ -0,0 +1,41 @@ +import pytest + +from ldclient.config import Config +from ldclient.user_deduplicator import UserDeduplicator + + +user = { 'key': 'user1' } + +def test_notice_user_returns_false_for_never_seen_user(): + ud = UserDeduplicator(Config()) + assert ud.notice_user(user) == False + +def test_notice_user_returns_true_for_previously_seen_user(): + ud = UserDeduplicator(Config()) + ud.notice_user(user) + assert ud.notice_user({ 'key': user['key'] }) == True + +def test_oldest_user_forgotten_if_capacity_exceeded(): + ud = UserDeduplicator(Config(user_keys_capacity = 2)) + user1 = { 'key': 'user1' } + user2 = { 'key': 'user2' } + user3 = { 'key': 'user3' } + ud.notice_user(user1) + ud.notice_user(user2) + ud.notice_user(user3) + assert ud.notice_user(user3) == True + assert ud.notice_user(user2) == True + assert ud.notice_user(user1) == False + +def test_user_becomes_new_again_each_time_we_notice_it(): + ud = UserDeduplicator(Config(user_keys_capacity = 2)) + user1 = { 'key': 'user1' } + user2 = { 'key': 'user2' } + user3 = { 'key': 'user3' } + ud.notice_user(user1) + ud.notice_user(user2) + ud.notice_user(user1) + ud.notice_user(user3) + assert ud.notice_user(user3) == True + assert ud.notice_user(user1) == True + assert ud.notice_user(user2) == False From 6108512e8b13bc198f3f2ae6e823e04b00474b90 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Thu, 29 Mar 2018 11:09:57 -0700 Subject: [PATCH 28/48] remove Python 2.6 support --- README.md | 7 ++----- ldclient/client.py | 2 +- ldclient/expiringdict.py | 6 +----- python2.6-requirements.txt | 1 - 4 files changed, 4 insertions(+), 12 deletions(-) delete mode 100644 python2.6-requirements.txt diff --git a/README.md b/README.md index aed91f43..d8f6ade8 100644 --- a/README.md +++ b/README.md @@ -38,12 +38,9 @@ Your first feature flag else: # the code to run if the feature is off -Python 2.6 +Supported Python versions ---------- -Python 2.6 requires an extra dependency. Here's how to set it up: - -1. Use the `python2.6` extra in your requirements.txt: - `ldclient-py[python2.6]` +The SDK is tested with the most recent patch releases of Python 2.7, 3.3, 3.4, 3.5, and 3.6. Python 2.6 is no longer supported. Learn more ----------- diff --git a/ldclient/client.py b/ldclient/client.py index 14a87e04..ef2e695f 100644 --- a/ldclient/client.py +++ b/ldclient/client.py @@ -21,7 +21,7 @@ import queue except: # noinspection PyUnresolvedReferences,PyPep8Naming - import Queue as queue + import Queue as queue # Python 3 from cachecontrol import CacheControl from threading import Lock diff --git a/ldclient/expiringdict.py b/ldclient/expiringdict.py index 8823be19..4b244c21 100644 --- a/ldclient/expiringdict.py +++ b/ldclient/expiringdict.py @@ -23,11 +23,7 @@ import time from threading import RLock -try: - from collections import OrderedDict -except ImportError: - # Python < 2.7 - from ordereddict import OrderedDict +from collections import OrderedDict class ExpiringDict(OrderedDict): diff --git a/python2.6-requirements.txt b/python2.6-requirements.txt deleted file mode 100644 index d73f64f0..00000000 --- a/python2.6-requirements.txt +++ /dev/null @@ -1 +0,0 @@ -ordereddict>=1.1 \ No newline at end of file From d5d0a38f1b92b87aff48a8f265e9101d98edf219 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Thu, 29 Mar 2018 21:10:18 -0700 Subject: [PATCH 29/48] fix setup.py --- setup.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/setup.py b/setup.py index 02307350..a083097f 100644 --- a/setup.py +++ b/setup.py @@ -12,14 +12,12 @@ # parse_requirements() returns generator of pip.req.InstallRequirement objects install_reqs = parse_requirements('requirements.txt', session=uuid.uuid1()) -python26_reqs = parse_requirements('python2.6-requirements.txt', session=uuid.uuid1()) test_reqs = parse_requirements('test-requirements.txt', session=uuid.uuid1()) redis_reqs = parse_requirements('redis-requirements.txt', session=uuid.uuid1()) # reqs is a list of requirement # e.g. ['django==1.5.1', 'mezzanine==1.4.6'] reqs = [str(ir.req) for ir in install_reqs] -python26reqs = [str(ir.req) for ir in python26_reqs] testreqs = [str(ir.req) for ir in test_reqs] redisreqs = [str(ir.req) for ir in redis_reqs] @@ -54,17 +52,17 @@ def run(self): 'License :: OSI Approved :: Apache Software License', 'Operating System :: OS Independent', 'Programming Language :: Python :: 2', - 'Programming Language :: Python :: 2.6', 'Programming Language :: Python :: 2.7', 'Programming Language :: Python :: 3', 'Programming Language :: Python :: 3.3', 'Programming Language :: Python :: 3.4', + 'Programming Language :: Python :: 3.5', + 'Programming Language :: Python :: 3.6', 'Topic :: Software Development', 'Topic :: Software Development :: Libraries', ], extras_require={ - "redis": redisreqs, - "python2.6": python26reqs + "redis": redisreqs }, tests_require=testreqs, cmdclass={'test': PyTest}, From 5a31bd860c9e89d581212dbe0bdf5434146599ba Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Tue, 3 Apr 2018 17:57:39 -0700 Subject: [PATCH 30/48] misc fixes --- ldclient/event_processor.py | 26 +++++++++++++------------- testing/test_event_processor.py | 22 +++++++++++++++++++++- 2 files changed, 34 insertions(+), 14 deletions(-) diff --git a/ldclient/event_processor.py b/ldclient/event_processor.py index 5155d243..327b0cdc 100644 --- a/ldclient/event_processor.py +++ b/ldclient/event_processor.py @@ -51,7 +51,7 @@ def flush(self): EventProcessorMessage = namedtuple('EventProcessorMessage', ['type', 'param']) -class EventOutputTransformer(object): +class EventOutputFormatter(object): def __init__(self, config): self._config = config self._user_filter = UserFilter(config) @@ -146,10 +146,10 @@ def start(self): def _run(self): try: - transformer = EventOutputTransformer(self._config) - output_events = [ transformer.make_output_event(e) for e in self._events ] + formatter = EventOutputFormatter(self._config) + output_events = [ formatter.make_output_event(e) for e in self._events ] if len(self._summary.counters) > 0: - output_events.append(transformer.make_summary_event(self._summary)) + output_events.append(formatter.make_summary_event(self._summary)) self._do_send(output_events, True) except: log.warning( @@ -179,7 +179,7 @@ def _do_send(self, output_events, should_retry): if inner.errno is not None and inner.errno == errno.ECONNRESET and should_retry: log.warning( 'ProtocolError exception caught while sending events. Retrying.') - self.do_send(output_events, False) + self._do_send(output_events, False) else: log.warning( 'Unhandled exception in event processor. Analytics events were not processed.', @@ -190,7 +190,7 @@ def _do_send(self, output_events, should_retry): exc_info=True) -class EventConsumer(object): +class EventDispatcher(object): def __init__(self, queue, config, session): self._queue = queue self._config = config @@ -202,7 +202,7 @@ def __init__(self, queue, config, session): self._events = [] self._summarizer = EventSummarizer() self._user_deduplicator = UserDeduplicator(config) - self._output_transformer = EventOutputTransformer(config) + self._formatter = EventOutputFormatter(config) self._last_known_past_time = 0 def start(self): @@ -302,19 +302,19 @@ def _handle_response(self, r): self._last_known_past_time = server_date if r.status_code == 401: log.error('Received 401 error, no further events will be posted since SDK key is invalid') - self._disabled = true + self._disabled = True return class DefaultEventProcessor(EventProcessor): def __init__(self, config, session=None): self._queue = queue.Queue(config.events_max_pending) - self._consumer = EventConsumer(self._queue, config, session) + self._dispatcher = EventDispatcher(self._queue, config, session) self._flush_timer = RepeatingTimer(config.flush_interval, self._flush_async) self._users_flush_timer = RepeatingTimer(config.user_keys_flush_interval, self._flush_users) def start(self): - self._consumer.start() + self._dispatcher.start() self._flush_timer.start() self._users_flush_timer.start() @@ -322,13 +322,13 @@ def stop(self): self._flush_timer.stop() self._users_flush_timer.stop() self.flush() - self._consumer.stop() + self._dispatcher.stop() def is_alive(self): - return self._consumer.is_alive() + return self._dispatcher.is_alive() def send_event(self, event): - event['creationDate'] = self._consumer.now() + event['creationDate'] = self._dispatcher.now() self._queue.put(EventProcessorMessage('event', event)) def flush(self): diff --git a/testing/test_event_processor.py b/testing/test_event_processor.py index 46c645a4..ccb531f0 100644 --- a/testing/test_event_processor.py +++ b/testing/test_event_processor.py @@ -43,6 +43,7 @@ class MockSession(object): def __init__(self): self._request_data = None self._request_headers = None + self._response_status = 200 self._server_time = None def post(self, uri, headers, timeout, data): @@ -51,7 +52,7 @@ def post(self, uri, headers, timeout, data): resp_hdr = CaseInsensitiveDict() if self._server_time is not None: resp_hdr['Date'] = formatdate(self._server_time / 1000, localtime=False, usegmt=True) - return MockResponse(200, resp_hdr) + return MockResponse(self._response_status, resp_hdr) def close(self): pass @@ -64,9 +65,16 @@ def request_data(self): def request_headers(self): return self._request_headers + def set_response_status(self, status): + self._response_status = status + def set_server_time(self, timestamp): self._server_time = timestamp + def clear(self): + self._request_headers = None + self._request_data = None + def setup_function(): global mock_session @@ -334,6 +342,18 @@ def test_sdk_key_is_sent(): assert mock_session.request_headers.get('Authorization') is 'SDK_KEY' +def test_no_more_payloads_are_sent_after_401_error(): + setup_processor(Config(sdk_key = 'SDK_KEY')) + + mock_session.set_response_status(401) + ep.send_event({ 'kind': 'identify', 'user': user }) + ep.flush() + mock_session.clear() + + ep.send_event({ 'kind': 'identify', 'user': user }) + ep.flush() + assert mock_session.request_data is None + def flush_and_get_events(): ep.flush() From 43784e047c8fe917499d67e48f37005d07a3ea72 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Tue, 3 Apr 2018 21:17:41 -0700 Subject: [PATCH 31/48] make flushes async, but ensure everything's been sent when shutting down --- ldclient/client.py | 3 +- ldclient/event_processor.py | 138 +++++++++++++++++--------------- ldclient/interfaces.py | 13 ++- testing/test_event_processor.py | 6 +- 4 files changed, 89 insertions(+), 71 deletions(-) diff --git a/ldclient/client.py b/ldclient/client.py index 6ab8451c..e8cc6e0a 100644 --- a/ldclient/client.py +++ b/ldclient/client.py @@ -53,7 +53,6 @@ def __init__(self, sdk_key=None, config=None, start_wait=5): self._event_processor = NullEventProcessor() else: self._event_processor = self._config.event_processor_class(self._config) - self._event_processor.start() if self._config.offline: log.info("Started LaunchDarkly Client in offline mode") @@ -103,7 +102,7 @@ def close(self): log.info("Closing LaunchDarkly client..") if self.is_offline(): return - if self._event_processor and self._event_processor.is_alive(): + if self._event_processor: self._event_processor.stop() if self._update_processor and self._update_processor.is_alive(): self._update_processor.stop() diff --git a/ldclient/event_processor.py b/ldclient/event_processor.py index 327b0cdc..9daf37d0 100644 --- a/ldclient/event_processor.py +++ b/ldclient/event_processor.py @@ -4,7 +4,7 @@ from email.utils import parsedate import errno import jsonpickle -from threading import Event, Thread +from threading import Event, Lock, Thread import time # noinspection PyBroadException @@ -133,13 +133,13 @@ def make_summary_event(self, summary): class EventPayloadSendTask(object): - def __init__(self, session, config, events, summary, response_listener, reply_event): + def __init__(self, session, config, events, summary, response_fn, completion_fn): self._session = session self._config = config self._events = events self._summary = summary - self._response_listener = response_listener - self._reply_event = reply_event + self._response_fn = response_fn + self._completion_fn = completion_fn def start(self): Thread(target = self._run).start() @@ -150,14 +150,15 @@ def _run(self): output_events = [ formatter.make_output_event(e) for e in self._events ] if len(self._summary.counters) > 0: output_events.append(formatter.make_summary_event(self._summary)) - self._do_send(output_events, True) + resp = self._do_send(output_events, True) + if resp is not None: + self._response_fn(resp) except: log.warning( 'Unhandled exception in event processor. Analytics events were not processed.', exc_info=True) finally: - if self._reply_event is not None: - self._reply_event.set() + self._completion_fn() def _do_send(self, output_events, should_retry): # noinspection PyBroadException @@ -170,9 +171,8 @@ def _do_send(self, output_events, should_retry): headers=hdrs, timeout=(self._config.connect_timeout, self._config.read_timeout), data=json_body) - if self._response_listener is not None: - self._response_listener(r) r.raise_for_status() + return r except ProtocolError as e: if e.args is not None and len(e.args) > 1 and e.args[1] is not None: inner = e.args[1] @@ -195,9 +195,6 @@ def __init__(self, queue, config, session): self._queue = queue self._config = config self._session = requests.Session() if session is None else session - self._main_thread = Thread(target=self._run_main_loop) - self._main_thread.daemon = True - self._running = False self._disabled = False self._events = [] self._summarizer = EventSummarizer() @@ -205,40 +202,36 @@ def __init__(self, queue, config, session): self._formatter = EventOutputFormatter(config) self._last_known_past_time = 0 - def start(self): - self._main_thread.start() - - def stop(self): - if self._running: - self._running = False - # Post a non-message so we won't keep blocking on the queue - self._queue.put(EventProcessorMessage('stop', None)) - - def is_alive(self): - return self._running + self._active_flush_workers_lock = Lock() + self._active_flush_workers_count = 0 + self._active_flush_workers_event = Event() - def now(self): - return int(time.time() * 1000) + self._main_thread = Thread(target=self._run_main_loop) + self._main_thread.daemon = True + self._main_thread.start() def _run_main_loop(self): log.info("Starting event processor") - self._running = True - while self._running: + while True: try: - self._process_next() + message = self._queue.get(block=True) + if message.type == 'event': + self._process_event(message.param) + elif message.type == 'flush': + self._trigger_flush() + elif message.type == 'flush_users': + self._user_deduplicator.reset_users() + elif message.type == 'test_sync': + self._wait_until_inactive() + message.param.set() + elif message.type == 'stop': + self._do_shutdown() + message.param.set() + return except Exception: log.error('Unhandled exception in event processor', exc_info=True) self._session.close() - - def _process_next(self): - message = self._queue.get(block=True) - if message.type == 'event': - self._process_event(message.param) - elif message.type == 'flush': - self._dispatch_flush(message.param) - elif message.type == 'flush_users': - self._user_deduplicator.reset_users() - + def _process_event(self, event): if self._disabled: return @@ -272,27 +265,26 @@ def _should_track_full_event(self, event): debug_until = event.get('debugEventsUntilDate') if debug_until is not None: last_past = self._last_known_past_time - if debug_until > last_past and debug_until > self.now(): + now = int(time.time() * 1000) + if debug_until > last_past and debug_until > now: return True return False else: return True - def _dispatch_flush(self, reply): + def _trigger_flush(self): if self._disabled: - if reply is not None: - reply.set() return - events = self._events self._events = [] snapshot = self._summarizer.snapshot() if len(events) > 0 or len(snapshot.counters) > 0: - task = EventPayloadSendTask(self._session, self._config, events, snapshot, self._handle_response, reply) + with self._active_flush_workers_lock: + # TODO: if we're at the limit, don't start a task and don't clear the state + self._active_flush_workers_count = self._active_flush_workers_count + 1 + task = EventPayloadSendTask(self._session, self._config, events, snapshot, + self._handle_response, self._release_flush_worker) task.start() - else: - if reply is not None: - reply.set() def _handle_response(self, r): server_date_str = r.headers.get('Date') @@ -305,40 +297,54 @@ def _handle_response(self, r): self._disabled = True return + def _release_flush_worker(self): + with self._active_flush_workers_lock: + self._active_flush_workers_count = self._active_flush_workers_count - 1 + self._active_flush_workers_event.clear() + self._active_flush_workers_event.set() + + def _wait_until_inactive(self): + while True: + with self._active_flush_workers_lock: + if self._active_flush_workers_count == 0: + return + self._active_flush_workers_event.wait() + + def _do_shutdown(self): + self._wait_until_inactive() + self._session.close() + class DefaultEventProcessor(EventProcessor): def __init__(self, config, session=None): self._queue = queue.Queue(config.events_max_pending) self._dispatcher = EventDispatcher(self._queue, config, session) - self._flush_timer = RepeatingTimer(config.flush_interval, self._flush_async) + self._flush_timer = RepeatingTimer(config.flush_interval, self.flush) self._users_flush_timer = RepeatingTimer(config.user_keys_flush_interval, self._flush_users) - - def start(self): - self._dispatcher.start() self._flush_timer.start() self._users_flush_timer.start() + def send_event(self, event): + event['creationDate'] = int(time.time() * 1000) + self._queue.put(EventProcessorMessage('event', event)) + + def flush(self): + self._queue.put(EventProcessorMessage('flush', None)) + def stop(self): self._flush_timer.stop() self._users_flush_timer.stop() self.flush() - self._dispatcher.stop() + self._post_message_and_wait('stop') - def is_alive(self): - return self._dispatcher.is_alive() + def _flush_users(self): + self._queue.put(EventProcessorMessage('flush_users', None)) - def send_event(self, event): - event['creationDate'] = self._dispatcher.now() - self._queue.put(EventProcessorMessage('event', event)) + # Used only in tests + def _wait_until_inactive(self): + self._post_message_and_wait('test_sync') - def flush(self): - # Put a flush message on the queue and wait until it's been processed. + def _post_message_and_wait(self, type): reply = Event() - self._queue.put(EventProcessorMessage('flush', reply)) + self._queue.put(EventProcessorMessage(type, reply)) reply.wait() - - def _flush_async(self): - self._queue.put(EventProcessorMessage('flush', None)) - - def _flush_users(self): - self._queue.put(EventProcessorMessage('flush_users', None)) diff --git a/ldclient/interfaces.py b/ldclient/interfaces.py index dd5065fa..39898408 100644 --- a/ldclient/interfaces.py +++ b/ldclient/interfaces.py @@ -113,7 +113,7 @@ def initialized(self): """ -class EventProcessor(BackgroundOperation): +class EventProcessor(object): """ Buffers analytics events and sends them to LaunchDarkly """ @@ -128,7 +128,16 @@ def send_event(self, event): @abstractmethod def flush(self): """ - Sends any outstanding events immediately. + Specifies that any buffered events should be sent as soon as possible, rather than waiting + for the next flush interval. This method is asynchronous, so events still may not be sent + until a later time. However, calling stop() will synchronously deliver any events that were + not yet delivered prior to shutting down. + """ + + @abstractmethod + def stop(self): + """ + Shuts down the event processor after first delivering all pending events. """ diff --git a/testing/test_event_processor.py b/testing/test_event_processor.py index ccb531f0..e40c2f8d 100644 --- a/testing/test_event_processor.py +++ b/testing/test_event_processor.py @@ -87,7 +87,6 @@ def teardown_function(): def setup_processor(config): global ep ep = DefaultEventProcessor(config, mock_session) - ep.start() def test_identify_event_is_queued(): @@ -332,6 +331,7 @@ def test_user_is_filtered_in_custom_event(): def test_nothing_is_sent_if_there_are_no_events(): setup_processor(Config()) ep.flush() + ep._wait_until_inactive() assert mock_session.request_data is None def test_sdk_key_is_sent(): @@ -339,6 +339,7 @@ def test_sdk_key_is_sent(): ep.send_event({ 'kind': 'identify', 'user': user }) ep.flush() + ep._wait_until_inactive() assert mock_session.request_headers.get('Authorization') is 'SDK_KEY' @@ -348,15 +349,18 @@ def test_no_more_payloads_are_sent_after_401_error(): mock_session.set_response_status(401) ep.send_event({ 'kind': 'identify', 'user': user }) ep.flush() + ep._wait_until_inactive() mock_session.clear() ep.send_event({ 'kind': 'identify', 'user': user }) ep.flush() + ep._wait_until_inactive() assert mock_session.request_data is None def flush_and_get_events(): ep.flush() + ep._wait_until_inactive() return None if mock_session.request_data is None else json.loads(mock_session.request_data) def check_index_event(data, source, user): From 06cc4a1ed9e93a46ae21c33710a84aa448ac35db Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Tue, 3 Apr 2018 21:39:15 -0700 Subject: [PATCH 32/48] put limit on concurrent flush tasks; put user dedup logic back into EventDispatcher --- ldclient/event_processor.py | 108 ++++++++++++++++++++---------- ldclient/event_summarizer.py | 7 +- ldclient/user_deduplicator.py | 25 ------- testing/test_event_processor.py | 5 +- testing/test_user_deduplicator.py | 41 ------------ 5 files changed, 80 insertions(+), 106 deletions(-) delete mode 100644 ldclient/user_deduplicator.py delete mode 100644 testing/test_user_deduplicator.py diff --git a/ldclient/event_processor.py b/ldclient/event_processor.py index 9daf37d0..4735d059 100644 --- a/ldclient/event_processor.py +++ b/ldclient/event_processor.py @@ -4,6 +4,7 @@ from email.utils import parsedate import errno import jsonpickle +import pylru from threading import Event, Lock, Thread import time @@ -20,7 +21,6 @@ import six from ldclient.event_summarizer import EventSummarizer -from ldclient.user_deduplicator import UserDeduplicator from ldclient.user_filter import UserFilter from ldclient.interfaces import EventProcessor from ldclient.repeating_timer import RepeatingTimer @@ -28,6 +28,8 @@ from ldclient.util import log +__MAX_FLUSH_THREADS__ = 5 + class NullEventProcessor(EventProcessor): def __init(self, config): pass @@ -53,12 +55,15 @@ def flush(self): class EventOutputFormatter(object): def __init__(self, config): - self._config = config + self._inline_users = config.inline_users_in_events self._user_filter = UserFilter(config) - """ - Transform an event into the format used for the event payload. - """ + def make_output_events(self, events, summary): + events_out = [ self.make_output_event(e) for e in events ] + if len(summary.counters) > 0: + events_out.append(self.make_summary_event(summary)) + return events_out + def make_output_event(self, e): kind = e['kind'] if kind == 'feature': @@ -72,7 +77,7 @@ def make_output_event(self, e): 'default': e.get('default'), 'prereqOf': e.get('prereqOf') } - if self._config.inline_users_in_events: + if self._inline_users: out['user'] = self._user_filter.filter_user_props(e['user']) else: out['userKey'] = e['user'].get('key') @@ -90,7 +95,7 @@ def make_output_event(self, e): 'key': e['key'], 'data': e.get('data') } - if self._config.inline_users_in_events: + if self._inline_users: out['user'] = self._user_filter.filter_user_props(e['user']) else: out['userKey'] = e['user'].get('key') @@ -133,23 +138,18 @@ def make_summary_event(self, summary): class EventPayloadSendTask(object): - def __init__(self, session, config, events, summary, response_fn, completion_fn): + def __init__(self, session, config, formatter, payload, response_fn, completion_fn): self._session = session self._config = config - self._events = events - self._summary = summary + self._formatter = formatter + self._payload = payload self._response_fn = response_fn self._completion_fn = completion_fn - - def start(self): Thread(target = self._run).start() def _run(self): try: - formatter = EventOutputFormatter(self._config) - output_events = [ formatter.make_output_event(e) for e in self._events ] - if len(self._summary.counters) > 0: - output_events.append(formatter.make_summary_event(self._summary)) + output_events = self._formatter.make_output_events(self._payload.events, self._payload.summary) resp = self._do_send(output_events, True) if resp is not None: self._response_fn(resp) @@ -190,15 +190,44 @@ def _do_send(self, output_events, should_retry): exc_info=True) +FlushPayload = namedtuple('FlushPayload', ['events', 'summary']) + + +class EventBuffer(object): + def __init__(self, capacity): + self._capacity = capacity + self._events = [] + self._summarizer = EventSummarizer() + self._exceeded_capacity = False + + def add_event(self, event): + if len(self._events) >= self._capacity: + if not self._exceeded_capacity: + log.warning("Event queue is full-- dropped an event") + self._exceeded_capacity = True + else: + self._events.append(event) + self._exceeded_capacity = False + + def add_to_summary(self, event): + self._summarizer.summarize_event(event) + + def get_payload(self): + return FlushPayload(self._events, self._summarizer.snapshot()) + + def clear(self): + self._events = [] + self._summarizer.clear() + + class EventDispatcher(object): def __init__(self, queue, config, session): self._queue = queue self._config = config self._session = requests.Session() if session is None else session self._disabled = False - self._events = [] - self._summarizer = EventSummarizer() - self._user_deduplicator = UserDeduplicator(config) + self._buffer = EventBuffer(config.events_max_pending) + self._user_keys = pylru.lrucache(config.user_keys_capacity) self._formatter = EventOutputFormatter(config) self._last_known_past_time = 0 @@ -220,7 +249,7 @@ def _run_main_loop(self): elif message.type == 'flush': self._trigger_flush() elif message.type == 'flush_users': - self._user_deduplicator.reset_users() + self._user_keys.clear() elif message.type == 'test_sync': self._wait_until_inactive() message.param.set() @@ -239,24 +268,29 @@ def _process_event(self, event): # For each user we haven't seen before, we add an index event - unless this is already # an identify event for that user. user = event.get('user') - if not self._config.inline_users_in_events and user and not self._user_deduplicator.notice_user(user): + if not self._config.inline_users_in_events and user and not self.notice_user(user): if event['kind'] != 'identify': ie = { 'kind': 'index', 'creationDate': event['creationDate'], 'user': user } - self._store_event(ie) + self._buffer.add_event(ie) # Always record the event in the summarizer. - self._summarizer.summarize_event(event) + self._buffer.add_to_summary(event) if self._should_track_full_event(event): # Queue the event as-is; we'll transform it into an output event when we're flushing # (to avoid doing that work on our main thread). - self._store_event(event) + self._buffer.add_event(event) - def _store_event(self, event): - if len(self._events) >= self._config.events_max_pending: - log.warning("Event queue is full-- dropped an event") - else: - self._events.append(event) + # Add to the set of users we've noticed, and return true if the user was already known to us. + def notice_user(self, user): + if user is None or 'key' not in user: + return False + key = user['key'] + if key in self._user_keys: + self._user_keys[key] # refresh cache item + return True + self._user_keys[key] = True + return False def _should_track_full_event(self, event): if event['kind'] == 'feature': @@ -275,16 +309,18 @@ def _should_track_full_event(self, event): def _trigger_flush(self): if self._disabled: return - events = self._events - self._events = [] - snapshot = self._summarizer.snapshot() - if len(events) > 0 or len(snapshot.counters) > 0: + payload = self._buffer.get_payload() + if len(payload.events) > 0 or len(payload.summary.counters) > 0: with self._active_flush_workers_lock: - # TODO: if we're at the limit, don't start a task and don't clear the state + if self._active_flush_workers_count >= __MAX_FLUSH_THREADS__: + # We're already at our limit of concurrent flushes; don't start a new task and + # do leave the events in the buffer + return self._active_flush_workers_count = self._active_flush_workers_count + 1 - task = EventPayloadSendTask(self._session, self._config, events, snapshot, + # Hand off the events to a new flush task and clear them from our buffer. + self._buffer.clear() + EventPayloadSendTask(self._session, self._config, self._formatter, payload, self._handle_response, self._release_flush_worker) - task.start() def _handle_response(self, r): server_date_str = r.headers.get('Date') diff --git a/ldclient/event_summarizer.py b/ldclient/event_summarizer.py index b67cf549..abdafc7d 100644 --- a/ldclient/event_summarizer.py +++ b/ldclient/event_summarizer.py @@ -29,11 +29,12 @@ def summarize_event(self, event): self.end_date = date """ - Return a snapshot of the current summarized event data, and reset this state. + Return the current summarized event data. """ def snapshot(self): - ret = EventSummary(start_date = self.start_date, end_date = self.end_date, counters = self.counters) + return EventSummary(start_date = self.start_date, end_date = self.end_date, counters = self.counters) + + def clear(self): self.start_date = 0 self.end_date = 0 self.counters = dict() - return ret diff --git a/ldclient/user_deduplicator.py b/ldclient/user_deduplicator.py deleted file mode 100644 index 0cb96af3..00000000 --- a/ldclient/user_deduplicator.py +++ /dev/null @@ -1,25 +0,0 @@ -import pylru - - -class UserDeduplicator(object): - def __init__(self, config): - self.user_keys = pylru.lrucache(config.user_keys_capacity) - - """ - Add to the set of users we've noticed, and return true if the user was already known to us. - """ - def notice_user(self, user): - if user is None or 'key' not in user: - return False - key = user['key'] - if key in self.user_keys: - self.user_keys[key] # refresh cache item - return True - self.user_keys[key] = True - return False - - """ - Reset the set of users we've seen. - """ - def reset_users(self): - self.user_keys.clear() diff --git a/testing/test_event_processor.py b/testing/test_event_processor.py index e40c2f8d..4263e965 100644 --- a/testing/test_event_processor.py +++ b/testing/test_event_processor.py @@ -361,7 +361,10 @@ def test_no_more_payloads_are_sent_after_401_error(): def flush_and_get_events(): ep.flush() ep._wait_until_inactive() - return None if mock_session.request_data is None else json.loads(mock_session.request_data) + if mock_session.request_data is None: + raise AssertionError('Expected to get an HTTP request but did not get one') + else: + return json.loads(mock_session.request_data) def check_index_event(data, source, user): assert data['kind'] == 'index' diff --git a/testing/test_user_deduplicator.py b/testing/test_user_deduplicator.py deleted file mode 100644 index 1649832b..00000000 --- a/testing/test_user_deduplicator.py +++ /dev/null @@ -1,41 +0,0 @@ -import pytest - -from ldclient.config import Config -from ldclient.user_deduplicator import UserDeduplicator - - -user = { 'key': 'user1' } - -def test_notice_user_returns_false_for_never_seen_user(): - ud = UserDeduplicator(Config()) - assert ud.notice_user(user) == False - -def test_notice_user_returns_true_for_previously_seen_user(): - ud = UserDeduplicator(Config()) - ud.notice_user(user) - assert ud.notice_user({ 'key': user['key'] }) == True - -def test_oldest_user_forgotten_if_capacity_exceeded(): - ud = UserDeduplicator(Config(user_keys_capacity = 2)) - user1 = { 'key': 'user1' } - user2 = { 'key': 'user2' } - user3 = { 'key': 'user3' } - ud.notice_user(user1) - ud.notice_user(user2) - ud.notice_user(user3) - assert ud.notice_user(user3) == True - assert ud.notice_user(user2) == True - assert ud.notice_user(user1) == False - -def test_user_becomes_new_again_each_time_we_notice_it(): - ud = UserDeduplicator(Config(user_keys_capacity = 2)) - user1 = { 'key': 'user1' } - user2 = { 'key': 'user2' } - user3 = { 'key': 'user3' } - ud.notice_user(user1) - ud.notice_user(user2) - ud.notice_user(user1) - ud.notice_user(user3) - assert ud.notice_user(user3) == True - assert ud.notice_user(user1) == True - assert ud.notice_user(user2) == False From 1702241bf200135d8134cd387afe0b4c79d733ef Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Tue, 3 Apr 2018 21:40:40 -0700 Subject: [PATCH 33/48] don't need to keep reference to dispatcher --- ldclient/event_processor.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ldclient/event_processor.py b/ldclient/event_processor.py index 4735d059..04b65666 100644 --- a/ldclient/event_processor.py +++ b/ldclient/event_processor.py @@ -354,11 +354,11 @@ def _do_shutdown(self): class DefaultEventProcessor(EventProcessor): def __init__(self, config, session=None): self._queue = queue.Queue(config.events_max_pending) - self._dispatcher = EventDispatcher(self._queue, config, session) self._flush_timer = RepeatingTimer(config.flush_interval, self.flush) self._users_flush_timer = RepeatingTimer(config.user_keys_flush_interval, self._flush_users) self._flush_timer.start() self._users_flush_timer.start() + EventDispatcher(self._queue, config, session) def send_event(self, event): event['creationDate'] = int(time.time() * 1000) From 3b328854bded4413380b67cd9bce2db63d152777 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Tue, 3 Apr 2018 21:53:03 -0700 Subject: [PATCH 34/48] fix time calculation --- ldclient/event_processor.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ldclient/event_processor.py b/ldclient/event_processor.py index 04b65666..90efd063 100644 --- a/ldclient/event_processor.py +++ b/ldclient/event_processor.py @@ -327,7 +327,7 @@ def _handle_response(self, r): if server_date_str is not None: server_date = parsedate(server_date_str) if server_date is not None: - self._last_known_past_time = server_date + self._last_known_past_time = int(time.mktime(server_date) * 1000) if r.status_code == 401: log.error('Received 401 error, no further events will be posted since SDK key is invalid') self._disabled = True From 3f62e01ea798b3d75df6bbc49d0be058635c1d35 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Tue, 3 Apr 2018 21:56:07 -0700 Subject: [PATCH 35/48] flush threads should be daemons --- ldclient/event_processor.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ldclient/event_processor.py b/ldclient/event_processor.py index 90efd063..e1d3466f 100644 --- a/ldclient/event_processor.py +++ b/ldclient/event_processor.py @@ -145,7 +145,9 @@ def __init__(self, session, config, formatter, payload, response_fn, completion_ self._payload = payload self._response_fn = response_fn self._completion_fn = completion_fn - Thread(target = self._run).start() + thread = Thread(target = self._run) + thread.daemon = True + thread.start() def _run(self): try: From 072f6cbc4373e7e662a20594e0228fa410c40181 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Tue, 3 Apr 2018 22:43:05 -0700 Subject: [PATCH 36/48] add thread pool class + misc fixes --- ldclient/event_processor.py | 56 +++++++++------------------- ldclient/fixed_thread_pool.py | 69 +++++++++++++++++++++++++++++++++++ 2 files changed, 87 insertions(+), 38 deletions(-) create mode 100644 ldclient/fixed_thread_pool.py diff --git a/ldclient/event_processor.py b/ldclient/event_processor.py index e1d3466f..4b32e006 100644 --- a/ldclient/event_processor.py +++ b/ldclient/event_processor.py @@ -21,6 +21,7 @@ import six from ldclient.event_summarizer import EventSummarizer +from ldclient.fixed_thread_pool import FixedThreadPool from ldclient.user_filter import UserFilter from ldclient.interfaces import EventProcessor from ldclient.repeating_timer import RepeatingTimer @@ -31,7 +32,7 @@ __MAX_FLUSH_THREADS__ = 5 class NullEventProcessor(EventProcessor): - def __init(self, config): + def __init__(self): pass def start(self): @@ -138,18 +139,14 @@ def make_summary_event(self, summary): class EventPayloadSendTask(object): - def __init__(self, session, config, formatter, payload, response_fn, completion_fn): + def __init__(self, session, config, formatter, payload, response_fn): self._session = session self._config = config self._formatter = formatter self._payload = payload self._response_fn = response_fn - self._completion_fn = completion_fn - thread = Thread(target = self._run) - thread.daemon = True - thread.start() - def _run(self): + def run(self): try: output_events = self._formatter.make_output_events(self._payload.events, self._payload.summary) resp = self._do_send(output_events, True) @@ -159,8 +156,6 @@ def _run(self): log.warning( 'Unhandled exception in event processor. Analytics events were not processed.', exc_info=True) - finally: - self._completion_fn() def _do_send(self, output_events, should_retry): # noinspection PyBroadException @@ -233,9 +228,7 @@ def __init__(self, queue, config, session): self._formatter = EventOutputFormatter(config) self._last_known_past_time = 0 - self._active_flush_workers_lock = Lock() - self._active_flush_workers_count = 0 - self._active_flush_workers_event = Event() + self._flush_workers = FixedThreadPool(__MAX_FLUSH_THREADS__, "ldclient.flush") self._main_thread = Thread(target=self._run_main_loop) self._main_thread.daemon = True @@ -253,7 +246,7 @@ def _run_main_loop(self): elif message.type == 'flush_users': self._user_keys.clear() elif message.type == 'test_sync': - self._wait_until_inactive() + self._flush_workers.wait() message.param.set() elif message.type == 'stop': self._do_shutdown() @@ -313,43 +306,30 @@ def _trigger_flush(self): return payload = self._buffer.get_payload() if len(payload.events) > 0 or len(payload.summary.counters) > 0: - with self._active_flush_workers_lock: - if self._active_flush_workers_count >= __MAX_FLUSH_THREADS__: - # We're already at our limit of concurrent flushes; don't start a new task and - # do leave the events in the buffer - return - self._active_flush_workers_count = self._active_flush_workers_count + 1 - # Hand off the events to a new flush task and clear them from our buffer. - self._buffer.clear() - EventPayloadSendTask(self._session, self._config, self._formatter, payload, - self._handle_response, self._release_flush_worker) + task = EventPayloadSendTask(self._session, self._config, self._formatter, payload, + self._handle_response) + if self._flush_workers.execute(task.run): + # The events have been handed off to a flush worker; clear them from our buffer. + self._buffer.clear() + else: + # We're already at our limit of concurrent flushes; leave the events in the buffer. + pass def _handle_response(self, r): server_date_str = r.headers.get('Date') if server_date_str is not None: server_date = parsedate(server_date_str) if server_date is not None: - self._last_known_past_time = int(time.mktime(server_date) * 1000) + timestamp = int(time.mktime(server_date) * 1000) + self._last_known_past_time = timestamp if r.status_code == 401: log.error('Received 401 error, no further events will be posted since SDK key is invalid') self._disabled = True return - def _release_flush_worker(self): - with self._active_flush_workers_lock: - self._active_flush_workers_count = self._active_flush_workers_count - 1 - self._active_flush_workers_event.clear() - self._active_flush_workers_event.set() - - def _wait_until_inactive(self): - while True: - with self._active_flush_workers_lock: - if self._active_flush_workers_count == 0: - return - self._active_flush_workers_event.wait() - def _do_shutdown(self): - self._wait_until_inactive() + self._flush_workers.stop() + self._flush_workers.wait() self._session.close() diff --git a/ldclient/fixed_thread_pool.py b/ldclient/fixed_thread_pool.py new file mode 100644 index 00000000..c0634f5b --- /dev/null +++ b/ldclient/fixed_thread_pool.py @@ -0,0 +1,69 @@ +from threading import Event, Lock, Thread + +# noinspection PyBroadException +try: + import queue +except: + # noinspection PyUnresolvedReferences,PyPep8Naming + import Queue as queue + +from ldclient.util import log + +""" +A simple fixed-size thread pool that rejects jobs when its limit is reached. +""" +class FixedThreadPool(object): + def __init__(self, size, name): + self._size = size + self._lock = Lock() + self._busy_count = 0 + self._event = Event() + self._job_queue = queue.Queue() + for i in range(0, size): + thread = Thread(target = self._run_worker) + thread.name = "%s.%d" % (name, i + 1) + thread.daemon = True + thread.start() + + """ + Schedules a job for execution if there is an available worker thread, and returns + true if successful; returns false if all threads are busy. + """ + def execute(self, jobFn): + with self._lock: + if self._busy_count >= self._size: + return False + self._busy_count = self._busy_count + 1 + self._job_queue.put(jobFn) + return True + + """ + Waits until all currently busy worker threads have completed their jobs. + """ + def wait(self): + while True: + with self._lock: + if self._busy_count == 0: + return + self._event.clear() + self._event.wait() + + """ + Tells all the worker threads to terminate once all active jobs have completed. + """ + def stop(self): + for i in range(0, self._size): + self._job_queue.put('stop') + + def _run_worker(self): + while True: + item = self._job_queue.get(block = True) + if item is 'stop': + return + try: + item() + except: + log.warning('Unhandled exception in worker thread', exc_info=True) + with self._lock: + self._busy_count = self._busy_count - 1 + self._event.set() From 710e1fb92cf740b5944fb0f27097373f0c7ff201 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Wed, 4 Apr 2018 11:13:35 -0700 Subject: [PATCH 37/48] fix overly broad exception catching --- ldclient/event_processor.py | 4 ++-- ldclient/fixed_thread_pool.py | 2 +- ldclient/polling.py | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/ldclient/event_processor.py b/ldclient/event_processor.py index 4b32e006..10e5df43 100644 --- a/ldclient/event_processor.py +++ b/ldclient/event_processor.py @@ -152,7 +152,7 @@ def run(self): resp = self._do_send(output_events, True) if resp is not None: self._response_fn(resp) - except: + except Exception: log.warning( 'Unhandled exception in event processor. Analytics events were not processed.', exc_info=True) @@ -181,7 +181,7 @@ def _do_send(self, output_events, should_retry): log.warning( 'Unhandled exception in event processor. Analytics events were not processed.', exc_info=True) - except: + except Exception: log.warning( 'Unhandled exception in event processor. Analytics events were not processed.', exc_info=True) diff --git a/ldclient/fixed_thread_pool.py b/ldclient/fixed_thread_pool.py index c0634f5b..a3c769e4 100644 --- a/ldclient/fixed_thread_pool.py +++ b/ldclient/fixed_thread_pool.py @@ -62,7 +62,7 @@ def _run_worker(self): return try: item() - except: + except Exception: log.warning('Unhandled exception in worker thread', exc_info=True) with self._lock: self._busy_count = self._busy_count - 1 diff --git a/ldclient/polling.py b/ldclient/polling.py index 4b71f668..8efa5913 100644 --- a/ldclient/polling.py +++ b/ldclient/polling.py @@ -34,7 +34,7 @@ def run(self): log.error('Received 401 error, no further polling requests will be made since SDK key is invalid') self.stop() break - except: + except Exception: log.exception( 'Error: Exception encountered when updating flags.') From dc201e65acb48c801a0aa1a78e0a795bd4ef8d79 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Wed, 4 Apr 2018 11:40:37 -0700 Subject: [PATCH 38/48] debugging --- ldclient/event_processor.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ldclient/event_processor.py b/ldclient/event_processor.py index 10e5df43..aedaffe0 100644 --- a/ldclient/event_processor.py +++ b/ldclient/event_processor.py @@ -365,4 +365,6 @@ def _wait_until_inactive(self): def _post_message_and_wait(self, type): reply = Event() self._queue.put(EventProcessorMessage(type, reply)) + print "*** waiting" reply.wait() + print "*** waited" From b4372d32cb57c7253d0155a5d7d9d8f225d2c739 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Wed, 4 Apr 2018 11:43:53 -0700 Subject: [PATCH 39/48] debugging --- ldclient/event_processor.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/ldclient/event_processor.py b/ldclient/event_processor.py index aedaffe0..5ed294a2 100644 --- a/ldclient/event_processor.py +++ b/ldclient/event_processor.py @@ -350,6 +350,7 @@ def flush(self): self._queue.put(EventProcessorMessage('flush', None)) def stop(self): + print("*** stopping: %d" % id(self)) self._flush_timer.stop() self._users_flush_timer.stop() self.flush() @@ -365,6 +366,6 @@ def _wait_until_inactive(self): def _post_message_and_wait(self, type): reply = Event() self._queue.put(EventProcessorMessage(type, reply)) - print "*** waiting" + print("*** waiting: %d" % id(self)) reply.wait() - print "*** waited" + print("*** waited: %d" % id(self)) From 72112ecdee26010f04397128a6bef98d8ca5af81 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Wed, 4 Apr 2018 11:53:59 -0700 Subject: [PATCH 40/48] tolerate being closed more than once --- ldclient/event_processor.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/ldclient/event_processor.py b/ldclient/event_processor.py index 5ed294a2..056a816b 100644 --- a/ldclient/event_processor.py +++ b/ldclient/event_processor.py @@ -340,6 +340,8 @@ def __init__(self, config, session=None): self._users_flush_timer = RepeatingTimer(config.user_keys_flush_interval, self._flush_users) self._flush_timer.start() self._users_flush_timer.start() + self._close_lock = Lock() + self._closed = False EventDispatcher(self._queue, config, session) def send_event(self, event): @@ -350,7 +352,10 @@ def flush(self): self._queue.put(EventProcessorMessage('flush', None)) def stop(self): - print("*** stopping: %d" % id(self)) + with self._close_lock: + if self._closed: + return + self._closed = True self._flush_timer.stop() self._users_flush_timer.stop() self.flush() @@ -366,6 +371,4 @@ def _wait_until_inactive(self): def _post_message_and_wait(self, type): reply = Event() self._queue.put(EventProcessorMessage(type, reply)) - print("*** waiting: %d" % id(self)) reply.wait() - print("*** waited: %d" % id(self)) From 4397b2ba0d336c7acc6d6b15a336978ab46c8063 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Wed, 4 Apr 2018 16:19:00 -0700 Subject: [PATCH 41/48] don't close session unless we created it --- ldclient/event_processor.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ldclient/event_processor.py b/ldclient/event_processor.py index 056a816b..8494323a 100644 --- a/ldclient/event_processor.py +++ b/ldclient/event_processor.py @@ -222,6 +222,7 @@ def __init__(self, queue, config, session): self._queue = queue self._config = config self._session = requests.Session() if session is None else session + self._close_session = (session is None) # so we know whether to close it later self._disabled = False self._buffer = EventBuffer(config.events_max_pending) self._user_keys = pylru.lrucache(config.user_keys_capacity) @@ -330,7 +331,8 @@ def _handle_response(self, r): def _do_shutdown(self): self._flush_workers.stop() self._flush_workers.wait() - self._session.close() + if self._close_session: + self._session.close() class DefaultEventProcessor(EventProcessor): From 821f4db0ff0fd3944d1dc64a005663adff3d1e47 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Mon, 9 Apr 2018 16:48:40 -0700 Subject: [PATCH 42/48] fix behavior of debug & index events --- ldclient/event_processor.py | 58 +++++++++++++++++++-------------- testing/test_event_processor.py | 34 ++++++++++++++++++- 2 files changed, 66 insertions(+), 26 deletions(-) diff --git a/ldclient/event_processor.py b/ldclient/event_processor.py index 8494323a..dc88fcc7 100644 --- a/ldclient/event_processor.py +++ b/ldclient/event_processor.py @@ -68,7 +68,7 @@ def make_output_events(self, events, summary): def make_output_event(self, e): kind = e['kind'] if kind == 'feature': - is_debug = (not e['trackEvents']) and (e.get('debugEventsUntilDate') is not None) + is_debug = e.get('debug') out = { 'kind': 'debug' if is_debug else 'feature', 'creationDate': e['creationDate'], @@ -78,7 +78,7 @@ def make_output_event(self, e): 'default': e.get('default'), 'prereqOf': e.get('prereqOf') } - if self._inline_users: + if self._inline_users or is_debug: out['user'] = self._user_filter.filter_user_props(e['user']) else: out['userKey'] = e['user'].get('key') @@ -261,21 +261,34 @@ def _process_event(self, event): if self._disabled: return - # For each user we haven't seen before, we add an index event - unless this is already - # an identify event for that user. - user = event.get('user') - if not self._config.inline_users_in_events and user and not self.notice_user(user): - if event['kind'] != 'identify': - ie = { 'kind': 'index', 'creationDate': event['creationDate'], 'user': user } - self._buffer.add_event(ie) - # Always record the event in the summarizer. self._buffer.add_to_summary(event) - if self._should_track_full_event(event): - # Queue the event as-is; we'll transform it into an output event when we're flushing - # (to avoid doing that work on our main thread). + # Decide whether to add the event to the payload. Feature events may be added twice, once for + # the event (if tracked) and once for debugging. + will_add_full_event = False + debug_event = None + if event['kind'] == "feature": + will_add_full_event = event['trackEvents'] + if self._should_debug_event(event): + debug_event = event.copy() + debug_event['debug'] = True + else: + will_add_full_event = True + + # For each user we haven't seen before, we add an index event - unless this is already + # an identify event for that user. + if not (will_add_full_event and self._config.inline_users_in_events): + user = event.get('user') + if user and not self.notice_user(user): + if event['kind'] != 'identify': + ie = { 'kind': 'index', 'creationDate': event['creationDate'], 'user': user } + self._buffer.add_event(ie) + + if will_add_full_event: self._buffer.add_event(event) + if debug_event is not None: + self._buffer.add_event(debug_event) # Add to the set of users we've noticed, and return true if the user was already known to us. def notice_user(self, user): @@ -288,19 +301,14 @@ def notice_user(self, user): self._user_keys[key] = True return False - def _should_track_full_event(self, event): - if event['kind'] == 'feature': - if event.get('trackEvents'): + def _should_debug_event(self, event): + debug_until = event.get('debugEventsUntilDate') + if debug_until is not None: + last_past = self._last_known_past_time + now = int(time.time() * 1000) + if debug_until > last_past and debug_until > now: return True - debug_until = event.get('debugEventsUntilDate') - if debug_until is not None: - last_past = self._last_known_past_time - now = int(time.time() * 1000) - if debug_until > last_past and debug_until > now: - return True - return False - else: - return True + return False def _trigger_flush(self): if self._disabled: diff --git a/testing/test_event_processor.py b/testing/test_event_processor.py index 4263e965..96146644 100644 --- a/testing/test_event_processor.py +++ b/testing/test_event_processor.py @@ -175,6 +175,20 @@ def test_user_is_filtered_in_feature_event(): check_feature_event(output[0], e, False, filtered_user) check_summary_event(output[1]) +def test_index_event_is_still_generated_if_inline_users_is_true_but_feature_event_is_not_tracked(): + setup_processor(Config(inline_users_in_events = True)) + + e = { + 'kind': 'feature', 'key': 'flagkey', 'version': 11, 'user': user, + 'variation': 1, 'value': 'value', 'default': 'default', 'trackEvents': False + } + ep.send_event(e) + + output = flush_and_get_events() + assert len(output) == 2 + check_index_event(output[0], e, user) + check_summary_event(output[1]) + def test_event_kind_is_debug_if_flag_is_temporarily_in_debug_mode(): setup_processor(Config()) @@ -189,9 +203,27 @@ def test_event_kind_is_debug_if_flag_is_temporarily_in_debug_mode(): output = flush_and_get_events() assert len(output) == 3 check_index_event(output[0], e, user) - check_feature_event(output[1], e, True, None) + check_feature_event(output[1], e, True, user) check_summary_event(output[2]) +def test_event_can_be_both_tracked_and_debugged(): + setup_processor(Config()) + + future_time = now() + 100000 + e = { + 'kind': 'feature', 'key': 'flagkey', 'version': 11, 'user': user, + 'variation': 1, 'value': 'value', 'default': 'default', + 'trackEvents': True, 'debugEventsUntilDate': future_time + } + ep.send_event(e) + + output = flush_and_get_events() + assert len(output) == 4 + check_index_event(output[0], e, user) + check_feature_event(output[1], e, False, None) + check_feature_event(output[2], e, True, user) + check_summary_event(output[3]) + def test_debug_mode_expires_based_on_client_time_if_client_time_is_later_than_server_time(): setup_processor(Config()) From 00fd2efeeb34310bfe7fc68d7d8fa32d9ad26a92 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Tue, 10 Apr 2018 17:04:41 -0700 Subject: [PATCH 43/48] try to clarify flow in _process_event --- ldclient/event_processor.py | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/ldclient/event_processor.py b/ldclient/event_processor.py index dc88fcc7..ad880a54 100644 --- a/ldclient/event_processor.py +++ b/ldclient/event_processor.py @@ -266,28 +266,31 @@ def _process_event(self, event): # Decide whether to add the event to the payload. Feature events may be added twice, once for # the event (if tracked) and once for debugging. - will_add_full_event = False - debug_event = None + add_full_event = False + add_debug_event = False + add_index_event = False if event['kind'] == "feature": - will_add_full_event = event['trackEvents'] - if self._should_debug_event(event): - debug_event = event.copy() - debug_event['debug'] = True + add_full_event = event['trackEvents'] + add_debug_event = self._should_debug_event(event) else: - will_add_full_event = True + add_full_event = True # For each user we haven't seen before, we add an index event - unless this is already # an identify event for that user. - if not (will_add_full_event and self._config.inline_users_in_events): + if not (add_full_event and self._config.inline_users_in_events): user = event.get('user') if user and not self.notice_user(user): if event['kind'] != 'identify': - ie = { 'kind': 'index', 'creationDate': event['creationDate'], 'user': user } - self._buffer.add_event(ie) + add_index_event = True - if will_add_full_event: + if add_index_event: + ie = { 'kind': 'index', 'creationDate': event['creationDate'], 'user': user } + self._buffer.add_event(ie) + if add_full_event: self._buffer.add_event(event) - if debug_event is not None: + if add_debug_event: + debug_event = event.copy() + debug_event['debug'] = True self._buffer.add_event(debug_event) # Add to the set of users we've noticed, and return true if the user was already known to us. From 83f2a03a428413379d9132e0b374c13b82029d96 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Wed, 18 Apr 2018 14:35:38 -0700 Subject: [PATCH 44/48] set schema header in event payload --- ldclient/event_processor.py | 1 + 1 file changed, 1 insertion(+) diff --git a/ldclient/event_processor.py b/ldclient/event_processor.py index ad880a54..b5c1690e 100644 --- a/ldclient/event_processor.py +++ b/ldclient/event_processor.py @@ -163,6 +163,7 @@ def _do_send(self, output_events, should_retry): json_body = jsonpickle.encode(output_events, unpicklable=False) log.debug('Sending events payload: ' + json_body) hdrs = _headers(self._config.sdk_key) + hdrs['X-LaunchDarkly-Event-Schema'] = '2' uri = self._config.events_uri r = self._session.post(uri, headers=hdrs, From 91bc3d88430b6e4033d5546890eaef740e63ed2f Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Mon, 23 Apr 2018 12:12:15 -0700 Subject: [PATCH 45/48] fix all_flags method + more unit tests --- ldclient/client.py | 11 +++--- ldclient/flag.py | 8 +++-- testing/test_flag.py | 28 +++++++-------- testing/test_ldclient.py | 74 +++++++++++++++++++++++++++------------- 4 files changed, 76 insertions(+), 45 deletions(-) diff --git a/ldclient/client.py b/ldclient/client.py index 0e91d8e9..c9af9d0a 100644 --- a/ldclient/client.py +++ b/ldclient/client.py @@ -186,14 +186,13 @@ def _evaluate(self, flag, user): return evaluate(flag, user, self._store) def _evaluate_and_send_events(self, flag, user, default): - variation, value, events = evaluate(flag, user, self._store) - for event in events or []: + result = evaluate(flag, user, self._store) + for event in result.events or []: self._send_event(event) - if value is None: - value = default + value = default if result.value is None else result.value self._send_event({'kind': 'feature', 'key': flag.get('key'), - 'user': user, 'variation': variation, 'value': value, + 'user': user, 'variation': result.variation, 'value': value, 'default': default, 'version': flag.get('version'), 'trackEvents': flag.get('trackEvents'), 'debugEventsUntilDate': flag.get('debugEventsUntilDate')}) @@ -225,7 +224,7 @@ def cb(all_flags): return self._store.all(FEATURES, cb) def _evaluate_multi(self, user, flags): - return dict([(k, self._evaluate(v, user)[0]) for k, v in flags.items() or {}]) + return dict([(k, self._evaluate(v, user).value) for k, v in flags.items() or {}]) def secure_mode_hash(self, user): if user.get('key') is None or self._config.sdk_key is None: diff --git a/ldclient/flag.py b/ldclient/flag.py index cc7572fe..7b0e9ed3 100644 --- a/ldclient/flag.py +++ b/ldclient/flag.py @@ -1,3 +1,4 @@ +from collections import namedtuple import hashlib import logging @@ -15,16 +16,19 @@ log = logging.getLogger(sys.modules[__name__].__name__) +EvalResult = namedtuple('EvalResult', ['variation', 'value', 'events']) + + def evaluate(flag, user, store): prereq_events = [] if flag.get('on', False): variation, value, prereq_events = _evaluate(flag, user, store) if value is not None: - return variation, value, prereq_events + return EvalResult(variation = variation, value = value, events = prereq_events) off_var = flag.get('offVariation') off_value = None if off_var is None else _get_variation(flag, off_var) - return off_var, off_value, prereq_events + return EvalResult(variation = off_var, value = off_value, events = prereq_events) def _evaluate(flag, user, store, prereq_events=None): diff --git a/testing/test_flag.py b/testing/test_flag.py index ef3ef69e..29d2bb61 100644 --- a/testing/test_flag.py +++ b/testing/test_flag.py @@ -1,6 +1,6 @@ import pytest from ldclient.feature_store import InMemoryFeatureStore -from ldclient.flag import _bucket_user, evaluate +from ldclient.flag import EvalResult, _bucket_user, evaluate from ldclient.versioned_data_kind import FEATURES, SEGMENTS @@ -16,7 +16,7 @@ def test_flag_returns_off_variation_if_flag_is_off(): 'variations': ['a', 'b', 'c'] } user = { 'key': 'x' } - assert evaluate(flag, user, empty_store) == (1, 'b', []) + assert evaluate(flag, user, empty_store) == EvalResult(1, 'b', []) def test_flag_returns_none_if_flag_is_off_and_off_variation_is_unspecified(): flag = { @@ -26,7 +26,7 @@ def test_flag_returns_none_if_flag_is_off_and_off_variation_is_unspecified(): 'variations': ['a', 'b', 'c'] } user = { 'key': 'x' } - assert evaluate(flag, user, empty_store) == (None, None, []) + assert evaluate(flag, user, empty_store) == EvalResult(None, None, []) def test_flag_returns_off_variation_if_prerequisite_not_found(): flag = { @@ -38,7 +38,7 @@ def test_flag_returns_off_variation_if_prerequisite_not_found(): 'variations': ['a', 'b', 'c'] } user = { 'key': 'x' } - assert evaluate(flag, user, empty_store) == (1, 'b', []) + assert evaluate(flag, user, empty_store) == EvalResult(1, 'b', []) def test_flag_returns_off_variation_and_event_if_prerequisite_is_not_met(): store = InMemoryFeatureStore() @@ -63,7 +63,7 @@ def test_flag_returns_off_variation_and_event_if_prerequisite_is_not_met(): user = { 'key': 'x' } events_should_be = [{'kind': 'feature', 'key': 'feature1', 'variation': 0, 'value': 'd', 'version': 2, 'user': user, 'prereqOf': 'feature0', 'trackEvents': False, 'debugEventsUntilDate': None}] - assert evaluate(flag, user, store) == (1, 'b', events_should_be) + assert evaluate(flag, user, store) == EvalResult(1, 'b', events_should_be) def test_flag_returns_fallthrough_and_event_if_prereq_is_met_and_there_are_no_rules(): store = InMemoryFeatureStore() @@ -88,7 +88,7 @@ def test_flag_returns_fallthrough_and_event_if_prereq_is_met_and_there_are_no_ru user = { 'key': 'x' } events_should_be = [{'kind': 'feature', 'key': 'feature1', 'variation': 1, 'value': 'e', 'version': 2, 'user': user, 'prereqOf': 'feature0', 'trackEvents': False, 'debugEventsUntilDate': None}] - assert evaluate(flag, user, store) == (0, 'a', events_should_be) + assert evaluate(flag, user, store) == EvalResult(0, 'a', events_should_be) def test_flag_matches_user_from_targets(): flag = { @@ -100,7 +100,7 @@ def test_flag_matches_user_from_targets(): 'variations': ['a', 'b', 'c'] } user = { 'key': 'userkey' } - assert evaluate(flag, user, empty_store) == (2, 'c', []) + assert evaluate(flag, user, empty_store) == EvalResult(2, 'c', []) def test_flag_matches_user_from_rules(): flag = { @@ -123,7 +123,7 @@ def test_flag_matches_user_from_rules(): 'variations': ['a', 'b', 'c'] } user = { 'key': 'userkey' } - assert evaluate(flag, user, empty_store) == (2, 'c', []) + assert evaluate(flag, user, empty_store) == EvalResult(2, 'c', []) def test_segment_match_clause_retrieves_segment_from_store(): store = InMemoryFeatureStore() @@ -154,7 +154,7 @@ def test_segment_match_clause_retrieves_segment_from_store(): ] } - assert evaluate(flag, user, store) == (1, True, []) + assert evaluate(flag, user, store) == EvalResult(1, True, []) def test_segment_match_clause_falls_through_with_no_errors_if_segment_not_found(): user = { "key": "foo" } @@ -177,7 +177,7 @@ def test_segment_match_clause_falls_through_with_no_errors_if_segment_not_found( ] } - assert evaluate(flag, user, empty_store) == (0, False, []) + assert evaluate(flag, user, empty_store) == EvalResult(0, False, []) def test_clause_matches_builtin_attribute(): clause = { @@ -187,7 +187,7 @@ def test_clause_matches_builtin_attribute(): } user = { 'key': 'x', 'name': 'Bob' } flag = _make_bool_flag_from_clause(clause) - assert evaluate(flag, user, empty_store) == (1, True, []) + assert evaluate(flag, user, empty_store) == EvalResult(1, True, []) def test_clause_matches_custom_attribute(): clause = { @@ -197,7 +197,7 @@ def test_clause_matches_custom_attribute(): } user = { 'key': 'x', 'name': 'Bob', 'custom': { 'legs': 4 } } flag = _make_bool_flag_from_clause(clause) - assert evaluate(flag, user, empty_store) == (1, True, []) + assert evaluate(flag, user, empty_store) == EvalResult(1, True, []) def test_clause_returns_false_for_missing_attribute(): clause = { @@ -207,7 +207,7 @@ def test_clause_returns_false_for_missing_attribute(): } user = { 'key': 'x', 'name': 'Bob' } flag = _make_bool_flag_from_clause(clause) - assert evaluate(flag, user, empty_store) == (0, False, []) + assert evaluate(flag, user, empty_store) == EvalResult(0, False, []) def test_clause_can_be_negated(): clause = { @@ -218,7 +218,7 @@ def test_clause_can_be_negated(): } user = { 'key': 'x', 'name': 'Bob' } flag = _make_bool_flag_from_clause(clause) - assert evaluate(flag, user, empty_store) == (0, False, []) + assert evaluate(flag, user, empty_store) == EvalResult(0, False, []) def _make_bool_flag_from_clause(clause): diff --git a/testing/test_ldclient.py b/testing/test_ldclient.py index 94a3efef..4ccbba34 100644 --- a/testing/test_ldclient.py +++ b/testing/test_ldclient.py @@ -48,30 +48,9 @@ def stop(self): def is_alive(self): return True + def initialized(self): + return True -feature = { - u'key': u'feature.key', - u'salt': u'abc', - u'on': True, - u'variations': [ - { - u'value': True, - u'weight': 100, - u'targets': [] - }, - { - u'value': False, - u'weight': 0, - u'targets': [] - } - ] -} -feature_store = InMemoryFeatureStore() -feature_store.init({ - FEATURES: { - 'feature.key': feature - } -}) client = LDClient(config=Config(base_uri="http://localhost:3000", event_processor_class = MockEventProcessor, update_processor_class = MockUpdateProcessor)) @@ -218,6 +197,55 @@ def test_no_defaults(): assert "bar" == offline_client.variation('foo', user, default="bar") +def test_event_for_existing_feature(): + feature = { + u'key': u'feature.key', + u'salt': u'abc', + u'on': True, + u'variations': ['a', 'b'], + u'fallthrough': { + u'variation': 1 + } + } + store = InMemoryFeatureStore() + store.init({FEATURES: {'feature.key': feature}}) + client = LDClient(config=Config(sdk_key = 'SDK_KEY', + base_uri="http://localhost:3000", + event_processor_class=MockEventProcessor, + update_processor_class=MockUpdateProcessor, + feature_store=store)) + assert 'b' == client.variation('feature.key', user, default='c') + e = get_first_event(client) + assert (e['kind'] == 'feature' and + e['key'] == 'feature.key' and + e['user'] == user and + e['value'] == 'b' and + e['variation'] == 1 and + e['default'] == 'c') + + +def test_all_flags(): + feature = { + u'key': u'feature.key', + u'salt': u'abc', + u'on': True, + u'variations': ['a', 'b'], + u'fallthrough': { + u'variation': 1 + } + } + store = InMemoryFeatureStore() + store.init({FEATURES: {'feature.key': feature}}) + client = LDClient(config=Config(sdk_key = 'SDK_KEY', + base_uri="http://localhost:3000", + event_processor_class=MockEventProcessor, + update_processor_class=MockUpdateProcessor, + feature_store=store)) + result = client.all_flags(user) + assert (len(result) == 1 and + result.get('feature.key') == 'b') + + def test_secure_mode_hash(): user = {'key': 'Message'} assert offline_client.secure_mode_hash(user) == "aa747c502a898200f9e4fa21bac68136f886a0e27aec70ba06daf2e2a5cb5597" From 60d5d423d752679412f000d44c8db76efff02bf0 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Mon, 23 Apr 2018 15:58:17 -0700 Subject: [PATCH 46/48] re-add redundant key property in identify event --- ldclient/event_processor.py | 1 + testing/test_event_processor.py | 2 ++ 2 files changed, 3 insertions(+) diff --git a/ldclient/event_processor.py b/ldclient/event_processor.py index b5c1690e..81683052 100644 --- a/ldclient/event_processor.py +++ b/ldclient/event_processor.py @@ -87,6 +87,7 @@ def make_output_event(self, e): return { 'kind': 'identify', 'creationDate': e['creationDate'], + 'key': e['user'].get('key'), 'user': self._user_filter.filter_user_props(e['user']) } elif kind == 'custom': diff --git a/testing/test_event_processor.py b/testing/test_event_processor.py index 96146644..149b5a70 100644 --- a/testing/test_event_processor.py +++ b/testing/test_event_processor.py @@ -100,6 +100,7 @@ def test_identify_event_is_queued(): assert output == [{ 'kind': 'identify', 'creationDate': e['creationDate'], + 'key': user['key'], 'user': user }] @@ -114,6 +115,7 @@ def test_user_is_filtered_in_identify_event(): assert output == [{ 'kind': 'identify', 'creationDate': e['creationDate'], + 'key': user['key'], 'user': filtered_user }] From 124377833908f2c83f011566c89be23af65f8e4e Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Tue, 24 Apr 2018 22:04:01 -0700 Subject: [PATCH 47/48] send as much of a feature event as possible even if user is invalid --- ldclient/client.py | 29 ++++++----- testing/test_ldclient.py | 102 +++++++++++++++++++++++++++++---------- 2 files changed, 93 insertions(+), 38 deletions(-) diff --git a/ldclient/client.py b/ldclient/client.py index c9af9d0a..22d63ea8 100644 --- a/ldclient/client.py +++ b/ldclient/client.py @@ -139,7 +139,8 @@ def toggle(self, key, user, default): def variation(self, key, user, default): default = self._config.get_default(key, default) - self._sanitize_user(user) + if user is not None: + self._sanitize_user(user) if self._config.offline: return default @@ -158,12 +159,7 @@ def send_event(value, version=None): send_event(default) return default - if user is None or user.get('key') is None: - log.warn("Missing user or user key when evaluating Feature Flag key: " + key + ". Returning default.") - send_event(default) - return default - - if user.get('key', "") == "": + if user is not None and user.get('key', "") == "": log.warn("User key is blank. Flag evaluation will proceed, but the user will not be stored in LaunchDarkly.") def cb(flag): @@ -177,6 +173,7 @@ def cb(flag): except Exception as e: log.error("Exception caught in variation: " + e.message + " for flag key: " + key + " and user: " + str(user)) + send_event(default) return default @@ -186,13 +183,19 @@ def _evaluate(self, flag, user): return evaluate(flag, user, self._store) def _evaluate_and_send_events(self, flag, user, default): - result = evaluate(flag, user, self._store) - for event in result.events or []: - self._send_event(event) - - value = default if result.value is None else result.value + if user is None or user.get('key') is None: + log.warn("Missing user or user key when evaluating Feature Flag key: " + flag.get('key') + ". Returning default.") + value = default + variation = None + else: + result = evaluate(flag, user, self._store) + for event in result.events or []: + self._send_event(event) + value = default if result.value is None else result.value + variation = result.variation + self._send_event({'kind': 'feature', 'key': flag.get('key'), - 'user': user, 'variation': result.variation, 'value': value, + 'user': user, 'variation': variation, 'value': value, 'default': default, 'version': flag.get('version'), 'trackEvents': flag.get('trackEvents'), 'debugEventsUntilDate': flag.get('debugEventsUntilDate')}) diff --git a/testing/test_ldclient.py b/testing/test_ldclient.py index 4ccbba34..b05a0057 100644 --- a/testing/test_ldclient.py +++ b/testing/test_ldclient.py @@ -88,6 +88,14 @@ def setup_function(function): } +def make_client(store): + return LDClient(config=Config(sdk_key = 'SDK_KEY', + base_uri="http://localhost:3000", + event_processor_class=MockEventProcessor, + update_processor_class=MockUpdateProcessor, + feature_store=store)) + + def get_first_event(c): return c._event_processor._events.pop(0) @@ -174,25 +182,6 @@ def test_defaults_and_online_no_default(): assert e['kind'] == 'feature' and e['key'] == u'baz' and e['user'] == user -def test_exception_in_retrieval(): - class ExceptionFeatureRequester(FeatureRequester): - def __init__(self, *_): - pass - - def get_all(self): - raise Exception("blah") - - client = LDClient(config=Config(base_uri="http://localhost:3000", - defaults={"foo": "bar"}, - feature_store=InMemoryFeatureStore(), - feature_requester_class=ExceptionFeatureRequester, - event_processor_class=MockEventProcessor, - update_processor_class=MockUpdateProcessor)) - assert "bar" == client.variation('foo', user, default="jim") - e = get_first_event(client) - assert e['kind'] == 'feature' and e['key'] == u'foo' and e['user'] == user - - def test_no_defaults(): assert "bar" == offline_client.variation('foo', user, default="bar") @@ -205,15 +194,12 @@ def test_event_for_existing_feature(): u'variations': ['a', 'b'], u'fallthrough': { u'variation': 1 - } + }, + u'trackEvents': True } store = InMemoryFeatureStore() store.init({FEATURES: {'feature.key': feature}}) - client = LDClient(config=Config(sdk_key = 'SDK_KEY', - base_uri="http://localhost:3000", - event_processor_class=MockEventProcessor, - update_processor_class=MockUpdateProcessor, - feature_store=store)) + client = make_client(store) assert 'b' == client.variation('feature.key', user, default='c') e = get_first_event(client) assert (e['kind'] == 'feature' and @@ -221,9 +207,75 @@ def test_event_for_existing_feature(): e['user'] == user and e['value'] == 'b' and e['variation'] == 1 and + e['default'] == 'c' and + e['trackEvents'] == True) + + +def test_event_for_unknown_feature(): + store = InMemoryFeatureStore() + store.init({FEATURES: {}}) + client = make_client(store) + assert 'c' == client.variation('feature.key', user, default='c') + e = get_first_event(client) + assert (e['kind'] == 'feature' and + e['key'] == 'feature.key' and + e['user'] == user and + e['value'] == 'c' and + e['variation'] == None and e['default'] == 'c') +def test_event_for_existing_feature_with_no_user(): + feature = { + u'key': u'feature.key', + u'salt': u'abc', + u'on': True, + u'variations': ['a', 'b'], + u'fallthrough': { + u'variation': 1 + }, + u'trackEvents': True + } + store = InMemoryFeatureStore() + store.init({FEATURES: {'feature.key': feature}}) + client = make_client(store) + assert 'c' == client.variation('feature.key', None, default='c') + e = get_first_event(client) + assert (e['kind'] == 'feature' and + e['key'] == 'feature.key' and + e['user'] == None and + e['value'] == 'c' and + e['variation'] == None and + e['default'] == 'c' and + e['trackEvents'] == True) + + +def test_event_for_existing_feature_with_no_user_key(): + feature = { + u'key': u'feature.key', + u'salt': u'abc', + u'on': True, + u'variations': ['a', 'b'], + u'fallthrough': { + u'variation': 1 + }, + u'trackEvents': True + } + store = InMemoryFeatureStore() + store.init({FEATURES: {'feature.key': feature}}) + client = make_client(store) + bad_user = { u'name': u'Bob' } + assert 'c' == client.variation('feature.key', bad_user, default='c') + e = get_first_event(client) + assert (e['kind'] == 'feature' and + e['key'] == 'feature.key' and + e['user'] == bad_user and + e['value'] == 'c' and + e['variation'] == None and + e['default'] == 'c' and + e['trackEvents'] == True) + + def test_all_flags(): feature = { u'key': u'feature.key', From 49e4a80ae5536c14b54dab8a671fd17651457235 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Mon, 30 Apr 2018 12:28:44 -0700 Subject: [PATCH 48/48] include variation index in events and bump schema version to 3 --- ldclient/event_processor.py | 6 +++++- testing/test_event_processor.py | 5 +++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/ldclient/event_processor.py b/ldclient/event_processor.py index 81683052..1ef54f0a 100644 --- a/ldclient/event_processor.py +++ b/ldclient/event_processor.py @@ -30,6 +30,7 @@ __MAX_FLUSH_THREADS__ = 5 +__CURRENT_EVENT_SCHEMA__ = 3 class NullEventProcessor(EventProcessor): def __init__(self): @@ -74,6 +75,7 @@ def make_output_event(self, e): 'creationDate': e['creationDate'], 'key': e['key'], 'version': e.get('version'), + 'variation': e.get('variation'), 'value': e.get('value'), 'default': e.get('default'), 'prereqOf': e.get('prereqOf') @@ -126,6 +128,8 @@ def make_summary_event(self, summary): 'count': cval['count'], 'value': cval['value'] } + if variation is not None: + counter['variation'] = variation if version is None: counter['unknown'] = True else: @@ -164,7 +168,7 @@ def _do_send(self, output_events, should_retry): json_body = jsonpickle.encode(output_events, unpicklable=False) log.debug('Sending events payload: ' + json_body) hdrs = _headers(self._config.sdk_key) - hdrs['X-LaunchDarkly-Event-Schema'] = '2' + hdrs['X-LaunchDarkly-Event-Schema'] = str(__CURRENT_EVENT_SCHEMA__) uri = self._config.events_uri r = self._session.post(uri, headers=hdrs, diff --git a/testing/test_event_processor.py b/testing/test_event_processor.py index 149b5a70..bb307773 100644 --- a/testing/test_event_processor.py +++ b/testing/test_event_processor.py @@ -323,11 +323,11 @@ def test_nontracked_events_are_summarized(): assert se['features'] == { 'flagkey1': { 'default': 'default1', - 'counters': [ { 'version': 11, 'value': 'value1', 'count': 1 } ] + 'counters': [ { 'version': 11, 'variation': 1, 'value': 'value1', 'count': 1 } ] }, 'flagkey2': { 'default': 'default2', - 'counters': [ { 'version': 22, 'value': 'value2', 'count': 1 } ] + 'counters': [ { 'version': 22, 'variation': 2, 'value': 'value2', 'count': 1 } ] } } @@ -410,6 +410,7 @@ def check_feature_event(data, source, debug, inline_user): assert data['creationDate'] == source['creationDate'] assert data['key'] == source['key'] assert data.get('version') == source.get('version') + assert data.get('variation') == source.get('variation') assert data.get('value') == source.get('value') assert data.get('default') == source.get('default') if inline_user is None: