re(api): Initial attempt at api access logging #28741
Conversation
From discussions last week. We are going to log api access information and leverage logging services to create metrics. This commit has some initial metrics to be logged with more that can be added later
| if ( | ||
| getattr(self.request.successful_authenticator, "token_name", None) | ||
| == TokenAuthentication.token_name | ||
| ): | ||
| request._metric_tags["backend_request"] = True | ||
|
|
There was a problem hiding this comment.
I see that you added this earlier, are we just no longer using it?
There was a problem hiding this comment.
From looking at datadog the last week, it more looks like this didn't work. We're now doing the same thing with logging, so I don't see a point of keeping this around.
src/sentry/api/base.py
Outdated
| token_type = getattr(self.request.auth, "__class__", None) | ||
| log_metrics = dict( | ||
| method=self.request.method, | ||
| view=str(self.request.parser_context["view"].__class__), |
There was a problem hiding this comment.
Is there ever a case where "view" isn't present?
There was a problem hiding this comment.
Not sure. If I add a try/catch we can find out :)
| @@ -160,6 +161,20 @@ def load_json_body(self, request): | |||
| except json.JSONDecodeError: | |||
| return | |||
|
|
|||
| def _create_api_access_log(self): | |||
There was a problem hiding this comment.
Maybe being a bit paranoid, but it might be worth just wrapping this in a try/catch to make sure this logging never breaks a request
There was a problem hiding this comment.
I actually really like this. I'll log an error in the api access log so we can see what went wrong.
| method=self.request.method, | ||
| view=str(self.request.parser_context["view"].__class__), | ||
| response=self.response.status_code, | ||
| user=str(self.request.user), |
There was a problem hiding this comment.
What will the user be if something like an integration or sentry app calls the API? Also, is there anyway to identify where the request came from if the user is AnonymousUser like how you showed in your example?
Just making sure we can use this to identify when users hammer our API.
There was a problem hiding this comment.
For the first question, I don't know what the user will be, but we will after we start logging it :)
There might be a way to track the IP address of the caller, but generally if we're getting an AnonymousUser call then that means it's an unauthenticated user. The objective here is to add more detail in subsequent iterations.
src/sentry/api/base.py
Outdated
| ) | ||
| api_access_logger.info(log_metrics) | ||
| except Exception as exc: | ||
| api_access_logger.error( |
There was a problem hiding this comment.
Let's use .exception here so that we capture the actual exception too. Then you don't need to include exc in your string format.
From discussions last week. We are going to log api access information and leverage logging services to create metrics. This commit has some initial metrics to be logged with more that can be added later
Log messages will look like: