Skip to content

Commit 73a7070

Browse files
prattmicgopherbot
authored andcommitted
gopls/internal/telemetry/cmd/stacks: paginate issue search
The GitHub API returns at most 100 results per page, but gopls already has 101 issues, so we are dropping issues and need pagination. The GitHub search API has a hard limit of 1000 results (https://docs.github.com/en/rest/search/search?apiVersion=2022-11-28#about-search), which we'll hit eventually. As an alternative, use the "List repository issues" API (https://docs.github.com/en/rest/issues/issues?apiVersion=2022-11-28#list-repository-issues). This allows filtering by label, which is all we need. Note that the old code was mistakenly searching all of GitHub, not just golang/go. That is now fixed. GitHub pagination uses an awkward header format (https://docs.github.com/en/rest/using-the-rest-api/using-pagination-in-the-rest-api?apiVersion=2022-11-28), but it is ultimately just ?page=1, ?page=2, etc, so we just keep trying new pages until we get no more results. Updates golang/go#71045. Change-Id: I6a6a636ccc17c9e1b1024369f98965f59456896a Reviewed-on: https://go-review.googlesource.com/c/tools/+/642436 Auto-Submit: Michael Pratt <[email protected]> Reviewed-by: Alan Donovan <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]>
1 parent fec8580 commit 73a7070

File tree

1 file changed

+54
-34
lines changed

1 file changed

+54
-34
lines changed

gopls/internal/telemetry/cmd/stacks/stacks.go

Lines changed: 54 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -391,19 +391,14 @@ func readReports(pcfg ProgramConfig, days int) (stacks map[string]map[Info]int64
391391
// predicates.
392392
func readIssues(pcfg ProgramConfig) ([]*Issue, error) {
393393
// Query GitHub for all existing GitHub issues with the report label.
394-
//
395-
// TODO(adonovan): by default GitHub returns at most 30
396-
// issues; we have lifted this to 100 using per_page=%d, but
397-
// that won't work forever; use paging.
398-
query := fmt.Sprintf("is:issue label:%s", pcfg.SearchLabel)
399-
res, err := searchIssues(query)
394+
issues, err := searchIssues(pcfg.SearchLabel)
400395
if err != nil {
401-
log.Fatalf("GitHub issues query %q failed: %v", query, err)
396+
log.Fatalf("GitHub issues label %q search failed: %v", pcfg.SearchLabel, err)
402397
}
403398

404399
// Extract and validate predicate expressions in ```#!stacks...``` code blocks.
405400
// See the package doc comment for the grammar.
406-
for _, issue := range res.Items {
401+
for _, issue := range issues {
407402
block := findPredicateBlock(issue.Body)
408403
if block != "" {
409404
expr, err := parser.ParseExpr(block)
@@ -480,7 +475,7 @@ func readIssues(pcfg ProgramConfig) ([]*Issue, error) {
480475
}
481476
}
482477

483-
return res.Items, nil
478+
return issues, nil
484479
}
485480

486481
// claimStack maps each stack ID to its issue (if any).
@@ -798,28 +793,58 @@ func frameURL(pclntab map[string]FileLine, info Info, frame string) string {
798793
// -- GitHub search --
799794

800795
// searchIssues queries the GitHub issue tracker.
801-
func searchIssues(query string) (*IssuesSearchResult, error) {
802-
q := url.QueryEscape(query)
796+
func searchIssues(label string) ([]*Issue, error) {
797+
label = url.QueryEscape(label)
803798

804-
req, err := http.NewRequest("GET", "https://api.github.com/search/issues?q="+q+"&per_page=100", nil)
805-
if err != nil {
806-
return nil, err
807-
}
808-
req.Header.Add("Authorization", "Bearer "+authToken)
809-
resp, err := http.DefaultClient.Do(req)
810-
if err != nil {
811-
return nil, err
812-
}
813-
defer resp.Body.Close()
814-
if resp.StatusCode != http.StatusOK {
815-
body, _ := io.ReadAll(resp.Body)
816-
return nil, fmt.Errorf("search query failed: %s (body: %s)", resp.Status, body)
799+
// Slurp all issues with the telemetry label.
800+
//
801+
// The pagination link headers have an annoying format, but ultimately
802+
// are just ?page=1, ?page=2, etc with no extra state. So just keep
803+
// trying new pages until we get no more results.
804+
//
805+
// NOTE: With this scheme, GitHub clearly has no protection against
806+
// race conditions, so presumably we could get duplicate issues or miss
807+
// issues across pages.
808+
809+
getPage := func(page int) ([]*Issue, error) {
810+
url := fmt.Sprintf("https://api.github.com/repos/golang/go/issues?state=all&labels=%s&per_page=100&page=%d", label, page)
811+
req, err := http.NewRequest("GET", url, nil)
812+
if err != nil {
813+
return nil, err
814+
}
815+
req.Header.Add("Authorization", "Bearer "+authToken)
816+
resp, err := http.DefaultClient.Do(req)
817+
if err != nil {
818+
return nil, err
819+
}
820+
defer resp.Body.Close()
821+
if resp.StatusCode != http.StatusOK {
822+
body, _ := io.ReadAll(resp.Body)
823+
return nil, fmt.Errorf("search query %s failed: %s (body: %s)", url, resp.Status, body)
824+
}
825+
var r []*Issue
826+
if err := json.NewDecoder(resp.Body).Decode(&r); err != nil {
827+
return nil, err
828+
}
829+
830+
return r, nil
817831
}
818-
var result IssuesSearchResult
819-
if err := json.NewDecoder(resp.Body).Decode(&result); err != nil {
820-
return nil, err
832+
833+
var results []*Issue
834+
for page := 1; ; page++ {
835+
r, err := getPage(page)
836+
if err != nil {
837+
return nil, err
838+
}
839+
if len(r) == 0 {
840+
// No more results.
841+
break
842+
}
843+
844+
results = append(results, r...)
821845
}
822-
return &result, nil
846+
847+
return results, nil
823848
}
824849

825850
// updateIssueBody updates the body of the numbered issue.
@@ -882,12 +907,7 @@ func addIssueComment(number int, comment string) error {
882907
return nil
883908
}
884909

885-
// See https://developer.github.com/v3/search/#search-issues.
886-
887-
type IssuesSearchResult struct {
888-
TotalCount int `json:"total_count"`
889-
Items []*Issue
890-
}
910+
// See https://docs.github.com/en/rest/issues/issues?apiVersion=2022-11-28#list-repository-issues.
891911

892912
type Issue struct {
893913
Number int

0 commit comments

Comments
 (0)