Skip to content

Commit b50875c

Browse files
authored
refac(discord): Update metrics and logging (#68819)
Update how we send metrics and handle logging during Discord responses. Using more standard metric events to alleviate logic handling in monitoring platforms. Success events end with `"success"`, and failures events with `"failure"`. We currently need to ignore specific user errors as a result of us not turning off base alert actions. Some alert actions are broken, because the user has deleted the channel/guild/team, or entered an improper id, but because these continue to execute, we continue to log and put them as part of our metrics. When the main problem is fixed, we will not see so many user errors come through. Resolves: https://github.com/getsentry/team-core-product-foundations/issues/218#issue-2200762819
1 parent d8f7510 commit b50875c

File tree

1 file changed

+71
-39
lines changed

1 file changed

+71
-39
lines changed

src/sentry/integrations/discord/client.py

Lines changed: 71 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import logging
44
from collections.abc import Mapping
5+
from typing import Any
56
from urllib.parse import urlencode
67

78
from rest_framework import status
@@ -43,6 +44,9 @@
4344
class DiscordClient(ApiClient):
4445
integration_name: str = "discord"
4546
base_url: str = DISCORD_BASE_URL
47+
_METRICS_FAILURE_KEY: str = "sentry.integrations.discord.failure"
48+
_METRICS_SUCCESS_KEY: str = "sentry.integrations.discord.success"
49+
_METRICS_USER_ERROR_KEY: str = "sentry.integrations.discord.failure.user_error"
4650

4751
def __init__(self):
4852
super().__init__()
@@ -118,13 +122,22 @@ def track_response_data(
118122
extra: Mapping[str, str] | None = None,
119123
) -> None:
120124
"""
121-
For all Discord api responses this:
122-
- Sends response metrics to Datadog
123-
- Sends response info to logs
125+
Handle response from Discord by logging and capturing metrics
124126
"""
125-
discord_error_code = "no_error_code"
126-
code_message = ""
127-
include_in_slo = True
127+
log_params = {
128+
"code": code,
129+
"error": str(error),
130+
"extra": extra,
131+
}
132+
133+
if self.integration_type:
134+
log_params[str(self.integration_type)] = self.name
135+
136+
try:
137+
logging_context = getattr(self, "logging_context", None)
138+
log_params["logging_context"] = logging_context
139+
except Exception:
140+
pass
128141

129142
is_ok = code in {
130143
status.HTTP_200_OK,
@@ -133,44 +146,63 @@ def track_response_data(
133146
status.HTTP_204_NO_CONTENT,
134147
}
135148

136-
if error:
137-
try:
138-
discord_error_response: dict = json.loads(resp.content.decode("utf-8")) or {} # type: ignore[union-attr]
139-
discord_error_code = str(discord_error_response.get("code", ""))
140-
if discord_error_code in DISCORD_ERROR_CODES:
141-
code_message = DISCORD_ERROR_CODES[discord_error_code]
142-
# These are excluded since they are not actionable from our side
143-
if discord_error_code in DISCORD_USER_ERRORS:
144-
include_in_slo = False
145-
except Exception:
146-
pass
149+
if not is_ok or error:
150+
self._handle_failure(log_params=log_params, resp=resp)
151+
else:
152+
self._handle_success(log_params=log_params)
153+
154+
def _handle_failure(
155+
self,
156+
log_params: dict[str, Any],
157+
resp: Response | None = None,
158+
) -> None:
159+
"""
160+
Do extra logic to handle an error from Discord
161+
"""
147162

163+
discord_error_response: dict | None = None
164+
if resp is not None:
165+
# Try to get the additional error code that Discord sent us to help determine what specific error happened
166+
try:
167+
discord_error_response = json.loads(resp.content.decode("utf-8"))
168+
log_params["discord_error_response"] = discord_error_response
169+
except Exception as err:
170+
self.logger.info(
171+
"error trying to handle discord error message", exc_info=err, extra=log_params
172+
)
173+
174+
discord_error_code = None
175+
if discord_error_response is not None:
176+
# Discord sends us a special code for errors in the response data
177+
# https://discord.com/developers/docs/topics/opcodes-and-status-codes#json
178+
discord_error_code = str(discord_error_response.get("code", ""))
179+
log_params["discord_error_code"] = discord_error_code
180+
181+
# Get the specific meaning for those codes
182+
if discord_error_code_message := DISCORD_ERROR_CODES.get(discord_error_code, None):
183+
log_params["discord_error_code_message"] = discord_error_code_message
184+
185+
# Check if the error is due to a user configuration error, which we do not have control over to fix
186+
# An example of this would be if the user deleted the discord guild and never updated the alert action
187+
is_user_error = discord_error_code in DISCORD_USER_ERRORS
188+
log_params["is_user_error"] = is_user_error
189+
190+
metrics_key = self._METRICS_USER_ERROR_KEY if is_user_error else self._METRICS_FAILURE_KEY
148191
metrics.incr(
149-
f"{self.metrics_prefix}.http_response",
192+
metrics_key,
150193
sample_rate=1.0,
151-
tags={
152-
str(self.integration_type): self.name,
153-
"status": code,
154-
"is_ok": is_ok,
155-
"include_in_slo": include_in_slo,
156-
"discord_code": discord_error_code,
157-
},
158194
)
195+
self.logger.info("handled discord error", extra=log_params)
159196

160-
log_params = {
161-
**(extra or {}),
162-
"status_string": str(code),
163-
"error": str(error)[:256] if error else None,
164-
"include_in_slo": include_in_slo,
165-
"discord_code": discord_error_code,
166-
"code_message": code_message if error else None,
167-
}
168-
169-
if self.integration_type:
170-
log_params[self.integration_type] = self.name
171-
172-
log_params.update(getattr(self, "logging_context", None) or {})
173-
self.logger.info("%s.http_response", self.integration_type, extra=log_params)
197+
def _handle_success(
198+
self,
199+
log_params: dict[str, Any],
200+
) -> None:
201+
metrics.incr(
202+
self._METRICS_SUCCESS_KEY,
203+
sample_rate=1.0,
204+
)
205+
self.logger.info("handled discord success", extra=log_params)
174206

175207
def send_message(
176208
self, channel_id: str, message: DiscordMessageBuilder, notification_uuid: str | None = None

0 commit comments

Comments
 (0)