-
Notifications
You must be signed in to change notification settings - Fork 18
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
API Call tracking #348
base: main
Are you sure you want to change the base?
API Call tracking #348
Conversation
Looks good. Question is whether we want to do this logging in the DB itself or another DB. I'm worried about quickly blowing up the DB in size. Can we get an estimate of expected DB growth with the current load if this goes in? |
FWIW, all connections are logged in gcloud. This may still be the easiest route to getting the statistics you want, but worth seeing what they store. |
@profjsb Posting about 2000 API calls, I get a table size of 840 kB. So if we assume an API call a second on production, we would get 36 MB per day. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. I have a couple of suggestions.
app/handlers/base.py
Outdated
api_call = APICall( | ||
user_id=user_id, | ||
method=self.request.method, | ||
uri=self.request.uri.split("?")[0], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure, but maybe worth while to save the parameters as well? Maybe limit the number of characters, just in case we get super-long parameter strings, but those parameters could contain lots of useful info.
Also, consider clipping the beginning of the string (I don't know if URI contains the full address or just the thing after api/
)
method=self.request.method, | ||
uri=self.request.uri.split("?")[0], | ||
size=sizeof(data), | ||
success=False, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think if you stick something like self.start_time = time.time()
up in the prepare
method, you could also get the runtime of the query. That should work unless there's async stuff happening, in which case who knows what will happen... but it is worth trying out and comparing to timing data you get from the side calling the API.
This PR adds API Call tracking.