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

🔧 chore: remove redundant slack logging & metrics #82852

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

iamrajjoshi
Copy link
Member

continuing #82744

we have completed the audit of our integration slos, so going back and cleaning up redundant logging and metrics.

we should also monitor our DD monitors to see if they need updating.

@iamrajjoshi iamrajjoshi self-assigned this Jan 3, 2025
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Jan 3, 2025
Copy link

codecov bot commented Jan 3, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

✅ All tests successful. No failed tests found.

Additional details and impacted files
@@                        Coverage Diff                        @@
##           raj/clean-slack-slo-logging-1   #82852      +/-   ##
=================================================================
- Coverage                          87.50%   87.50%   -0.01%     
=================================================================
  Files                               9410     9410              
  Lines                             536814   536793      -21     
  Branches                           21049    21049              
=================================================================
- Hits                              469724   469699      -25     
- Misses                             66721    66725       +4     
  Partials                             369      369              

@iamrajjoshi iamrajjoshi marked this pull request as ready for review January 3, 2025 13:58
@iamrajjoshi iamrajjoshi requested review from a team as code owners January 3, 2025 13:58
Base automatically changed from raj/clean-slack-slo-logging-1 to master January 3, 2025 20:15
@iamrajjoshi iamrajjoshi requested a review from a team as a code owner January 3, 2025 20:15
Comment on lines -101 to -108
logger.error(".unlink-user.no-identity.error", extra={"slack_request": slack_request})
metrics.incr(self._METRIC_FAILURE_KEY + "unlink_user.no_identity", sample_rate=1.0)

return self.reply(slack_request, NOT_LINKED_MESSAGE)

if not (slack_request.integration and slack_request.user_id and slack_request.channel_id):
logger.error(".unlink-user.bad_request.error", extra={"slack_request": slack_request})
metrics.incr(self._METRIC_FAILURE_KEY + "unlink_user.bad_request", sample_rate=1.0)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's update our webhook metrics dashboards for webhooks before merging this please!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Scope: Backend Automatically applied to PRs that change backend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants