Skip to content
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

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

Merged
merged 101 commits into from
Jul 25, 2023

Conversation

chloeho7
Copy link
Contributor

@chloeho7 chloeho7 commented Jul 17, 2023

Continuing #51126
Milestone 1 of Notify on Disabled Integration project

Implementing IntegrationRequestBuffer using Redis for logging errors and detecting broken integrations to be disabled. Request buffer stores daily aggregate error, fatal and success counts for an integration for 30 days. If an integration consistently fails for 7 days or has a fatal response, then the integration is broken. Later users will be alerted of broken integrations with an email or in-app.

Disabling implemented under feature flag "slack-disable-on-broken".
Conditions for logging Implemented in BaseApiClient and SlackClient.

@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Jul 17, 2023
@codecov
Copy link

codecov bot commented Jul 17, 2023

Codecov Report

Merging #52994 (3ca3e64) into master (4ceafc4) will increase coverage by 3.46%.
The diff coverage is 73.17%.

❗ Current head 3ca3e64 differs from pull request most recent head fad3435. Consider uploading reports for the commit fad3435 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #52994      +/-   ##
==========================================
+ Coverage   76.07%   79.54%   +3.46%     
==========================================
  Files        4942     4942              
  Lines      209107   208946     -161     
  Branches    35613    35593      -20     
==========================================
+ Hits       159082   166206    +7124     
+ Misses      44779    37662    -7117     
+ Partials     5246     5078     -168     
Files Changed Coverage Δ
src/sentry/models/integrations/integration.py 92.18% <50.00%> (-2.90%) ⬇️
src/sentry/shared_integrations/client/base.py 84.00% <65.93%> (+5.38%) ⬆️
src/sentry/integrations/request_buffer.py 80.70% <80.70%> (ø)
src/sentry/conf/server.py 91.47% <100.00%> (+0.01%) ⬆️
src/sentry/features/__init__.py 100.00% <100.00%> (ø)
src/sentry/integrations/discord/client.py 100.00% <100.00%> (ø)
src/sentry/integrations/slack/client.py 95.83% <100.00%> (+0.37%) ⬆️
src/sentry/integrations/slack/integration.py 91.66% <100.00%> (ø)
src/sentry/shared_integrations/client/proxy.py 98.46% <100.00%> (+1.53%) ⬆️

... and 529 files with indirect coverage changes

@chloeho7 chloeho7 enabled auto-merge (squash) July 25, 2023 16:14
@chloeho7 chloeho7 merged commit 75d6446 into master Jul 25, 2023
55 checks passed
@chloeho7 chloeho7 deleted the chloedisablenotifyslack branch July 25, 2023 16:42
chloeho7 added a commit that referenced this pull request Jul 25, 2023
…oken (#52994)

Continuing #51126
Milestone 1 of [Notify on Disabled Integration project
](https://www.notion.so/sentry/Tech-Spec-Notify-on-Disabled-Integration-Spec-e7ea0f86ccd6419cb3e564067cf4a2ef?pvs=4
)

Implementing `IntegrationRequestBuffer` using Redis for logging errors
and detecting broken integrations to be disabled. Request buffer stores
daily aggregate error, fatal and success counts for an integration for
30 days. If an integration consistently fails for 7 days or has a fatal
response, then the integration is broken. Later users will be alerted of
broken integrations with an email or in-app.

Disabling implemented under feature flag `"disable-on-broken"`.
Conditions for logging Implemented in `BaseApiClient` and `SlackClient`.

---------

Co-authored-by: Colleen O'Rourke <[email protected]>
ceorourke added a commit that referenced this pull request Jul 25, 2023
@scefali scefali added the Trigger: Revert add to a merged PR to revert it (skips CI) label Jul 25, 2023
@getsentry-bot
Copy link
Contributor

PR reverted: 8ae2b10

getsentry-bot added a commit that referenced this pull request Jul 25, 2023
@chloeho7 chloeho7 restored the chloedisablenotifyslack branch July 26, 2023 16:30
chloeho7 added a commit that referenced this pull request Jul 27, 2023
…oken (#53621)

Continuing #52994 after revert

> 
> Continuing #51126
> Milestone 1 of [Notify on Disabled Integration
project](https://www.notion.so/sentry/Tech-Spec-Notify-on-Disabled-Integration-Spec-e7ea0f86ccd6419cb3e564067cf4a2ef?pvs=4)
> 
> Implementing IntegrationRequestBuffer using Redis for logging errors
and detecting broken integrations to be disabled. Request buffer stores
daily aggregate error, fatal and success counts for an integration for
30 days. If an integration consistently fails for 7 days or has a fatal
response, then the integration is broken. Later users will be alerted of
broken integrations with an email or in-app.
> 
> Disabling implemented under feature flag "slack-disable-on-broken".
> Conditions for logging Implemented in BaseApiClient and SlackClient.

---------

Co-authored-by: getsantry[bot] <66042841+getsantry[bot]@users.noreply.github.com>
Co-authored-by: Colleen O'Rourke <[email protected]>
@github-actions github-actions bot locked and limited conversation to collaborators Aug 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Scope: Backend Automatically applied to PRs that change backend components Trigger: Revert add to a merged PR to revert it (skips CI)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants