From 01b6219a293771c6631d165b4ba548520f6c78f9 Mon Sep 17 00:00:00 2001 From: Gilbert Szeto Date: Wed, 2 Aug 2023 10:25:05 -0700 Subject: [PATCH 1/6] style --- src/sentry/integrations/request_buffer.py | 99 ++++++++++++----------- 1 file changed, 50 insertions(+), 49 deletions(-) diff --git a/src/sentry/integrations/request_buffer.py b/src/sentry/integrations/request_buffer.py index cbd6b0c7249a09..5964b85bf90854 100644 --- a/src/sentry/integrations/request_buffer.py +++ b/src/sentry/integrations/request_buffer.py @@ -2,13 +2,15 @@ from django.conf import settings -from sentry.utils import json, redis +from sentry.utils import redis BUFFER_SIZE = 30 # 30 days KEY_EXPIRY = 60 * 60 * 24 * 30 # 30 days IS_BROKEN_RANGE = 7 # 7 days +VALID_KEYS = ["success", "error", "fatal"] + class IntegrationRequestBuffer: """ @@ -22,42 +24,26 @@ def __init__(self, key): cluster_id = settings.SENTRY_INTEGRATION_ERROR_LOG_REDIS_CLUSTER self.client = redis.redis_clusters.get(cluster_id) - def _convert_obj_to_dict(self, redis_object): - """ - Convert the request string stored in Redis to a python dict - """ - return json.loads(redis_object) - - def _get_all_from_buffer(self): - """ - Get the list at the buffer key. - """ + def add(self, count: str): + if count not in VALID_KEYS: + raise Exception("Requires a valid key param.") - ret = [] - now = datetime.now() - i = 0 - buffer_key = f"{self.integrationkey}:{now.strftime('%Y-%m-%d')}" - while self.client.hgetall(buffer_key): - ret.append(self.client.hgetall(buffer_key)) - cur = (now - timedelta(days=i)).strftime("%Y-%m-%d") - buffer_key = f"{self.integrationkey}:{cur}" - i += 1 + now = datetime.now().strftime("%Y-%m-%d") + buffer_key = f"{self.integrationkey}:{now}" - return ret + pipe = self.client.pipeline() + pipe.hincrby(buffer_key, count + "_count", 1) + pipe.expire(buffer_key, KEY_EXPIRY) + pipe.execute() - def _get_broken_range_from_buffer(self): - """ - Get the list at the buffer key in the broken range. - """ + def record_error(self): + self.add("error") - ret = [] - now = datetime.now() - for i in range(IS_BROKEN_RANGE): - cur = (now - timedelta(days=i)).strftime("%Y-%m-%d") - buffer_key = f"{self.integrationkey}:{cur}" - ret.append(self.client.hgetall(buffer_key)) + def record_success(self): + self.add("success") - return ret + def record_fatal(self): + self.add("fatal") def is_integration_broken(self): """ @@ -85,25 +71,40 @@ def is_integration_broken(self): return True - def add(self, count: str): - VALID_KEYS = ["success", "error", "fatal"] - if count not in VALID_KEYS: - raise Exception("Requires a valid key param.") + def _get_all_from_buffer(self): + """ + Get the list at the buffer key. + """ - now = datetime.now().strftime("%Y-%m-%d") + now = datetime.now() + all_range = [ + f"{self.integrationkey}:{(now - timedelta(days=i)).strftime('%Y-%m-%d')}" + for i in range(BUFFER_SIZE) + ] - buffer_key = f"{self.integrationkey}:{now}" - pipe = self.client.pipeline() - pipe.hincrby(buffer_key, count + "_count", 1) - pipe.expire(buffer_key, KEY_EXPIRY) - pipe.execute() - pipe.reset() + return self._get_range_buffers(all_range) - def record_error(self): - self.add("error") + def _get_broken_range_from_buffer(self): + """ + Get the list at the buffer key in the broken range. + """ - def record_success(self): - self.add("success") + now = datetime.now() + broken_range_keys = [ + f"{self.integrationkey}:{(now - timedelta(days=i)).strftime('%Y-%m-%d')}" + for i in range(IS_BROKEN_RANGE) + ] - def record_fatal(self): - self.add("fatal") + return self._get_range_buffers(broken_range_keys) + + def _get_range_buffers(self, keys): + pipe = self.client.pipeline() + ret = [] + for key in keys: + pipe.hgetall(key) + response = pipe.execute() + for item in response: + if item: + ret.append(item) + + return ret From 933bde8a832bf9bfa0e58e0e81a64dff2f75a90e Mon Sep 17 00:00:00 2001 From: Gilbert Szeto Date: Wed, 2 Aug 2023 10:30:50 -0700 Subject: [PATCH 2/6] ref, don't filter out items in pipeline if that day range doesn't exist --- src/sentry/integrations/request_buffer.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/sentry/integrations/request_buffer.py b/src/sentry/integrations/request_buffer.py index 5964b85bf90854..bcdd20230ad7e8 100644 --- a/src/sentry/integrations/request_buffer.py +++ b/src/sentry/integrations/request_buffer.py @@ -19,7 +19,7 @@ class IntegrationRequestBuffer: """ def __init__(self, key): - self.integrationkey = key + self.integration_key = key cluster_id = settings.SENTRY_INTEGRATION_ERROR_LOG_REDIS_CLUSTER self.client = redis.redis_clusters.get(cluster_id) @@ -29,7 +29,7 @@ def add(self, count: str): raise Exception("Requires a valid key param.") now = datetime.now().strftime("%Y-%m-%d") - buffer_key = f"{self.integrationkey}:{now}" + buffer_key = f"{self.integration_key}:{now}" pipe = self.client.pipeline() pipe.hincrby(buffer_key, count + "_count", 1) @@ -78,7 +78,7 @@ def _get_all_from_buffer(self): now = datetime.now() all_range = [ - f"{self.integrationkey}:{(now - timedelta(days=i)).strftime('%Y-%m-%d')}" + f"{self.integration_key}:{(now - timedelta(days=i)).strftime('%Y-%m-%d')}" for i in range(BUFFER_SIZE) ] @@ -91,7 +91,7 @@ def _get_broken_range_from_buffer(self): now = datetime.now() broken_range_keys = [ - f"{self.integrationkey}:{(now - timedelta(days=i)).strftime('%Y-%m-%d')}" + f"{self.integration_key}:{(now - timedelta(days=i)).strftime('%Y-%m-%d')}" for i in range(IS_BROKEN_RANGE) ] @@ -104,7 +104,6 @@ def _get_range_buffers(self, keys): pipe.hgetall(key) response = pipe.execute() for item in response: - if item: - ret.append(item) + ret.append(item) return ret From ffffd541cd6178abba8449788da02ed26cfdf6fc Mon Sep 17 00:00:00 2001 From: Gilbert Szeto Date: Wed, 2 Aug 2023 10:42:27 -0700 Subject: [PATCH 3/6] rename variables to be more descriptive --- src/sentry/integrations/request_buffer.py | 25 +++++++++++++---------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/src/sentry/integrations/request_buffer.py b/src/sentry/integrations/request_buffer.py index bcdd20230ad7e8..dfe55f603b2455 100644 --- a/src/sentry/integrations/request_buffer.py +++ b/src/sentry/integrations/request_buffer.py @@ -7,7 +7,7 @@ BUFFER_SIZE = 30 # 30 days KEY_EXPIRY = 60 * 60 * 24 * 30 # 30 days -IS_BROKEN_RANGE = 7 # 7 days +BROKEN_RANGE_DAYS = 7 # 7 days VALID_KEYS = ["success", "error", "fatal"] @@ -50,23 +50,26 @@ def is_integration_broken(self): Integration is broken if we have 7 consecutive days of errors and no successes OR have a fatal error """ - items = self._get_broken_range_from_buffer() + broken_day_range = self._get_broken_range_from_buffer() - data = [item for item in items if int(item.get("fatal_count", 0)) > 0] + days_fatal = [ + day_count for day_count in broken_day_range if int(day_count.get("fatal_count", 0)) > 0 + ] - if len(data) > 0: + if len(days_fatal) > 0: return True - data = [ - item - for item in items - if int(item.get("error_count", 0)) > 0 and int(item.get("success_count", 0)) == 0 + days_error = [ + day_count + for day_count in broken_day_range + if int(day_count.get("error_count", 0)) > 0 + and int(day_count.get("success_count", 0)) == 0 ] - if not len(data): + if not len(days_error): return False - if len(data) < IS_BROKEN_RANGE: + if len(days_error) < BROKEN_RANGE_DAYS: return False return True @@ -92,7 +95,7 @@ def _get_broken_range_from_buffer(self): now = datetime.now() broken_range_keys = [ f"{self.integration_key}:{(now - timedelta(days=i)).strftime('%Y-%m-%d')}" - for i in range(IS_BROKEN_RANGE) + for i in range(BROKEN_RANGE_DAYS) ] return self._get_range_buffers(broken_range_keys) From 319e88e6f3b015896d1b3c0be1ebdc1c24becb6e Mon Sep 17 00:00:00 2001 From: Gilbert Szeto Date: Wed, 2 Aug 2023 10:43:42 -0700 Subject: [PATCH 4/6] ref --- src/sentry/integrations/request_buffer.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/sentry/integrations/request_buffer.py b/src/sentry/integrations/request_buffer.py index dfe55f603b2455..75779eae944803 100644 --- a/src/sentry/integrations/request_buffer.py +++ b/src/sentry/integrations/request_buffer.py @@ -50,10 +50,12 @@ def is_integration_broken(self): Integration is broken if we have 7 consecutive days of errors and no successes OR have a fatal error """ - broken_day_range = self._get_broken_range_from_buffer() + broken_range_days_counts = self._get_broken_range_from_buffer() days_fatal = [ - day_count for day_count in broken_day_range if int(day_count.get("fatal_count", 0)) > 0 + day_count + for day_count in broken_range_days_counts + if int(day_count.get("fatal_count", 0)) > 0 ] if len(days_fatal) > 0: @@ -61,7 +63,7 @@ def is_integration_broken(self): days_error = [ day_count - for day_count in broken_day_range + for day_count in broken_range_days_counts if int(day_count.get("error_count", 0)) > 0 and int(day_count.get("success_count", 0)) == 0 ] From 7bfcfb316b800ac4bb845388891bc203bee63f9d Mon Sep 17 00:00:00 2001 From: Gilbert Szeto Date: Wed, 2 Aug 2023 10:46:38 -0700 Subject: [PATCH 5/6] style --- src/sentry/integrations/request_buffer.py | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/src/sentry/integrations/request_buffer.py b/src/sentry/integrations/request_buffer.py index 75779eae944803..12568d93ef7d36 100644 --- a/src/sentry/integrations/request_buffer.py +++ b/src/sentry/integrations/request_buffer.py @@ -52,22 +52,21 @@ def is_integration_broken(self): """ broken_range_days_counts = self._get_broken_range_from_buffer() - days_fatal = [ - day_count - for day_count in broken_range_days_counts - if int(day_count.get("fatal_count", 0)) > 0 - ] + days_fatal = [] + days_error = [] + + for day_count in broken_range_days_counts: + if int(day_count.get("fatal_count", 0)) > 0: + days_fatal.append(day_count) + elif ( + int(day_count.get("error_count", 0)) > 0 + and int(day_count.get("success_count", 0)) == 0 + ): + days_error.append(day_count) if len(days_fatal) > 0: return True - days_error = [ - day_count - for day_count in broken_range_days_counts - if int(day_count.get("error_count", 0)) > 0 - and int(day_count.get("success_count", 0)) == 0 - ] - if not len(days_error): return False From 4679a345cd9af56bd755aee1984b8dddc328fbbb Mon Sep 17 00:00:00 2001 From: Gilbert Szeto Date: Wed, 2 Aug 2023 10:49:33 -0700 Subject: [PATCH 6/6] protected method --- src/sentry/integrations/request_buffer.py | 30 +++++++++++------------ 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/src/sentry/integrations/request_buffer.py b/src/sentry/integrations/request_buffer.py index 12568d93ef7d36..490bb53bf56cd3 100644 --- a/src/sentry/integrations/request_buffer.py +++ b/src/sentry/integrations/request_buffer.py @@ -24,26 +24,14 @@ def __init__(self, key): cluster_id = settings.SENTRY_INTEGRATION_ERROR_LOG_REDIS_CLUSTER self.client = redis.redis_clusters.get(cluster_id) - def add(self, count: str): - if count not in VALID_KEYS: - raise Exception("Requires a valid key param.") - - now = datetime.now().strftime("%Y-%m-%d") - buffer_key = f"{self.integration_key}:{now}" - - pipe = self.client.pipeline() - pipe.hincrby(buffer_key, count + "_count", 1) - pipe.expire(buffer_key, KEY_EXPIRY) - pipe.execute() - def record_error(self): - self.add("error") + self._add("error") def record_success(self): - self.add("success") + self._add("success") def record_fatal(self): - self.add("fatal") + self._add("fatal") def is_integration_broken(self): """ @@ -75,6 +63,18 @@ def is_integration_broken(self): return True + def _add(self, count: str): + if count not in VALID_KEYS: + raise Exception("Requires a valid key param.") + + now = datetime.now().strftime("%Y-%m-%d") + buffer_key = f"{self.integration_key}:{now}" + + pipe = self.client.pipeline() + pipe.hincrby(buffer_key, count + "_count", 1) + pipe.expire(buffer_key, KEY_EXPIRY) + pipe.execute() + def _get_all_from_buffer(self): """ Get the list at the buffer key.