-
Notifications
You must be signed in to change notification settings - Fork 113
feat: Enable detailed GitHub API logging for debugging #2199
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
feat: Enable detailed GitHub API logging for debugging #2199
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary of Changes
Hello @chmouel, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
I've implemented a new feature to enhance debugging for GitHub API interactions within the system. This change introduces a profiling utility that wraps all GitHub API calls, enabling detailed logging of call duration, requested URLs, and remaining rate-limit information at the debug level. This will significantly aid in troubleshooting API-related issues. Additionally, I've reorganized and updated the logging configuration documentation to guide users on how to enable these detailed logs.
Highlights
- New Profiling Utility: Introduced a new profiler.go utility to wrap all GitHub API calls, centralizing logging and metrics collection.
- Enhanced Debug Logging: Enabled detailed debug logging for GitHub API interactions, capturing call duration, requested URLs, and rate-limit information.
- Documentation Reorganization: Migrated and updated logging configuration documentation to a dedicated logging.md file, improving clarity and guidance for users.
- Comprehensive API Call Wrapping: Integrated the new API call wrapping across various GitHub provider functions, ensuring comprehensive coverage for debugging.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command>
or @gemini-code-assist <command>
. Below is a summary of the supported commands.
Feature | Command | Description |
---|---|---|
Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/
folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments or fill out our survey to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds detailed GitHub API logging for debugging purposes by wrapping all GitHub API calls with a profiling utility. This enables debugging of GitHub API interactions by logging call duration, requested URL, and rate-limit information at debug level.
- Added a new
Wrap
function that instruments GitHub API calls with logging and metrics - Replaced direct GitHub API calls throughout the codebase with wrapped versions
- Migrated logging configuration documentation to a dedicated file with instructions for enabling API debugging
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
pkg/provider/github/profiler.go | New profiling utility with Wrap functions for GitHub API calls |
pkg/provider/github/status.go | Wrapped status-related GitHub API calls |
pkg/provider/github/parse_payload.go | Wrapped pull request listing API call |
pkg/provider/github/github.go | Wrapped various GitHub API calls and removed direct metrics recording |
pkg/provider/github/acl.go | Wrapped ACL-related GitHub API calls |
hack/dev/kind/install.sh | Added debug logging configuration for development |
docs/content/docs/install/settings.md | Removed logging configuration section |
docs/content/docs/install/logging.md | New dedicated logging documentation |
Comments suppressed due to low confidence (1)
pkg/provider/github/github.go:702
- The closing brace for the if statement appears to be misplaced. The comment creation logic on lines 704-708 should be inside the if statement block, not after it.
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a valuable debugging feature by adding detailed logging for GitHub API calls. The implementation is straightforward, wrapping existing API calls with a new profiling utility. My review focuses on the new profiler.go
file, with suggestions to improve maintainability by reducing code duplication and ensuring consistency in logging. The documentation changes are clear and provide good guidance for users on how to enable this new feature.
1586a90
to
00ac3d9
Compare
This is huge for observability. One thing I'm not sure about is the wrapping- I'd worry that it's too easy to accidentally omit the That being said, I'm not sure what the alternative could be, besides literally creating a middleware struct which enumerates and instruments every API method we use :/ I've opened an issue on go-github for an instrumentation mechanism. In the meantime though this seems probably as good as any other option we have for instrumentation |
00ac3d9
to
fc575a6
Compare
yeah i tried so many other ways trying to override the http Transport to add the logging, but this was getting way too messy with oauth and others.. i don't like much either, but wrapping as well allows us to do some proper categorization, we should be able to get reviewer or gemini code assit to check those, and if we miss one it's not the end of the world.... great about the new go-github issue that would be fantastic. |
6c0a60f
to
c779c41
Compare
/gemini review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces detailed logging for GitHub API calls, which is a great addition for debugging and monitoring. The implementation is solid, wrapping API calls to log duration, URL, and rate-limit information. The documentation has also been updated accordingly.
My review includes a few suggestions to improve consistency and maintainability. I've recommended refactoring the new profiler to reduce code duplication, using structured logging consistently for rate limit warnings, and updating the documentation and tests to reflect these changes. I've also pointed out a minor inconsistency in an operation name.
19f3dea
to
50ff8b4
Compare
/retest |
github E2E tests are now showing the api call they make, it's not completely rock solid but it's 99% good enough to detect which api call has been made for that particuliar e2e test, shows like this, the commit,... They are collected in the run artifact, i am not sure yet how we can do report detection across run since we need to store somewhere but it's a good start nonetheless: ![]() |
* Wrapped all GitHub API calls with comprehensive profiling utilities that provide detailed performance and quota monitoring capabilities. * Added intelligent rate limit monitoring with threshold-based alerting: - ERROR level: Critical alerts when <50 calls remaining - WARN level: Warnings when <100 calls remaining - INFO level: Notifications when <500 calls remaining - DEBUG level: All API calls with duration, URL, and rate limit data * Enhanced logging includes human-readable reset timestamps, total quotas, and repository context for better operational visibility. * Created comprehensive test coverage including edge cases for all rate limit threshold scenarios and API response variations. * Migrated logging configuration to dedicated documentation with detailed examples, troubleshooting guides, and operational best practices. * Added production-ready monitoring capabilities to prevent service disruptions and optimize API usage patterns. This enhancement provides administrators with proactive rate limit management, detailed API interaction debugging, and early warning systems for quota exhaustion. Jira: https://issues.redhat.com/browse/SRVKP-7037 Signed-off-by: Chmouel Boudjnah <[email protected]>
- Introduced a mechanism to collect GitHub API call metrics from controller logs during E2E tests. - Added logic to parse structured log entries for API calls at the end of each test run. - Stored the collected data, including operation, duration, and status code, in a structured JSON report. - Updated the CI workflow to archive the generated reports for analysis and monitoring. Signed-off-by: Chmouel Boudjnah <[email protected]>
50ff8b4
to
a93974c
Compare
/retest |
1 similar comment
/retest |
- Introduced a mechanism to collect GitHub API call metrics from controller logs during E2E tests. - Added logic to parse structured log entries for API calls at the end of each test run. - Stored the collected data, including operation, duration, and status code, in a structured JSON report. - Updated the CI workflow to archive the generated reports for analysis and monitoring. Signed-off-by: Chmouel Boudjnah <[email protected]>
information at debug level.
interactions.
logs.
I have tried a few different methods to wrap the http.Transport of the github.Client but it was too much of a pain to do, and having a stupid wrap even if it's cumbersome allows us to have more information like the repo/namespace target
Next thing is to wrap the E2E tests to reuse those and so then we can show the e2e tests calls how long they took
📝 Description of the Change
🔗 Linked GitHub Issue
N/A
👨🏻 Linked Jira
Jira:https://issues.redhat.com/browse/SRVKP-8271
🚀 Type of Change
fix:
)feat:
)feat!:
,fix!:
)docs:
)chore:
)refactor:
)enhance:
)🧪 Testing Strategy
✅ Submitter Checklist
fix:
,feat:
) matches the "Type of Change" I selected above.make test
andmake lint
locally to check for and fix anyissues. For an efficient workflow, I have considered installing
pre-commit and running
pre-commit install
toautomate these checks.