Skip to content

Commit 1472007

Browse files
committed
perf(github): reduce GetTektonDir from 3 REST calls to 1 GraphQL
Fetch .tekton directory tree with inline blob contents in single GraphQL query instead of 3 sequential API calls. Since processing any supported event requires fetching the tekton directory, GetTektonDir is a hot path and API calls can add up before without any PipelineRuns executing. Reducing the load in this hot path has an outsized impact. In some cases I saw, this change would have eliminated over 1000 API calls in an hour for a single repository. Assisted-By: Claude Sonnet 4.5 <noreply@anthropic.com>
1 parent 861a507 commit 1472007

5 files changed

Lines changed: 627 additions & 460 deletions

File tree

pkg/provider/github/github.go

Lines changed: 31 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -375,11 +375,10 @@ func (v *Provider) GetCommitStatuses(_ context.Context, _ *info.Event) ([]provid
375375

376376
// GetTektonDir retrieves all YAML files from the .tekton directory and returns them as a single concatenated multi-document YAML file.
377377
func (v *Provider) GetTektonDir(ctx context.Context, runevent *info.Event, path, provenance string) (string, error) {
378-
tektonDirSha := ""
379-
380378
v.provenance = provenance
381-
// default set provenance from the SHA
382379
revision := runevent.SHA
380+
381+
// For default_branch, get the branch SHA first (keep REST API call for simplicity)
383382
if provenance == "default_branch" {
384383
v.Logger.Infof("Using PipelineRun definition from default_branch: %s", runevent.DefaultBranch)
385384
branch, _, err := wrapAPI(v, "get_default_branch", func() (*github.Branch, *github.Response, error) {
@@ -400,38 +399,43 @@ func (v *Provider) GetTektonDir(ctx context.Context, runevent *info.Event, path,
400399
v.Logger.Infof("Using PipelineRun definition from source %s %s on commit SHA %s", runevent.TriggerTarget.String(), prInfo, runevent.SHA)
401400
}
402401

403-
rootobjects, _, err := wrapAPI(v, "get_root_tree", func() (*github.Tree, *github.Response, error) {
404-
return v.Client().Git.GetTree(ctx, runevent.Organization, runevent.Repository, revision, false)
405-
})
402+
// Try GraphQL - fetches tekton tree + blob contents in single request
403+
if v.graphQLClient == nil {
404+
var err error
405+
v.graphQLClient, err = newGraphQLClient(v)
406+
if err != nil {
407+
return "", fmt.Errorf("failed to create GraphQL client: %w", err)
408+
}
409+
}
410+
411+
result, err := v.graphQLClient.fetchTektonDirGraphQL(ctx, runevent.Organization, runevent.Repository, revision, path)
406412
if err != nil {
407413
return "", err
408414
}
409-
for _, object := range rootobjects.Entries {
410-
if object.GetPath() == path {
411-
if object.GetType() != "tree" {
412-
return "", fmt.Errorf("%s has been found but is not a directory", path)
413-
}
414-
tektonDirSha = object.GetSHA()
415-
}
416-
}
417415

418-
// If we didn't find a .tekton directory then just silently ignore the error.
419-
if tektonDirSha == "" {
416+
// If no yaml files found, return empty (directory doesn't exist or is empty)
417+
if len(result.FileContents) == 0 {
420418
return "", nil
421419
}
422420

423-
// Get all files in the .tekton directory recursively
424-
// there is a limit on this recursive calls to 500 entries, as documented here:
425-
// https://docs.github.com/en/rest/reference/git#get-a-tree
426-
// so we may need to address it in the future.
427-
tektonDirObjects, _, err := wrapAPI(v, "get_tekton_tree", func() (*github.Tree, *github.Response, error) {
428-
return v.Client().Git.GetTree(ctx, runevent.Organization, runevent.Repository, tektonDirSha,
429-
true)
430-
})
431-
if err != nil {
432-
return "", err
421+
// Concatenate YAML files (result already filtered to .yaml/.yml blobs)
422+
var buf strings.Builder
423+
for filePath, content := range result.FileContents {
424+
// Validate YAML
425+
if err := provider.ValidateYaml(content, filePath); err != nil {
426+
return "", err
427+
}
428+
429+
// Concatenate with separator
430+
if buf.Len() > 0 && !strings.HasPrefix(string(content), "---") {
431+
buf.WriteString("---")
432+
}
433+
buf.WriteString("\n")
434+
buf.Write(content)
435+
buf.WriteString("\n")
433436
}
434-
return v.concatAllYamlFiles(ctx, tektonDirObjects.Entries, runevent, path, revision)
437+
438+
return buf.String(), nil
435439
}
436440

437441
// GetCommitInfo get info (url and title) on a commit in runevent, this needs to
@@ -539,58 +543,6 @@ func (v *Provider) GetFileInsideRepo(ctx context.Context, runevent *info.Event,
539543
return string(getobj), nil
540544
}
541545

542-
// concatAllYamlFiles concat all yaml files from a directory as one big multi document yaml string.
543-
func (v *Provider) concatAllYamlFiles(ctx context.Context, objects []*github.TreeEntry, runevent *info.Event, tektonDirPath, ref string) (string, error) {
544-
var yamlFiles []string
545-
for _, value := range objects {
546-
if strings.HasSuffix(value.GetPath(), ".yaml") ||
547-
strings.HasSuffix(value.GetPath(), ".yml") {
548-
fullPath := tektonDirPath + "/" + value.GetPath()
549-
yamlFiles = append(yamlFiles, fullPath)
550-
}
551-
}
552-
553-
if len(yamlFiles) == 0 {
554-
return "", nil
555-
}
556-
557-
if v.graphQLClient == nil {
558-
var err error
559-
v.graphQLClient, err = newGraphQLClient(v)
560-
if err != nil {
561-
return "", fmt.Errorf("failed to create GraphQL client: %w", err)
562-
}
563-
}
564-
565-
graphQLResults, err := v.graphQLClient.fetchFiles(ctx, runevent.Organization, runevent.Repository, ref, yamlFiles)
566-
if err != nil {
567-
return "", fmt.Errorf("failed to fetch .tekton files via GraphQL: %w", err)
568-
}
569-
570-
var buf strings.Builder
571-
for _, path := range yamlFiles {
572-
content, ok := graphQLResults[path]
573-
if !ok {
574-
return "", fmt.Errorf("file %s not found in GraphQL response", path)
575-
}
576-
577-
// it used to be like that (stripped prefix) before we moved to GraphQL so
578-
// let's keep it that way.
579-
relativePath := strings.TrimPrefix(path, tektonDirPath+"/")
580-
if err := provider.ValidateYaml(content, relativePath); err != nil {
581-
return "", err
582-
}
583-
if buf.Len() > 0 && !strings.HasPrefix(string(content), "---") {
584-
buf.WriteString("---")
585-
}
586-
buf.WriteString("\n")
587-
buf.Write(content)
588-
buf.WriteString("\n")
589-
}
590-
591-
return buf.String(), nil
592-
}
593-
594546
// getPullRequest get a pull request details, caching the result for the lifetime of the event.
595547
func (v *Provider) getPullRequest(ctx context.Context, runevent *info.Event) (*info.Event, error) {
596548
if v.cachedPullRequest != nil {

pkg/provider/github/github_test.go

Lines changed: 110 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -252,10 +252,8 @@ func TestGetTektonDir(t *testing.T) {
252252
expectedString: "PipelineRun",
253253
treepath: "testdata/tree/simple",
254254
filterMessageSnippet: "Using PipelineRun definition from source pull_request tekton/cat#0",
255-
// 1. Get Repo root objects
256-
// 2. Get Tekton Dir objects
257-
// 3. GraphQL batch fetch for 2 files (replaces 2 REST calls)
258-
expectedGHApiCalls: 3,
255+
// 1. Single GraphQL call fetches tekton tree + inline blobs
256+
expectedGHApiCalls: 1,
259257
},
260258
{
261259
name: "test no subtree on push",
@@ -268,10 +266,8 @@ func TestGetTektonDir(t *testing.T) {
268266
expectedString: "PipelineRun",
269267
treepath: "testdata/tree/simple",
270268
filterMessageSnippet: "Using PipelineRun definition from source push",
271-
// 1. Get Repo root objects
272-
// 2. Get Tekton Dir objects
273-
// 3. GraphQL batch fetch for 2 files (replaces 2 REST calls)
274-
expectedGHApiCalls: 3,
269+
// 1. Single GraphQL call fetches tekton tree + inline blobs
270+
expectedGHApiCalls: 1,
275271
},
276272
{
277273
name: "test provenance default_branch ",
@@ -285,10 +281,8 @@ func TestGetTektonDir(t *testing.T) {
285281
provenance: "default_branch",
286282
filterMessageSnippet: "Using PipelineRun definition from default_branch: main",
287283
// 1. Resolve default branch to a commit SHA
288-
// 2. Get Repo root objects
289-
// 3. Get Tekton Dir objects
290-
// 4. GraphQL batch fetch for 2 files
291-
expectedGHApiCalls: 4,
284+
// 2. Single GraphQL call fetches tekton tree + inline blobs
285+
expectedGHApiCalls: 2,
292286
},
293287
{
294288
name: "test with subtree",
@@ -299,10 +293,8 @@ func TestGetTektonDir(t *testing.T) {
299293
},
300294
expectedString: "FROMSUBTREE",
301295
treepath: "testdata/tree/subdir",
302-
// 1. Get Repo root objects
303-
// 2. Get Tekton Dir objects
304-
// 3-5. Get object content for each object (foo/bar/pipeline.yaml)
305-
expectedGHApiCalls: 3,
296+
// 1. Single GraphQL call fetches tekton tree + inline blobs (including subdirs)
297+
expectedGHApiCalls: 1,
306298
},
307299
{
308300
name: "test with badly formatted yaml",
@@ -314,10 +306,9 @@ func TestGetTektonDir(t *testing.T) {
314306
expectedString: "FROMSUBTREE",
315307
treepath: "testdata/tree/badyaml",
316308
wantErr: "error unmarshalling yaml file badyaml.yaml: yaml: line 2: did not find expected key",
317-
// 1. Get Repo root objects
318-
// 2. Get Tekton Dir objects
319-
// 3. Get object content for object (badyaml.yaml)
320-
expectedGHApiCalls: 3,
309+
// 1. Single GraphQL call fetches tekton tree + inline blobs
310+
// Error occurs during YAML validation after fetch
311+
expectedGHApiCalls: 1,
321312
},
322313
{
323314
name: "test no tekton directory",
@@ -432,15 +423,48 @@ func TestGetTektonDirGraphQL(t *testing.T) {
432423
skipSetupGitTree bool
433424
}{
434425
{
435-
name: "graphql batch fetch reduces api calls",
426+
name: "graphql fetch tekton dir with inline blobs",
436427
event: &info.Event{
437428
Organization: "tekton",
438429
Repository: "cat",
439430
TriggerTarget: triggertype.PullRequest,
440431
},
441-
treepath: "testdata/tree/simple",
442-
wantLogSnippet: "GraphQL batch fetch",
443-
expectedAPICount: 3,
432+
skipSetupGitTree: true,
433+
setup: func(t *testing.T, mux *http.ServeMux, event *info.Event) {
434+
t.Helper()
435+
shaDir := fmt.Sprintf("%x", sha256.Sum256([]byte("testdata/tree/simple")))
436+
event.SHA = shaDir
437+
438+
// Setup GraphQL endpoint with inline blob contents
439+
mux.HandleFunc("/api/graphql", func(w http.ResponseWriter, _ *http.Request) {
440+
_ = json.NewEncoder(w).Encode(map[string]any{
441+
"data": map[string]any{
442+
"repository": map[string]any{
443+
"tektonTree": map[string]any{
444+
"entries": []map[string]any{
445+
{
446+
"name": "pipeline.yaml",
447+
"type": "blob",
448+
"path": "pipeline.yaml",
449+
"oid": "pipeline-sha",
450+
"object": map[string]any{"text": "kind: Pipeline\nmetadata:\n name: test\n"},
451+
},
452+
{
453+
"name": "task.yaml",
454+
"type": "blob",
455+
"path": "task.yaml",
456+
"oid": "task-sha",
457+
"object": map[string]any{"text": "kind: Task\nmetadata:\n name: test\n"},
458+
},
459+
},
460+
},
461+
},
462+
},
463+
})
464+
})
465+
},
466+
wantLogSnippet: "GitHub API call completed",
467+
expectedAPICount: 1, // Single GraphQL call fetches tree + blobs
444468
},
445469
{
446470
name: "graphql error handling",
@@ -500,7 +524,7 @@ func TestGetTektonDirGraphQL(t *testing.T) {
500524
http.Error(w, "GraphQL endpoint not available", http.StatusNotFound)
501525
})
502526
},
503-
wantErr: "failed to fetch .tekton files via GraphQL",
527+
wantErr: "GraphQL request failed with status 404",
504528
},
505529
{
506530
name: "default branch uses resolved sha for graphql",
@@ -514,7 +538,6 @@ func TestGetTektonDirGraphQL(t *testing.T) {
514538
setup: func(t *testing.T, mux *http.ServeMux, _ *info.Event) {
515539
t.Helper()
516540
resolvedSHA := "resolved-default-branch-sha"
517-
tektonDirSHA := "tektondirsha"
518541

519542
mux.HandleFunc("/repos/tekton/cat/branches/main", func(rw http.ResponseWriter, _ *http.Request) {
520543
branch := &github.Branch{
@@ -526,58 +549,80 @@ func TestGetTektonDirGraphQL(t *testing.T) {
526549
b, _ := json.Marshal(branch)
527550
fmt.Fprint(rw, string(b))
528551
})
529-
mux.HandleFunc("/repos/tekton/cat/git/trees/"+resolvedSHA, func(rw http.ResponseWriter, _ *http.Request) {
530-
tree := &github.Tree{
531-
SHA: github.Ptr(resolvedSHA),
532-
Entries: []*github.TreeEntry{
533-
{
534-
Path: github.Ptr(".tekton"),
535-
Type: github.Ptr("tree"),
536-
SHA: github.Ptr(tektonDirSHA),
537-
},
538-
},
539-
}
540-
b, _ := json.Marshal(tree)
541-
fmt.Fprint(rw, string(b))
542-
})
543-
mux.HandleFunc("/repos/tekton/cat/git/trees/"+tektonDirSHA, func(rw http.ResponseWriter, _ *http.Request) {
544-
tree := &github.Tree{
545-
SHA: github.Ptr(tektonDirSHA),
546-
Entries: []*github.TreeEntry{
547-
{
548-
Path: github.Ptr("pipeline.yaml"),
549-
Type: github.Ptr("blob"),
550-
SHA: github.Ptr("pipeline-sha"),
551-
},
552-
{
553-
Path: github.Ptr("pipelinerun.yaml"),
554-
Type: github.Ptr("blob"),
555-
SHA: github.Ptr("pipelinerun-sha"),
556-
},
557-
},
558-
}
559-
b, _ := json.Marshal(tree)
560-
fmt.Fprint(rw, string(b))
561-
})
562552
mux.HandleFunc("/api/graphql", func(w http.ResponseWriter, r *http.Request) {
563553
var graphQLReq struct {
564-
Query string `json:"query"`
554+
Query string `json:"query"`
555+
Variables map[string]any `json:"variables"`
565556
}
566557
assert.NilError(t, json.NewDecoder(r.Body).Decode(&graphQLReq))
567-
assert.Assert(t, strings.Contains(graphQLReq.Query, resolvedSHA+":.tekton/pipeline.yaml"), graphQLReq.Query)
568-
assert.Assert(t, !strings.Contains(graphQLReq.Query, `main:.tekton/pipeline.yaml`), graphQLReq.Query)
558+
// New implementation uses tektonExpr variable: "resolvedSHA:.tekton"
559+
assert.Assert(t, strings.Contains(graphQLReq.Query, "tektonTree:"), graphQLReq.Query)
560+
assert.Assert(t, graphQLReq.Variables["tektonExpr"] == resolvedSHA+":.tekton", "expected tektonExpr=%s:.tekton, got %v", resolvedSHA, graphQLReq.Variables["tektonExpr"])
569561

562+
// Return tree structure with inline blob contents
570563
_ = json.NewEncoder(w).Encode(map[string]any{
571564
"data": map[string]any{
572565
"repository": map[string]any{
573-
"file0": map[string]any{"text": "kind: Pipeline\nmetadata:\n name: pipeline\n"},
574-
"file1": map[string]any{"text": "kind: PipelineRun\nmetadata:\n name: run\n"},
566+
"tektonTree": map[string]any{
567+
"entries": []map[string]any{
568+
{
569+
"name": "pipeline.yaml",
570+
"type": "blob",
571+
"path": "pipeline.yaml",
572+
"oid": "pipeline-sha",
573+
"object": map[string]any{"text": "kind: Pipeline\nmetadata:\n name: pipeline\n"},
574+
},
575+
{
576+
"name": "pipelinerun.yaml",
577+
"type": "blob",
578+
"path": "pipelinerun.yaml",
579+
"oid": "pipelinerun-sha",
580+
"object": map[string]any{"text": "kind: PipelineRun\nmetadata:\n name: run\n"},
581+
},
582+
},
583+
},
584+
},
585+
},
586+
})
587+
})
588+
},
589+
expectedAPICount: 2, // 1 for get_default_branch + 1 for GraphQL
590+
},
591+
{
592+
name: "invalid yaml validation failure",
593+
event: &info.Event{
594+
Organization: "tekton",
595+
Repository: "cat",
596+
TriggerTarget: triggertype.PullRequest,
597+
},
598+
skipSetupGitTree: true,
599+
setup: func(t *testing.T, mux *http.ServeMux, event *info.Event) {
600+
t.Helper()
601+
shaDir := fmt.Sprintf("%x", sha256.Sum256([]byte("testdata/tree/badyaml")))
602+
event.SHA = shaDir
603+
604+
mux.HandleFunc("/api/graphql", func(w http.ResponseWriter, _ *http.Request) {
605+
_ = json.NewEncoder(w).Encode(map[string]any{
606+
"data": map[string]any{
607+
"repository": map[string]any{
608+
"tektonTree": map[string]any{
609+
"entries": []map[string]any{
610+
{
611+
"name": "badyaml.yaml",
612+
"type": "blob",
613+
"path": "badyaml.yaml",
614+
"oid": "badyaml-sha",
615+
"object": map[string]any{"text": "bad:\n yaml:\n - indent\n error"},
616+
},
617+
},
618+
},
575619
},
576620
},
577621
})
578622
})
579623
},
580-
expectedAPICount: 4,
624+
wantErr: "error unmarshalling yaml file badyaml.yaml",
625+
expectedAPICount: 1, // Single GraphQL call
581626
},
582627
}
583628

0 commit comments

Comments
 (0)