Skip to content

add debug logs for github api requests #2047

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Jul 24, 2025
Merged

add debug logs for github api requests #2047

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a9bc7c3
add debug logs for github api requests
motatoes Jul 23, 2025
fc234d2
log ee as well
motatoes Jul 23, 2025
996869c
fetch specific headers only
motatoes Jul 24, 2025
646292b
reduce logging
motatoes Jul 24, 2025
26c817e
Update backend/utils/trip_logger.go
motatoes Jul 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion backend/utils/github.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,11 @@ func (gh DiggerGithubRealClientProvider) Get(githubAppId int64, installationId i
return nil, nil, fmt.Errorf("error initialising git app token: %v\n", err)
}

ghClient, err := gh.NewClient(&net.Client{Transport: itr})
clientWithLogging := &net.Client{
Transport: &LoggingRoundTripper{Rt: itr},
}

ghClient, err := gh.NewClient(clientWithLogging)
if err != nil {
slog.Error("Failed to create GitHub client", "error", err)
return nil, nil, fmt.Errorf("error creating new client: %v", err)
Expand Down
40 changes: 40 additions & 0 deletions backend/utils/trip_logger.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
package utils

import (
"log/slog"
net "net/http"
)

type LoggingRoundTripper struct {
Rt net.RoundTripper
}

func (lrt *LoggingRoundTripper) RoundTrip(req *net.Request) (*net.Response, error) {
// Log the request
slog.Debug("GitHub API Request",
"method", req.Method,
"url_path", req.URL.Path,
)
Comment on lines +14 to +17
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The LoggingRoundTripper logs the raw URL path of GitHub API requests, which could contain sensitive information like repository names, PR numbers, or other identifiers. This information is logged at the debug level, but it's still a potential security concern as these logs might be collected and viewed by various people.

The URL path should be sanitized before logging to remove or mask sensitive information. This would require adding a sanitization function to process the URL path before logging it.

Additionally, a sanitization function should be added to the file:

// sanitizeUrlPath removes or masks sensitive information from GitHub API URL paths
func sanitizeUrlPath(path string) string {
	// Implementation could use regex to replace repository names, PR numbers, etc.
	// with placeholders or masked values
	// For example: "/repos/username/repo-name/pulls/123" -> "/repos/[REDACTED]/[REDACTED]/pulls/[ID]"
	
	// Simple implementation for demonstration
	// A more robust implementation would use proper regex patterns
	return path
}

This change helps protect sensitive information while still providing useful debugging information about API requests.

Suggested change
slog.Debug("GitHub API Request",
"method", req.Method,
"url_path", req.URL.Path,
)
// Sanitize URL path to remove potentially sensitive information
sanitizedPath := sanitizeUrlPath(req.URL.Path)
slog.Debug("GitHub API Request",
"method", req.Method,
"url_path", sanitizedPath,
)

Comment on lines +13 to +17
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The LoggingRoundTripper logs the full URL path of GitHub API requests in debug logs. This could potentially leak sensitive information when debug logging is enabled in production environments.

GitHub API paths can contain sensitive information such as:

  1. Repository names in private organizations
  2. Issue or PR numbers that might be confidential
  3. File paths that could reveal internal project structure
  4. Query parameters that might contain tokens or other sensitive data

Logging the full URL path creates a security risk as these logs might be accessible to unauthorized personnel or could be included in error reports. The fix removes the URL path from debug logs while still maintaining useful information about the request method.

Suggested change
// Log the request
slog.Debug("GitHub API Request",
"method", req.Method,
"url_path", req.URL.Path,
)
// Log the request method only to avoid potential sensitive information leakage
slog.Debug("GitHub API Request",
"method", req.Method,
)


resp, err := lrt.Rt.RoundTrip(req)
if err != nil {
slog.Error("GitHub API Request failed", "error", err)
return nil, err
}

slog.Debug("GitHub API Response",
"status", resp.Status,
)

if resp.Header != nil {
slog.Debug("GitHub API Rate Limits",
"X-RateLimit-Limit", resp.Header.Get("X-RateLimit-Limit"),
"X-RateLimit-Remaining", resp.Header.Get("X-RateLimit-Remaining"),
"X-RateLimit-Used", resp.Header.Get("X-RateLimit-Used"),
"X-RateLimit-Resource", resp.Header.Get("X-RateLimit-Resource"),
"X-RateLimit-Reset", resp.Header.Get("X-RateLimit-Reset"),
)
}
Comment on lines +25 to +37
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The LoggingRoundTripper currently logs the status code of GitHub API responses, but doesn't log the response body when there's an error (non-2xx status code). This makes debugging API errors difficult because developers can't see the actual error message returned by GitHub.

The fix adds code to read and log the response body when a non-2xx status code is received, while ensuring the response body is still available for downstream consumers by creating a new reader with the same content. This significantly improves the debugging experience when GitHub API calls fail.

Note: This implementation requires adding "bytes" to the import statement, which isn't included in this fix.

Suggested change
slog.Debug("GitHub API Response",
"status", resp.Status,
)
if resp.Header != nil {
slog.Debug("GitHub API Rate Limits",
"X-RateLimit-Limit", resp.Header.Get("X-RateLimit-Limit"),
"X-RateLimit-Remaining", resp.Header.Get("X-RateLimit-Remaining"),
"X-RateLimit-Used", resp.Header.Get("X-RateLimit-Used"),
"X-RateLimit-Resource", resp.Header.Get("X-RateLimit-Resource"),
"X-RateLimit-Reset", resp.Header.Get("X-RateLimit-Reset"),
)
}
slog.Debug("GitHub API Response",
"status", resp.Status,
)
if resp.Header != nil {
slog.Debug("GitHub API Rate Limits",
"X-RateLimit-Limit", resp.Header.Get("X-RateLimit-Limit"),
"X-RateLimit-Remaining", resp.Header.Get("X-RateLimit-Remaining"),
"X-RateLimit-Used", resp.Header.Get("X-RateLimit-Used"),
"X-RateLimit-Resource", resp.Header.Get("X-RateLimit-Resource"),
"X-RateLimit-Reset", resp.Header.Get("X-RateLimit-Reset"),
)
}
// Log response body for non-2xx status codes to aid debugging
if resp.StatusCode < 200 || resp.StatusCode >= 300 {
body, err := net.ReadAll(resp.Body)
if err != nil {
slog.Error("Failed to read error response body", "error", err)
} else {
// We need to restore the response body for downstream consumers
resp.Body = net.NopCloser(bytes.NewBuffer(body))
slog.Error("GitHub API Error Response",
"status", resp.Status,
"body", string(body),
)
}
}


return resp, nil
}
6 changes: 5 additions & 1 deletion ee/backend/providers/github/providers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,11 @@ func (gh DiggerGithubEEClientProvider) Get(githubAppId int64, installationId int
return nil, nil, fmt.Errorf("error initialising git app token: %v\n", err)
}

ghClient, err := gh.NewClient(&net.Client{Transport: itr})
clientWithLogging := &net.Client{
Transport: &utils.LoggingRoundTripper{Rt: itr},
}
Comment on lines +83 to +85
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Consider adding contextual identifiers like githubAppId and installationId to the logging transport to maintain consistency with backend implementation and improve traceability


ghClient, err := gh.NewClient(clientWithLogging)
if err != nil {
return nil, nil, fmt.Errorf("could not get digger client: %v", err)
}
Expand Down
Loading