-
Notifications
You must be signed in to change notification settings - Fork 2.4k
Add actions job log buffer and profiler #866
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
Changes from 33 commits
8a5efb9
e6ef962
271c7c2
e65c0f0
ade3852
5a76cbd
8e60fb2
e1c3143
75dc8e7
b128e44
88d16d2
4bf84b2
6b8f2ba
8002fbd
52e531e
8f85398
0d19480
f104e67
9d273b9
4e43327
2ff2d4f
c6f5f7f
1c1061c
75b8c94
71bfac8
a43b03c
d9c8825
ec070ee
0434b7e
fb301c6
106d802
516c0f7
6c3c31a
82d4ce2
e40e289
4310db8
e0767fe
9677c07
ed6bc2d
696e5fd
10e1995
2eb2e16
47aaa01
6b910ff
4a673c9
556a41c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,67 @@ | ||
package buffer | ||
|
||
import ( | ||
"bufio" | ||
"fmt" | ||
"net/http" | ||
"strings" | ||
) | ||
|
||
mattdholloway marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// ProcessResponseAsRingBufferToEnd reads the body of an HTTP response line by line, | ||
// storing only the last maxJobLogLines lines using a ring buffer (sliding window). | ||
// This efficiently retains the most recent lines, overwriting older ones as needed. | ||
// | ||
// Parameters: | ||
// httpResp: The HTTP response whose body will be read. | ||
// maxJobLogLines: The maximum number of log lines to retain. | ||
// | ||
// Returns: | ||
// string: The concatenated log lines (up to maxJobLogLines), separated by newlines. | ||
// int: The total number of lines read from the response. | ||
// *http.Response: The original HTTP response. | ||
// error: Any error encountered during reading. | ||
// | ||
// The function uses a ring buffer to efficiently store only the last maxJobLogLines lines. | ||
// If the response contains more lines than maxJobLogLines, only the most recent lines are kept. | ||
func ProcessResponseAsRingBufferToEnd(httpResp *http.Response, maxJobLogLines int) (string, int, *http.Response, error) { | ||
lines := make([]string, maxJobLogLines) | ||
validLines := make([]bool, maxJobLogLines) | ||
totalLines := 0 | ||
writeIndex := 0 | ||
|
||
scanner := bufio.NewScanner(httpResp.Body) | ||
scanner.Buffer(make([]byte, 0, 64*1024), 1024*1024) | ||
mattdholloway marked this conversation as resolved.
Show resolved
Hide resolved
mattdholloway marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
for scanner.Scan() { | ||
line := scanner.Text() | ||
totalLines++ | ||
|
||
lines[writeIndex] = line | ||
validLines[writeIndex] = true | ||
writeIndex = (writeIndex + 1) % maxJobLogLines | ||
} | ||
|
||
if err := scanner.Err(); err != nil { | ||
return "", 0, httpResp, fmt.Errorf("failed to read log content: %w", err) | ||
} | ||
|
||
var result []string | ||
linesInBuffer := totalLines | ||
if linesInBuffer > maxJobLogLines { | ||
linesInBuffer = maxJobLogLines | ||
} | ||
|
||
startIndex := 0 | ||
if totalLines > maxJobLogLines { | ||
startIndex = writeIndex | ||
} | ||
|
||
for i := 0; i < linesInBuffer; i++ { | ||
idx := (startIndex + i) % maxJobLogLines | ||
if validLines[idx] { | ||
result = append(result, lines[idx]) | ||
} | ||
} | ||
|
||
return strings.Join(result, "\n"), totalLines, httpResp, nil | ||
} |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -4,12 +4,13 @@ import ( | |||||
"context" | ||||||
"encoding/json" | ||||||
"fmt" | ||||||
"io" | ||||||
"net/http" | ||||||
"strconv" | ||||||
"strings" | ||||||
|
||||||
buffer "github.com/github/github-mcp-server/pkg/buffer" | ||||||
ghErrors "github.com/github/github-mcp-server/pkg/errors" | ||||||
"github.com/github/github-mcp-server/pkg/profiler" | ||||||
"github.com/github/github-mcp-server/pkg/translations" | ||||||
"github.com/google/go-github/v74/github" | ||||||
"github.com/mark3labs/mcp-go/mcp" | ||||||
|
@@ -530,7 +531,7 @@ func ListWorkflowJobs(getClient GetClientFn, t translations.TranslationHelperFun | |||||
} | ||||||
|
||||||
// GetJobLogs creates a tool to download logs for a specific workflow job or efficiently get all failed job logs for a workflow run | ||||||
func GetJobLogs(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { | ||||||
func GetJobLogs(getClient GetClientFn, t translations.TranslationHelperFunc, contentWindowSize int) (tool mcp.Tool, handler server.ToolHandlerFunc) { | ||||||
return mcp.NewTool("get_job_logs", | ||||||
mcp.WithDescription(t("TOOL_GET_JOB_LOGS_DESCRIPTION", "Download logs for a specific workflow job or efficiently get all failed job logs for a workflow run")), | ||||||
mcp.WithToolAnnotation(mcp.ToolAnnotation{ | ||||||
|
@@ -613,18 +614,18 @@ func GetJobLogs(getClient GetClientFn, t translations.TranslationHelperFunc) (to | |||||
|
||||||
if failedOnly && runID > 0 { | ||||||
// Handle failed-only mode: get logs for all failed jobs in the workflow run | ||||||
return handleFailedJobLogs(ctx, client, owner, repo, int64(runID), returnContent, tailLines) | ||||||
return handleFailedJobLogs(ctx, client, owner, repo, int64(runID), returnContent, tailLines, contentWindowSize) | ||||||
} else if jobID > 0 { | ||||||
// Handle single job mode | ||||||
return handleSingleJobLogs(ctx, client, owner, repo, int64(jobID), returnContent, tailLines) | ||||||
return handleSingleJobLogs(ctx, client, owner, repo, int64(jobID), returnContent, tailLines, contentWindowSize) | ||||||
} | ||||||
|
||||||
return mcp.NewToolResultError("Either job_id must be provided for single job logs, or run_id with failed_only=true for failed job logs"), nil | ||||||
} | ||||||
} | ||||||
|
||||||
// handleFailedJobLogs gets logs for all failed jobs in a workflow run | ||||||
func handleFailedJobLogs(ctx context.Context, client *github.Client, owner, repo string, runID int64, returnContent bool, tailLines int) (*mcp.CallToolResult, error) { | ||||||
func handleFailedJobLogs(ctx context.Context, client *github.Client, owner, repo string, runID int64, returnContent bool, tailLines int, contentWindowSize int) (*mcp.CallToolResult, error) { | ||||||
// First, get all jobs for the workflow run | ||||||
jobs, resp, err := client.Actions.ListWorkflowJobs(ctx, owner, repo, runID, &github.ListWorkflowJobsOptions{ | ||||||
Filter: "latest", | ||||||
|
@@ -656,7 +657,7 @@ func handleFailedJobLogs(ctx context.Context, client *github.Client, owner, repo | |||||
// Collect logs for all failed jobs | ||||||
var logResults []map[string]any | ||||||
for _, job := range failedJobs { | ||||||
jobResult, resp, err := getJobLogData(ctx, client, owner, repo, job.GetID(), job.GetName(), returnContent, tailLines) | ||||||
jobResult, resp, err := getJobLogData(ctx, client, owner, repo, job.GetID(), job.GetName(), returnContent, tailLines, contentWindowSize) | ||||||
if err != nil { | ||||||
// Continue with other jobs even if one fails | ||||||
jobResult = map[string]any{ | ||||||
|
@@ -689,8 +690,8 @@ func handleFailedJobLogs(ctx context.Context, client *github.Client, owner, repo | |||||
} | ||||||
|
||||||
// handleSingleJobLogs gets logs for a single job | ||||||
func handleSingleJobLogs(ctx context.Context, client *github.Client, owner, repo string, jobID int64, returnContent bool, tailLines int) (*mcp.CallToolResult, error) { | ||||||
jobResult, resp, err := getJobLogData(ctx, client, owner, repo, jobID, "", returnContent, tailLines) | ||||||
func handleSingleJobLogs(ctx context.Context, client *github.Client, owner, repo string, jobID int64, returnContent bool, tailLines int, contentWindowSize int) (*mcp.CallToolResult, error) { | ||||||
jobResult, resp, err := getJobLogData(ctx, client, owner, repo, jobID, "", returnContent, tailLines, contentWindowSize) | ||||||
if err != nil { | ||||||
return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to get job logs", resp, err), nil | ||||||
} | ||||||
|
@@ -704,7 +705,7 @@ func handleSingleJobLogs(ctx context.Context, client *github.Client, owner, repo | |||||
} | ||||||
|
||||||
// getJobLogData retrieves log data for a single job, either as URL or content | ||||||
func getJobLogData(ctx context.Context, client *github.Client, owner, repo string, jobID int64, jobName string, returnContent bool, tailLines int) (map[string]any, *github.Response, error) { | ||||||
func getJobLogData(ctx context.Context, client *github.Client, owner, repo string, jobID int64, jobName string, returnContent bool, tailLines int, contentWindowSize int) (map[string]any, *github.Response, error) { | ||||||
// Get the download URL for the job logs | ||||||
url, resp, err := client.Actions.GetWorkflowJobLogs(ctx, owner, repo, jobID, 1) | ||||||
if err != nil { | ||||||
|
@@ -721,7 +722,7 @@ func getJobLogData(ctx context.Context, client *github.Client, owner, repo strin | |||||
|
||||||
if returnContent { | ||||||
// Download and return the actual log content | ||||||
content, originalLength, httpResp, err := downloadLogContent(url.String(), tailLines) //nolint:bodyclose // Response body is closed in downloadLogContent, but we need to return httpResp | ||||||
content, originalLength, httpResp, err := downloadLogContent(ctx, url.String(), tailLines, contentWindowSize) //nolint:bodyclose // Response body is closed in downloadLogContent, but we need to return httpResp | ||||||
if err != nil { | ||||||
// To keep the return value consistent wrap the response as a GitHub Response | ||||||
ghRes := &github.Response{ | ||||||
|
@@ -742,9 +743,11 @@ func getJobLogData(ctx context.Context, client *github.Client, owner, repo strin | |||||
return result, resp, nil | ||||||
} | ||||||
|
||||||
// downloadLogContent downloads the actual log content from a GitHub logs URL | ||||||
func downloadLogContent(logURL string, tailLines int) (string, int, *http.Response, error) { | ||||||
httpResp, err := http.Get(logURL) //nolint:gosec // URLs are provided by GitHub API and are safe | ||||||
func downloadLogContent(ctx context.Context, logURL string, tailLines int, maxLines int) (string, int, *http.Response, error) { | ||||||
prof := profiler.New(nil, profiler.IsProfilingEnabled()) | ||||||
mattdholloway marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
finish := prof.Start(ctx, "log_buffer_processing") | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Creating a new profiler instance for each call is inefficient. Consider reusing the global profiler or passing the profiler as a parameter to avoid repeated initialization.
Suggested change
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||||||
|
||||||
httpResp, err := http.Get(logURL) //nolint:gosec | ||||||
if err != nil { | ||||||
return "", 0, httpResp, fmt.Errorf("failed to download logs: %w", err) | ||||||
} | ||||||
|
@@ -754,36 +757,29 @@ func downloadLogContent(logURL string, tailLines int) (string, int, *http.Respon | |||||
return "", 0, httpResp, fmt.Errorf("failed to download logs: HTTP %d", httpResp.StatusCode) | ||||||
} | ||||||
|
||||||
content, err := io.ReadAll(httpResp.Body) | ||||||
if err != nil { | ||||||
return "", 0, httpResp, fmt.Errorf("failed to read log content: %w", err) | ||||||
if tailLines <= 0 { | ||||||
tailLines = 1000 | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The magic number 1000 should be defined as a named constant to improve maintainability and make the default value explicit throughout the codebase.
Suggested change
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||||||
} | ||||||
|
||||||
// Clean up and format the log content for better readability | ||||||
logContent := strings.TrimSpace(string(content)) | ||||||
bufferSize := maxLines | ||||||
if tailLines > maxLines && tailLines <= 500 { | ||||||
bufferSize = tailLines | ||||||
} | ||||||
|
||||||
trimmedContent, lineCount := trimContent(logContent, tailLines) | ||||||
return trimmedContent, lineCount, httpResp, nil | ||||||
} | ||||||
processedInput, totalLines, httpResp, err := buffer.ProcessResponseAsRingBufferToEnd(httpResp, bufferSize) | ||||||
if err != nil { | ||||||
return "", 0, httpResp, fmt.Errorf("failed to process log content: %w", err) | ||||||
} | ||||||
|
||||||
// trimContent trims the content to a maximum length and returns the trimmed content and an original length | ||||||
func trimContent(content string, tailLines int) (string, int) { | ||||||
// Truncate to tail_lines if specified | ||||||
lineCount := 0 | ||||||
if tailLines > 0 { | ||||||
|
||||||
// Count backwards to find the nth newline from the end and a total number of lines | ||||||
for i := len(content) - 1; i >= 0 && lineCount < tailLines; i-- { | ||||||
if content[i] == '\n' { | ||||||
lineCount++ | ||||||
// If we have reached the tailLines, trim the content | ||||||
if lineCount == tailLines { | ||||||
content = content[i+1:] | ||||||
} | ||||||
} | ||||||
} | ||||||
lines := strings.Split(processedInput, "\n") | ||||||
if len(lines) > tailLines { | ||||||
lines = lines[len(lines)-tailLines:] | ||||||
} | ||||||
return content, lineCount | ||||||
finalResult := strings.Join(lines, "\n") | ||||||
|
||||||
_ = finish(len(lines), int64(len(finalResult))) | ||||||
|
||||||
return finalResult, totalLines, httpResp, nil | ||||||
} | ||||||
|
||||||
// RerunWorkflowRun creates a tool to re-run an entire workflow run | ||||||
|
Uh oh!
There was an error while loading. Please reload this page.