Feat-Implement httpcache middleware for GitHub API #203

naveensrinivasan · 2021-02-21T14:39:19Z

Please check if the PR fulfills these requirements

Tests for the changes have been added (for bug fixes/features)

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
Feature - Caching
What is the current behavior? (You can also link to an open issue here)
Scorecard scalability limitation: Reduce GitHub API calls #80 Reducing GitHub API calls to scale scanning repositories #202
What is the new behavior (if this is a feature change)?
The GitHub API supports conditional requests
https://docs.github.com/en/rest/overview/resources-in-the-rest-api#conditional-requests

https://github.com/google/go-github supports Conditional requests
https://github.com/google/go-github#conditional-requests

As we are scaling more and more projects this would add a lot of value.

Initial run fetches information using httpcache as a middleware,
which caches the HTTP response initially in a large disk (PVC),
probably move to Redis later as a cache instead of disk.

Subsequent cron runs will utilize the httpcache for checking content modification and
load it from the cache if it isn't modified, which reduces the hitting the
Rate Limit of the GitHub API.

Also fixed the golang-ci warnings.

Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)
None
Other information:

Subsequent cache runs on 50 repositories takes about 18 minutes with 3 GitHub tokens

❯ time ./cron.sh
github.com/18F/identity-idp
github.com/1N3/Sn1per
github.com/24pullrequests/24pullrequests
github.com/AElfProject/AElf
github.com/Activiti/Activiti
github.com/AdguardTeam/AdguardFilters
github.com/AlchemyCMS/alchemy_cms
github.com/AliasIO/wappalyzer
github.com/Amanieu/parking_lot
github.com/abonas/kubeclient
github.com/abpframework/abp
github.com/acassen/keepalived
github.com/acmesh-official/acme.sh
github.com/actions/runner
github.com/activeadmin/activeadmin
github.com/activemerchant/active_merchant
github.com/activerecord-hackery/ransack
github.com/activescaffold/active_scaffold
github.com/actix/actix
github.com/actix/actix-extras
github.com/actix/actix-net
github.com/actix/actix-web
github.com/adafruit/Adafruit_CircuitPython_Bundle
github.com/adempiere/adempiere
github.com/afragen/github-updater
github.com/aframevr/aframe
github.com/ai/autoprefixer-rails
github.com/aio-libs/aiohttp
github.com/airbnb/javascript
github.com/airbrake/airbrake
github.com/akaunting/akaunting
github.com/akeneo/pim-community-dev
github.com/akka/akka
github.com/akka/akka-http
github.com/akkadotnet/akka.net
github.com/alacritty/alacritty
github.com/alanmcgovern/monotorrent
github.com/alexcrichton/cc-rs
github.com/alexcrichton/curl-rust
github.com/alexcrichton/proc-macro2
github.com/alexcrichton/toml-rs
github.com/alexreisner/geocoder
github.com/alibaba/dragonwell8
github.com/alibaba/druid
github.com/alibaba/fastjson
github.com/alibaba/nacos
github.com/allenai/allennlp
github.com/allinurl/goaccess
github.com/alphagov/whitehall
github.com/alpinelinux/aports
./cron.sh  46.68s user 19.95s system 6% cpu 18:26.70 total

Folder size

du -sh ./cache
801M

Files in the folder

find ./cache -type f | wc -l
10347

naveensrinivasan · 2021-02-21T14:40:28Z

go.mod

