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

API Call tracking #348

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

API Call tracking #348

wants to merge 5 commits into from

Conversation

mcoughlin
Copy link
Collaborator

This PR adds API Call tracking.

@profjsb
Copy link
Collaborator

profjsb commented Jan 15, 2023

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?

@stefanv
Copy link
Contributor

stefanv commented Jan 15, 2023

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.

@mcoughlin
Copy link
Collaborator Author

@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.

Copy link
Collaborator

@guynir42 guynir42 left a 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.

api_call = APICall(
user_id=user_id,
method=self.request.method,
uri=self.request.uri.split("?")[0],
Copy link
Collaborator

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,
Copy link
Collaborator

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants