Don't look up approval candidates when no approval is required

[iainlane]

We always check the approval candidates when evaluating an approval policy. This process ends up making an API call to list the comments on the PR, so that the approvers given in the policy can be checked against the commenters or reviewers on the PR and only the relevant people considered.

This search currently happens regardless of whether any approvals are required. If there aren't any then we can save the API calls and a bit of time by skipping the search. PRs can be evaluated fairly frequently depending on the policy, so this can add up to a lot of API calls.

I noticed this because we had a problem yesterday where we somehow managed to run out of GitHub API quota which I didn't think would be reasonably possible 😱 . This query stood out when I checked the logs, but I'm not sure it's the end of the story.

Hopefully I'm not missing a reason why this was the way it was. :-)

[bluekeyes]

I noticed this because we had a problem yesterday where we somehow managed to run out of GitHub API quota which I didn't think would be reasonably possible 😱 . This query stood out when I checked the logs, but I'm not sure it's the end of the story.

Policy Bot can be pretty request-heavy. I think we've done most of the big optimizations, but I'm sure there's more stuff like this that we've missed.

Another note is that GitHub App rate limits scale based on users and repositories in the organization (up to 12,500 requests/hour.) If you work primarily in a monorepo, you may have a lower rate limit than you would if an equivalent level of development was spread across multiple repositories. GitHub Support may be able to make a custom adjustment to your rate limit in that case.

[iainlane]

I noticed this because we had a problem yesterday where we somehow managed to run out of GitHub API quota which I didn't think would be reasonably possible 😱 . This query stood out when I checked the logs, but I'm not sure it's the end of the story.

Policy Bot can be pretty request-heavy. I think we've done most of the big optimizations, but I'm sure there's more stuff like this that we've missed.

One other one that I've noticed we could do is to allow the default repository to be disabled. It seems like 404s aren't cached and so we do a lot of requests to check for grafana/.github having a policy and find it doesn't every time. I'll send over a PR for that shortly. (edit: this was merged in https://github.com/palantir/policy-bot/commit/550e0326d417032a6a7ca64010e592913e0f3cda - thanks!)

Another note is that GitHub App rate limits scale based on users and repositories in the organization (up to 12,500 requests/hour.) If you work primarily in a monorepo, you may have a lower rate limit than you would if an equivalent level of development was spread across multiple repositories. GitHub Support may be able to make a custom adjustment to your rate limit in that case.

Cheers for the tip. For us, it's not only one monorepo that drives activity. It is one of those but also a few of the larger OSS repos we have.

Looking into what happened a bit more, I noticed a few things:

• As reported by the Prometheus metrics from the app, there are two rate limits in play: one with our installation ID of 15,000 (we are on GitHub Enterprise Cloud) and another with no installation ID (0) of 12,500
• It's that second one which we exhausted. The installation's rate limit did get used up proportionally but it bottomed out at 5,300 that hour. Bear in mind we were erroring so we probably would have made more requests if the other one hadn't been hit.
• I'm not sure what that 12,500 rate limit is, actually! The docs don't mention a rate limit for non-installation (JWT) requests but perhaps it is that. Maybe this could help me focus where I'm looking in the code and logs? Just from a quick look that might mean the installations service.
• This happening is fairly anomalous. Here's a screenshot of the time when this happened. I can see one or two other similar instances if I look back further, but never this steep. I didn't manage to pin down a cause yet, but I would suspect it's due to a particular pattern of activity in the org's repos.

I'm sending in fixes for what I can see and then continuing to observe the effects they have!

Just a brain dump 🙂

[bluekeyes]

The requests with no installation ID are interesting. It could be application requests using the JWT, but I don't think we're making many of those. If you have debug logging enabled, it might be helpful to look at the logs with the github_request message during that period to see if they also show the requests with no installation ID.

It's also possible we're not correctly attaching the installation ID to the metrics in some situations, but the fact that the maximum limit is different from your actual installation makes that seem less likely.