Skip to content

feat(integration-slack): store request error counts and disable on broken #53619

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 102 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
102 commits
Select commit Hold shift + click to select a range
1add83f
from https://github.com/getsentry/sentry/pull/51126/files with fata…
chloeho7 Jul 12, 2023
f5067dd
adding fatal flag, integration id and uninstall
chloeho7 Jul 14, 2023
c215063
Merge branch 'master' into chloedisablenotifyslack
chloeho7 Jul 17, 2023
6aa9d08
adding slack
chloeho7 Jul 17, 2023
aadad52
Merge branch 'master' into chloedisablenotifyslack
chloeho7 Jul 17, 2023
5a5b875
adding mandatory integration id to slack and test cases
chloeho7 Jul 17, 2023
d2f2e32
:hammer_and_wrench: apply pre-commit fixes
getsantry[bot] Jul 17, 2023
f18dc24
:hammer_and_wrench: apply pre-commit fixes
getsantry[bot] Jul 17, 2023
71de356
making buffer legit
chloeho7 Jul 17, 2023
1e7a359
:hammer_and_wrench: apply pre-commit fixes
getsantry[bot] Jul 17, 2023
bf9d3fa
adding uninstall when broken
chloeho7 Jul 18, 2023
c147717
Merge branch 'master' into chloedisablenotifyslack
chloeho7 Jul 18, 2023
5dd5132
:hammer_and_wrench: apply pre-commit fixes
getsantry[bot] Jul 18, 2023
c2c1a50
:hammer_and_wrench: apply pre-commit fixes
getsantry[bot] Jul 18, 2023
0e6138d
disabling Integration
chloeho7 Jul 18, 2023
d3108c0
:hammer_and_wrench: apply pre-commit fixes
getsantry[bot] Jul 18, 2023
f1cb088
pass integration_id and get rpcintegration
ceorourke Jul 18, 2023
8b3ab28
Merge branch 'master' into chloedisablenotifyslack
chloeho7 Jul 18, 2023
cbd9169
making integrationid optional
chloeho7 Jul 18, 2023
77975f7
making integrationid optional
chloeho7 Jul 18, 2023
9ace2ec
fixing is response error
chloeho7 Jul 18, 2023
fd59006
Merge branch 'master' into chloedisablenotifyslack
chloeho7 Jul 18, 2023
42e4978
:hammer_and_wrench: apply pre-commit fixes
getsantry[bot] Jul 18, 2023
a0b7781
adding slack fatal errors and fixing tests
chloeho7 Jul 18, 2023
976c1d3
:hammer_and_wrench: apply pre-commit fixes
getsantry[bot] Jul 18, 2023
7687f0f
fixing buffer key
chloeho7 Jul 18, 2023
5bbf526
fixing buffer var names
chloeho7 Jul 18, 2023
fd82865
record_request and buffer fixes
chloeho7 Jul 18, 2023
b7f6442
:hammer_and_wrench: apply pre-commit fixes
getsantry[bot] Jul 18, 2023
d432edc
:hammer_and_wrench: apply pre-commit fixes
getsantry[bot] Jul 18, 2023
5c95bd0
Merge branch 'master' into chloedisablenotifyslack
chloeho7 Jul 18, 2023
d85d753
checking for integrationid w hasattr()
chloeho7 Jul 18, 2023
a4aff25
:hammer_and_wrench: apply pre-commit fixes
getsantry[bot] Jul 18, 2023
4b389fd
bruh
chloeho7 Jul 18, 2023
64360b7
uninstall method
chloeho7 Jul 18, 2023
2a770f0
uninstall method w logging
chloeho7 Jul 18, 2023
442c507
:hammer_and_wrench: apply pre-commit fixes
getsantry[bot] Jul 18, 2023
45a0acc
Merge branch 'master' into chloedisablenotifyslack
chloeho7 Jul 19, 2023
54a6bda
:hammer_and_wrench: apply pre-commit fixes
getsantry[bot] Jul 19, 2023
7a82dc4
fixing slack super init for failed testcases and renaming uninstall t…
chloeho7 Jul 19, 2023
1d25021
:hammer_and_wrench: apply pre-commit fixes
getsantry[bot] Jul 19, 2023
92c0140
:hammer_and_wrench: apply pre-commit fixes
getsantry[bot] Jul 19, 2023
dec6d4f
Merge branch 'master' into chloedisablenotifyslack
chloeho7 Jul 19, 2023
630c69a
starting test cases
chloeho7 Jul 19, 2023
cf83a27
:hammer_and_wrench: apply pre-commit fixes
getsantry[bot] Jul 19, 2023
ece9425
adding using flag
chloeho7 Jul 19, 2023
f14e25e
Merge branch 'master' into chloedisablenotifyslack
chloeho7 Jul 19, 2023
0930e4a
:hammer_and_wrench: apply pre-commit fixes
getsantry[bot] Jul 19, 2023
f5339d6
Merge branch 'master' into chloedisablenotifyslack
chloeho7 Jul 19, 2023
479f9dd
cleanining up comments and simplifying record requests
chloeho7 Jul 19, 2023
ed7b85c
:hammer_and_wrench: apply pre-commit fixes
getsantry[bot] Jul 19, 2023
17be7e4
:hammer_and_wrench: apply pre-commit fixes
getsantry[bot] Jul 19, 2023
2567334
slack account inactive test case
chloeho7 Jul 19, 2023
123c9d1
new fatal check
chloeho7 Jul 19, 2023
06af9bf
:hammer_and_wrench: apply pre-commit fixes
getsantry[bot] Jul 19, 2023
66acdaa
:hammer_and_wrench: apply pre-commit fixes
getsantry[bot] Jul 19, 2023
490ffba
:hammer_and_wrench: apply pre-commit fixes
getsantry[bot] Jul 19, 2023
905ec9c
trailing )
chloeho7 Jul 19, 2023
6226f14
syntax
chloeho7 Jul 19, 2023
e196abe
:hammer_and_wrench: apply pre-commit fixes
getsantry[bot] Jul 19, 2023
f0d8bbf
Merge branch 'master' into chloedisablenotifyslack
chloeho7 Jul 19, 2023
502f20c
test cases for errors and broken
chloeho7 Jul 20, 2023
5476e49
:hammer_and_wrench: apply pre-commit fixes
getsantry[bot] Jul 20, 2023
90fe510
:hammer_and_wrench: apply pre-commit fixes
getsantry[bot] Jul 20, 2023
812bbcf
:hammer_and_wrench: apply pre-commit fixes
getsantry[bot] Jul 20, 2023
e12d1d0
revising after review
chloeho7 Jul 20, 2023
de42ef3
:hammer_and_wrench: apply pre-commit fixes
getsantry[bot] Jul 20, 2023
e2faafd
Merge branch 'master' into chloedisablenotifyslack
chloeho7 Jul 20, 2023
cf07b83
:hammer_and_wrench: apply pre-commit fixes
getsantry[bot] Jul 20, 2023
ced71a7
working on setting up feature flag
chloeho7 Jul 20, 2023
eace13c
trying to seperate error and response recording
chloeho7 Jul 20, 2023
af5ad83
finishing feature flag
chloeho7 Jul 20, 2023
8312093
fixing feature flag and error check
chloeho7 Jul 20, 2023
d7318b8
fixing feature flag and error check
chloeho7 Jul 20, 2023
ed934bc
switch type to full baseapiresponse for recording
chloeho7 Jul 20, 2023
0b1fe04
add flag off testcase
chloeho7 Jul 20, 2023
211236e
finishing up tests
chloeho7 Jul 20, 2023
c7b5f07
cleaning up and fixing merge
chloeho7 Jul 20, 2023
4053ef4
Merge branch 'master' into chloedisablenotifyslack
chloeho7 Jul 20, 2023
6c170ef
:hammer_and_wrench: apply pre-commit fixes
getsantry[bot] Jul 20, 2023
8f5db70
:hammer_and_wrench: apply pre-commit fixes
getsantry[bot] Jul 20, 2023
2e207fe
is_response_error code
chloeho7 Jul 20, 2023
f08061d
cleaning up comments and updating docstring
chloeho7 Jul 20, 2023
b301b12
updating docstring and is_response_success
chloeho7 Jul 21, 2023
008d2b6
fix tests
ceorourke Jul 21, 2023
bb1a836
typing
ceorourke Jul 21, 2023
b296fb8
fixing conf/server diff and removing fatal flag
chloeho7 Jul 21, 2023
22de707
is_considered error and feature flag changes
chloeho7 Jul 21, 2023
46f6863
test cases for slow but not broken
chloeho7 Jul 21, 2023
ae31737
sign change
chloeho7 Jul 21, 2023
34f4c7e
Merge branch 'master' into chloedisablenotifyslack
chloeho7 Jul 24, 2023
1449055
fixing typing errors
chloeho7 Jul 24, 2023
7acf507
Merge branch 'master' into chloedisablenotifyslack
chloeho7 Jul 24, 2023
5f9ab43
fixing tests with new types
chloeho7 Jul 24, 2023
43a0ff1
Update base.py with rpc_integration is none case
chloeho7 Jul 24, 2023
2fd0ec2
Update base.py w is none
chloeho7 Jul 24, 2023
702d8d2
fixing typing
chloeho7 Jul 24, 2023
768731e
Update __init__.py
chloeho7 Jul 24, 2023
03a1936
Merge branch 'master' into chloedisablenotifyslack
chloeho7 Jul 24, 2023
3ca3e64
Merge branch 'master' into chloedisablenotifyslack
chloeho7 Jul 24, 2023
fad3435
Merge branch 'master' into chloedisablenotifyslack
chloeho7 Jul 25, 2023
feec59c
adding try catch for cluster init
chloeho7 Jul 26, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/sentry/conf/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ def env(
SENTRY_TRANSACTION_NAMES_REDIS_CLUSTER = "default"
SENTRY_WEBHOOK_LOG_REDIS_CLUSTER = "default"
SENTRY_ARTIFACT_BUNDLES_INDEXING_REDIS_CLUSTER = "default"
SENTRY_INTEGRATION_ERROR_LOG_REDIS_CLUSTER = "default"
SENTRY_DEBUG_FILES_REDIS_CLUSTER = "default"

# Hosts that are allowed to use system token authentication.
Expand Down Expand Up @@ -1348,6 +1349,8 @@ def SOCIAL_AUTH_DEFAULT_USERNAME() -> str:
"organizations:crons-timeline-listing-page": False,
# Enable usage of customer domains on the frontend
"organizations:customer-domains": False,
# Allow disabling integrations when broken is detected
"organizations:slack-disable-on-broken": False,
# Enable the 'discover' interface.
"organizations:discover": False,
# Enables events endpoint rate limit
Expand Down
2 changes: 2 additions & 0 deletions src/sentry/features/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -259,8 +259,10 @@
default_manager.add("organizations:pr-comment-bot", OrganizationFeature, FeatureHandlerStrategy.INTERNAL)
default_manager.add("organizations:ds-org-recalibration", OrganizationFeature, FeatureHandlerStrategy.INTERNAL)
default_manager.add("organizations:slack-use-new-lookup", OrganizationFeature, FeatureHandlerStrategy.REMOTE)
default_manager.add("organizations:slack-disable-on-broken", OrganizationFeature, FeatureHandlerStrategy.REMOTE)
default_manager.add("organizations:sourcemaps-bundle-flat-file-indexing", OrganizationFeature, FeatureHandlerStrategy.REMOTE)


# Project scoped features
default_manager.add("projects:alert-filters", ProjectFeature, FeatureHandlerStrategy.INTERNAL)
default_manager.add("projects:custom-inbound-filters", ProjectFeature, FeatureHandlerStrategy.INTERNAL)
Expand Down
2 changes: 1 addition & 1 deletion src/sentry/integrations/discord/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ def __init__(
org_integration_id = infer_org_integration(
integration_id=integration_id, ctx_logger=logger
)
super().__init__(org_integration_id, verify_ssl, logging_context)
super().__init__(integration_id, org_integration_id, verify_ssl, logging_context)

@control_silo_function
def authorize_request(self, prepared_request: PreparedRequest) -> PreparedRequest:
Expand Down
133 changes: 133 additions & 0 deletions src/sentry/integrations/request_buffer.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
import logging
from datetime import datetime, timedelta

from django.conf import settings

from sentry.utils import json, redis

BUFFER_SIZE = 30 # 30 days
KEY_EXPIRY = 60 * 60 * 24 * 30 # 30 days

IS_BROKEN_RANGE = 7 # 7 days


class IntegrationRequestBuffer:
"""
Create a data structure to store daily successful and failed request counts for each installed integration in Redis
This should store the aggregate counts of each type for last 30 days for each integration
"""

def __init__(self, key):
self.integrationkey = key

logger = logging.getLogger(__name__)

try:
cluster_id = settings.SENTRY_INTEGRATION_ERROR_LOG_REDIS_CLUSTER
self.client = redis.redis_clusters.get(cluster_id)
except KeyError as e:
logger.info("no_redis_cluster", extra={"error": e, "cluster_id": 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, buffer_key):
"""
Get the list at the buffer key.
"""

return self.client.lrange(buffer_key, 0, BUFFER_SIZE - 1)

def _get(self):
"""
Returns the list of daily aggregate error counts.
"""
return [
self._convert_obj_to_dict(obj) for obj in self._get_all_from_buffer(self.integrationkey)
]

def is_integration_broken(self):
"""
Integration is broken if we have 7 consecutive days of errors and no successes OR have a fatal error

"""
data = [
datetime.strptime(item.get("date"), "%Y-%m-%d").date()
for item in self._get()
if item.get("fatal_count", 0) > 0 and item.get("date")
][0:IS_BROKEN_RANGE]

if len(data) > 0:
return True

data = [
datetime.strptime(item.get("date"), "%Y-%m-%d").date()
for item in self._get()
if item.get("error_count", 0) > 0
and item.get("success_count", 0) == 0
and item.get("date")
][0:IS_BROKEN_RANGE]

if not len(data):
return False

if len(data) < IS_BROKEN_RANGE:
return False

date_set = {data[0] - timedelta(x) for x in range((data[0] - data[-1]).days)}
missing = list(date_set - set(data))
if len(missing):
return False
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.")

other_count1, other_count2 = list(set(VALID_KEYS).difference([count]))[0:2]
buffer_key = self.integrationkey
now = datetime.now().strftime("%Y-%m-%d")

pipe = self.client.pipeline()

# get first element from array
recent_item_array = self.client.lrange(buffer_key, 0, 1)
if len(recent_item_array):
recent_item = json.loads(recent_item_array[0])
if recent_item.get("date") == now:
recent_item[f"{count}_count"] += 1
pipe.lset(buffer_key, 0, json.dumps(recent_item))
else:
data = {
"date": now,
f"{count}_count": 1,
f"{other_count1}_count": 0,
f"{other_count2}_count": 0,
}
pipe.lpush(buffer_key, json.dumps(data))

else:
data = {
"date": now,
f"{count}_count": 1,
f"{other_count1}_count": 0,
f"{other_count2}_count": 0,
}
pipe.lpush(buffer_key, json.dumps(data))

pipe.expire(buffer_key, KEY_EXPIRY)
pipe.execute()

def record_error(self):
self.add("error")

def record_success(self):
self.add("success")

def record_fatal(self):
self.add("fatal")
16 changes: 13 additions & 3 deletions src/sentry/integrations/slack/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,12 @@ def __init__(
integration_id=self.integration_id, ctx_logger=logger
)

super().__init__(org_integration_id, verify_ssl, logging_context)
super().__init__(
org_integration_id=org_integration_id,
verify_ssl=verify_ssl,
integration_id=integration_id,
logging_context=logging_context,
)

@control_silo_function
def authorize_request(self, prepared_request: PreparedRequest) -> PreparedRequest:
Expand All @@ -57,13 +62,19 @@ def authorize_request(self, prepared_request: PreparedRequest) -> PreparedReques
if not integration:
logger.info("no_integration", extra={"path_url": prepared_request.path_url})
return prepared_request

token = (
integration.metadata.get("user_access_token") or integration.metadata["access_token"]
)
prepared_request.headers["Authorization"] = f"Bearer {token}"
return prepared_request

def is_response_fatal(self, response: Response) -> bool:
resp_json = response.json()
if not resp_json.get("ok"):
if "account_inactive" == resp_json.get("error", ""):
return True
return False

def track_response_data(
self,
code: Union[str, int],
Expand All @@ -74,7 +85,6 @@ def track_response_data(
# if no span was passed, create a dummy to which to add data to avoid having to wrap every
# span call in `if span`
span = span or Span()

try:
span.set_http_status(int(code))
except ValueError:
Expand Down
2 changes: 1 addition & 1 deletion src/sentry/integrations/slack/integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ class SlackIntegration(SlackNotifyBasicMixin, IntegrationInstallation):
def get_client(self) -> SlackClient:
if not self.org_integration:
raise IntegrationError("Organization Integration does not exist")
return SlackClient(org_integration_id=self.org_integration.id)
return SlackClient(org_integration_id=self.org_integration.id, integration_id=self.model.id)

def get_config_data(self) -> Mapping[str, str]:
metadata_ = self.model.metadata
Expand Down
12 changes: 10 additions & 2 deletions src/sentry/models/integrations/integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ def get_provider(self) -> IntegrationProvider:

def delete(self, *args, **kwds):
with outbox_context(
transaction.atomic(router.db_for_write(OrganizationIntegration)), flush=False
transaction.atomic(using=router.db_for_write(OrganizationIntegration)), flush=False
):
for organization_integration in self.organizationintegration_set.all():
organization_integration.delete()
Expand Down Expand Up @@ -107,7 +107,7 @@ def add_organization(self, organization: RpcOrganization, user=None, default_aut
from sentry.models import OrganizationIntegration

try:
with transaction.atomic(router.db_for_write(OrganizationIntegration)):
with transaction.atomic(using=router.db_for_write(OrganizationIntegration)):
org_integration, created = OrganizationIntegration.objects.get_or_create(
organization_id=organization.id,
integration_id=self.id,
Expand All @@ -134,3 +134,11 @@ def add_organization(self, organization: RpcOrganization, user=None, default_aut
},
)
return False

def disable(self):
"""
Disable this integration
"""

self.update(status=ObjectStatus.DISABLED)
self.save()
Loading