Add Prometheus metrics for GitHub API calls#808
Conversation
There was a problem hiding this comment.
1 issue found across 5 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="internal/source/metrics_transport.go">
<violation number="1" location="internal/source/metrics_transport.go:63">
P2: `status_code` is taken after ETag transport rewrites 304 cache hits to 200, so conditional GitHub responses are misreported in the counter.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
internal/source/metrics_transport.go
Outdated
| Name: "kelos_github_api_requests_total", | ||
| Help: "Total number of GitHub API requests", | ||
| }, | ||
| []string{"method", "status_code"}, |
There was a problem hiding this comment.
Let's add label to specify the kind of the resource. (Issue, pull request, comments, or others)
|
🤖 Kelos Task Status Task |
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="internal/source/metrics_transport.go">
<violation number="1" location="internal/source/metrics_transport.go:92">
P2: The default case returns `other` immediately, so known resources earlier in the path are never considered. For paths like `/repos/.../issues/123/events`, this mislabels the resource as `other` instead of `issues`.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
🤖 Kelos Task Status Task |
Introduce a metricsTransport RoundTripper that records kelos_github_api_requests_total (counter by method, status_code) and kelos_github_api_request_duration_seconds (histogram by method) for every outgoing GitHub API request. The transport wraps the existing ETag transport in the spawner and is also used by the GitHubReporter. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Fix status_code misreporting: move metrics transport inside ETag transport (etag → metrics → default) so it records the real HTTP status codes from GitHub instead of rewritten 304→200 responses - Add resource label to both counter and histogram metrics to classify API requests by resource kind (issues, pulls, comments, reviews, collaborators, other) - Add unit tests for classifyResource URL path parsing Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The default case in the switch statement returned "other" immediately, preventing known resources earlier in the path from being detected. For paths like /repos/.../issues/123/events, this mislabeled the resource as "other" instead of "issues". Remove the default return so the loop continues scanning earlier segments. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
c15b4f0 to
670f104
Compare
|
🤖 Kelos Agent @gjkim42 /kelos needs-input All review feedback was addressed in previous commits. Ready for re-review. Changes made:
CI status: build, test, test-integration, verify all pass. The |
What type of PR is this?
/kind feature
What this PR does / why we need it:
Adds Prometheus metrics to track GitHub API usage by kelos. A new
metricsTransportRoundTripper wraps the HTTP transport used for all GitHub API calls (both discovery and reporting), recording:kelos_github_api_requests_total— counter with labelsmethodandstatus_codekelos_github_api_request_duration_seconds— histogram with labelmethodWhich issue(s) this PR is related to:
Fixes #807
Special notes for your reviewer:
The metrics transport is placed as the outermost layer in the transport chain (metrics → etag → default), so it captures all requests including conditional (304) responses. The GitHubReporter now also uses the instrumented HTTP client so reporting API calls are tracked as well.
Does this PR introduce a user-facing change?
Summary by cubic
Add Prometheus metrics for all GitHub API calls to track usage, latency, and resource type, addressing #807. A new
metricsTransportwraps the HTTP client so both discovery and reporting requests are recorded.New Features
metricsTransportRoundTripper that recordskelos_github_api_requests_total(labels:method,status_code,resource) andkelos_github_api_request_duration_seconds(labels:method,resource).Bug Fixes
status_codemisreporting by placing metrics inside the ETag transport (etag → metrics → default); corrected resource parsing so sub-resources likeeventsdon’t mask parent resources (e.g.,issues).Written for commit 670f104. Summary will update on new commits.