@@ -7,10 +7,11 @@ require (
 	github.com/golang/protobuf v1.4.3 // indirect
 	github.com/google/go-github/v32 v32.1.0
 	github.com/kr/text v0.2.0 // indirect
+	github.com/naveensrinivasan/httpcache v1.2.1


This is a fork of gregjones/httpcache#104 this PR.

github-actions · 2021-02-21T14:41:15Z

Integration tests failure for 35f3b88dbb4e5dad84ca2916dc6db3d8e7e32d05

github-actions · 2021-02-21T14:43:19Z

Integration tests success for 35f3b88dbb4e5dad84ca2916dc6db3d8e7e32d05

github-actions · 2021-02-21T16:54:41Z

Integration tests success for 73ebc1a54963240d4ff9241dce169df0f5131478

github-actions · 2021-02-21T23:49:14Z

Integration tests failure for e5b609b096fc67b393e928b3245a122f3919ef31

github-actions · 2021-02-21T23:52:28Z

Integration tests success for e5b609b096fc67b393e928b3245a122f3919ef31

dlorenc

A couple tiny nits, LGTM!

dlorenc · 2021-02-22T16:24:35Z

README.md

@@ -153,6 +152,20 @@ Signed-Releases: Fail 0
 Signed-Tags: Fail 10
 ```

+### Caching
+
+Scorecard uses `httpcache` with <https://docs.github.com/en/rest/overview/resources-in-the-rest-api#conditional-requests> for caching httpresponse. The default cache is in-memory.


Maybe link to the etags stuff in GitHub - the real benefit is avoiding the API quota

dlorenc · 2021-02-22T16:25:48Z

roundtripper/roundtripper.go

+// shouldUseDiskCache checks the env variables USE_DISK_CACHE and DISK_CACHE_PATH to determine if
+// disk should be used for caching.
+func shouldUseDiskCache() (string, bool) {
+	if isDiskCache := os.Getenv(UseDiskCache); isDiskCache != "" {


nit: I think you can avoid this if statement and go straight into ParseBool since "" parses as false.

inferno-chromium

Thanks a lot! This is very exciting.

inferno-chromium · 2021-02-22T16:21:16Z

.gitignore

@@ -23,3 +23,6 @@ results.json

 # tools 
 bin
+
+#temp


remove this.

inferno-chromium · 2021-02-22T16:29:58Z

README.md

+
+To use disk cache two env variables have to be set `USE_DISK_CACHE=true` and `DISK_CACHE_PATH=./cache`.
+
+There are not TTL on cache.


"There is no"

inferno-chromium · 2021-02-22T16:31:01Z

roundtripper/roundtripper.go

+			}
+		}
+	}
+	return "", false


nit: maybe nil instead of ""

can't do nil for a string in go

inferno-chromium · 2021-02-22T16:32:36Z

roundtripper/roundtripper_test.go

+	t.Parallel()
+	tests := []struct {
+		name  string
+		want  string


s/want/diskCachePath
s/want1/useDiskCache

The GitHub API supports conditional requests https://docs.github.com/en/rest/overview/resources-in-the-rest-api#conditional-requests https://github.com/google/go-github supports Conditional requests https://github.com/google/go-github#conditional-requests As we are scaling more and more projects this would add a lot of value. Initial run fetches information using `httpcache` as a middleware, which caches the HTTP response initially in a large disk (PVC), probably move to Redis later as a cache instead of disk. Subsequent `cron runs` will utilize the `httpcache` for checking content modification and load it from the cache if it isn't modified, which reduces the hitting the Rate Limit of the GitHub API.

github-actions · 2021-02-22T17:18:19Z

Integration tests success for 9645825ac6716a3a988b682ddc00145bb62695df

* set GITHUB_TOKEN as default token * updates * Update doc * Update doc * updates * updates * update * update * update * update * updates

naveensrinivasan requested review from dlorenc and inferno-chromium February 21, 2021 14:39

naveensrinivasan self-assigned this Feb 21, 2021

naveensrinivasan commented Feb 21, 2021

View reviewed changes

naveensrinivasan mentioned this pull request Feb 21, 2021

Move to more reliable and scalable caching solution #205

Closed

dlorenc reviewed Feb 22, 2021

View reviewed changes

inferno-chromium approved these changes Feb 22, 2021

View reviewed changes

naveensrinivasan force-pushed the feat/caching branch from e5b609b to 9645825 Compare February 22, 2021 17:15

naveensrinivasan enabled auto-merge (squash) February 22, 2021 17:17

naveensrinivasan merged commit db81680 into main Feb 22, 2021

naveensrinivasan mentioned this pull request May 10, 2021

🌱 Setup PubSub framework code #428

Merged

2 tasks

naveensrinivasan deleted the feat/caching branch January 14, 2022 22:52

justaugustus pushed a commit to justaugustus/scorecard that referenced this pull request May 25, 2022

✨ Update documentation (ossf#203)

1c09180

* set GITHUB_TOKEN as default token * updates * Update doc * Update doc * updates * updates * update * update * update * update * updates

@@ @@ -23,3 +23,6 @@ results.json @@
               # tools
               bin
+              #temp


		To use disk cache two env variables have to be set `USE_DISK_CACHE=true` and `DISK_CACHE_PATH=./cache`.

		There are not TTL on cache.

Feat-Implement httpcache middleware for GitHub API #203

Feat-Implement httpcache middleware for GitHub API #203

Uh oh!

Conversation

naveensrinivasan commented Feb 21, 2021

Folder size

Files in the folder

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Feb 21, 2021

Uh oh!

github-actions bot commented Feb 21, 2021

Uh oh!

github-actions bot commented Feb 21, 2021

Uh oh!

github-actions bot commented Feb 21, 2021

Uh oh!

github-actions bot commented Feb 21, 2021

Uh oh!

dlorenc left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

inferno-chromium left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Feb 22, 2021

Uh oh!

Uh oh!