chore(metrics): usage metrics for modules#3737
Conversation
✅ Deploy Preview for testcontainers-go ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
Summary by CodeRabbit
WalkthroughAdds a new modules usage-metrics workflow and dashboard, updates the core workflow and collector to use separate CSV outputs, and rewires MkDocs, helper scripts, and documentation for the split metrics structure. ChangesUsage Metrics System Overhaul
Sequence Diagram(s)sequenceDiagram
participant Workflow as usage-metrics-modules.yml
participant Collect as collect.go modules
participant GitHubAPI as gh api /search/code
participant CSV as docs/usage-metrics/modules.csv
participant PR as branch + pull request
Workflow->>Collect: go run collect.go modules -module ...
loop per module and retry pass
Collect->>GitHubAPI: search query
GitHubAPI-->>Collect: total_count
Collect->>CSV: appendToCSV
end
Collect->>CSV: sortCSV
Workflow->>PR: create/update month-scoped branch
Workflow->>PR: open PR to main if none exists
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
mdelapenya
force-pushed
the
feat/modules-usage-metrics
branch
from
6056710 to
accd6e7
Compare
Signed-off-by: mdelapenya <mdelapenya@gmail.com>
Signed-off-by: mdelapenya <mdelapenya@gmail.com>
Signed-off-by: mdelapenya <mdelapenya@gmail.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3509b9b1eb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| git fetch origin "$BRANCH_NAME" | ||
| git checkout "$BRANCH_NAME" |
There was a problem hiding this comment.
In the existing-branch path, this fetch does not make $BRANCH_NAME checkoutable in the default actions/checkout clone: I checked git fetch -h, where the branch argument is just a <refspec> and fetched refs are written to FETCH_HEAD unless a destination ref is provided. When a modules metrics PR already exists for the month, the following git checkout "$BRANCH_NAME" can fail with pathspec ... did not match, so reruns cannot update the open PR; fetch into refs/remotes/origin/$BRANCH_NAME or check out FETCH_HEAD with -B.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/usage-metrics-modules.yml:
- Around line 37-60: Inline interpolation of github.event.inputs.modules in the
workflow shell block can be interpreted as shell code, so move the modules input
into an env variable and read it from the script instead. Update the
usage-metrics-modules workflow’s run step to reference a safe variable such as
MODULES_INPUT, then assign MODULES from that env var before the existing
MODULE_FLAGS parsing logic in the shell block. Keep the existing module parsing
and go run collect.go invocation unchanged apart from sourcing the input from
env.
In `@usage-metrics/collect.go`:
- Around line 182-186: The CSV write loop in collect.go is currently swallowing
appendToCSV failures by logging a warning and continuing, which can leave
partial or stale output while still reporting success. Update the
results-processing path around appendToCSV so that a write error is treated as
fatal: stop processing immediately and propagate the error back to the caller
instead of continuing the loop. Use the existing appendToCSV and results
iteration in the collection flow to locate the change, and ensure the
surrounding function returns the failure so the overall operation fails fast.
- Around line 262-267: The sort comparator in sort.SliceStable on data assumes
every row has at least two columns, which can panic on malformed input. Add a
pre-sort validation step in the collect.go flow to check each row’s width before
accessing data[i][0] and data[i][1], and handle invalid rows by skipping them or
returning an error; use the data slice and the sort.SliceStable comparator as
the key locations to update.
- Around line 216-220: The gh api invocation in collect.go can hang indefinitely
because exec.Command is used without a deadline. Update the external call in the
collection flow to use a context with timeout (around the gh api call that
produces output), and thread that context through the command creation so hung
requests are canceled; keep the fix localized around the gh api execution path
used by the collection logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 545d1962-8133-4daa-b15d-b3614335f54c
⛔ Files ignored due to path filters (2)
docs/usage-metrics/core.csvis excluded by!**/*.csvdocs/usage-metrics/modules.csvis excluded by!**/*.csv
📒 Files selected for processing (14)
.github/workflows/usage-metrics-modules.yml.github/workflows/usage-metrics.ymldocs/css/usage-metrics.cssdocs/js/modules-usage-metrics.jsdocs/js/usage-metrics.jsdocs/usage-metrics/index.mddocs/usage-metrics/modules.mdmkdocs.ymlmodulegen/internal/mkdocs/types.goscripts/changed-modules.shusage-metrics/README.mdusage-metrics/collect-metrics.gousage-metrics/collect.gousage-metrics/collect_test.go
💤 Files with no reviewable changes (1)
- usage-metrics/collect-metrics.go
| run: | | ||
| # Build module flags array | ||
| MODULE_FLAGS=() | ||
| MODULES="${{ github.event.inputs.modules }}" | ||
| if [ -z "$MODULES" ]; then | ||
| # Discover all modules by listing subdirectories under modules/ that contain a go.mod file | ||
| while IFS= read -r go_mod_path; do | ||
| module="${go_mod_path#../modules/}" | ||
| module="${module%/go.mod}" | ||
| [ -n "$module" ] && MODULE_FLAGS+=("-module" "$module") | ||
| done < <(find ../modules -mindepth 2 -maxdepth 2 -name go.mod | sort) | ||
| else | ||
| IFS=',' read -ra MODULE_ARRAY <<< "$MODULES" | ||
| for module in "${MODULE_ARRAY[@]}"; do | ||
| module="${module#"${module%%[![:space:]]*}"}" # trim leading whitespace | ||
| module="${module%"${module##*[![:space:]]}"}" # trim trailing whitespace | ||
| [ -n "$module" ] && MODULE_FLAGS+=("-module" "$module") | ||
| done | ||
| fi | ||
|
|
||
| echo "Querying modules: ${MODULE_FLAGS[*]}" | ||
|
|
||
| # Query all modules in a single command | ||
| go run collect.go modules "${MODULE_FLAGS[@]}" -csv "../docs/usage-metrics/modules.csv" |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Avoid template injection: pass inputs.modules via env, not inline interpolation.
${{ github.event.inputs.modules }} is expanded directly into the shell script body at Line 40, so a crafted workflow_dispatch input is interpreted as shell code rather than data. Bind it to an env var and reference it with $MODULES_INPUT instead.
🔒 Proposed fix
- name: Query modules
id: query
working-directory: usage-metrics
+ env:
+ MODULES_INPUT: ${{ github.event.inputs.modules }}
run: |
# Build module flags array
MODULE_FLAGS=()
- MODULES="${{ github.event.inputs.modules }}"
+ MODULES="$MODULES_INPUT"🧰 Tools
🪛 zizmor (1.26.1)
[error] 40-40: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/usage-metrics-modules.yml around lines 37 - 60, Inline
interpolation of github.event.inputs.modules in the workflow shell block can be
interpreted as shell code, so move the modules input into an env variable and
read it from the script instead. Update the usage-metrics-modules workflow’s run
step to reference a safe variable such as MODULES_INPUT, then assign MODULES
from that env var before the existing MODULE_FLAGS parsing logic in the shell
block. Keep the existing module parsing and go run collect.go invocation
unchanged apart from sourcing the input from env.
Source: Linters/SAST tools
| for item, count := range results { | ||
| if err := appendToCSV(csvPath, column, date, item, count); err != nil { | ||
| log.Printf("Warning: failed to write %s=%s: %v", column, item, err) | ||
| continue | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Fail fast on CSV write errors instead of silently continuing.
Line 182-Line 186 currently logs write failures and continues, which can produce partial/stale datasets while returning success.
Suggested fix
for item, count := range results {
if err := appendToCSV(csvPath, column, date, item, count); err != nil {
- log.Printf("Warning: failed to write %s=%s: %v", column, item, err)
- continue
+ return fmt.Errorf("write %s=%s: %w", column, item, err)
}
fmt.Printf("Successfully recorded: %s=%s has %d usages on %s\n", column, item, count, date)
}As per coding guidelines, data write failures that can silently corrupt downstream outputs should be treated as major integrity issues.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for item, count := range results { | |
| if err := appendToCSV(csvPath, column, date, item, count); err != nil { | |
| log.Printf("Warning: failed to write %s=%s: %v", column, item, err) | |
| continue | |
| } | |
| for item, count := range results { | |
| if err := appendToCSV(csvPath, column, date, item, count); err != nil { | |
| return fmt.Errorf("write %s=%s: %w", column, item, err) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@usage-metrics/collect.go` around lines 182 - 186, The CSV write loop in
collect.go is currently swallowing appendToCSV failures by logging a warning and
continuing, which can leave partial or stale output while still reporting
success. Update the results-processing path around appendToCSV so that a write
error is treated as fatal: stop processing immediately and propagate the error
back to the caller instead of continuing the loop. Use the existing appendToCSV
and results iteration in the collection flow to locate the change, and ensure
the surrounding function returns the failure so the overall operation fails
fast.
Source: Coding guidelines
| output, err := exec.Command("gh", "api", | ||
| "-H", "Accept: application/vnd.github+json", | ||
| "-H", "X-GitHub-Api-Version: 2022-11-28", | ||
| endpoint, | ||
| ).Output() |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Add a timeout to gh api calls to avoid hung collection runs.
Line 216-Line 220 runs an external process without a deadline; if it hangs, the job can stall until workflow timeout.
Suggested fix
import (
+ "context"
"encoding/csv"
"encoding/json"
"errors"
@@
func runGHSearch(query string) (int, error) {
params := url.Values{}
params.Add("q", query)
endpoint := "/search/code?" + params.Encode()
- output, err := exec.Command("gh", "api",
+ ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
+ defer cancel()
+
+ output, err := exec.CommandContext(ctx, "gh", "api",
"-H", "Accept: application/vnd.github+json",
"-H", "X-GitHub-Api-Version: 2022-11-28",
endpoint,
).Output()
if err != nil {
+ if errors.Is(ctx.Err(), context.DeadlineExceeded) {
+ return 0, fmt.Errorf("gh api timeout: %w", ctx.Err())
+ }
exitErr := &exec.ExitError{}
if errors.As(err, &exitErr) {
return 0, fmt.Errorf("gh api failed: %s", string(exitErr.Stderr))
}As per coding guidelines, blocking external calls without timeouts are release-blocking reliability hazards.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| output, err := exec.Command("gh", "api", | |
| "-H", "Accept: application/vnd.github+json", | |
| "-H", "X-GitHub-Api-Version: 2022-11-28", | |
| endpoint, | |
| ).Output() | |
| ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) | |
| defer cancel() | |
| output, err := exec.CommandContext(ctx, "gh", "api", | |
| "-H", "Accept: application/vnd.github+json", | |
| "-H", "X-GitHub-Api-Version: 2022-11-28", | |
| endpoint, | |
| ).Output() | |
| if err != nil { | |
| if errors.Is(ctx.Err(), context.DeadlineExceeded) { | |
| return 0, fmt.Errorf("gh api timeout: %w", ctx.Err()) | |
| } | |
| exitErr := &exec.ExitError{} | |
| if errors.As(err, &exitErr) { | |
| return 0, fmt.Errorf("gh api failed: %s", string(exitErr.Stderr)) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@usage-metrics/collect.go` around lines 216 - 220, The gh api invocation in
collect.go can hang indefinitely because exec.Command is used without a
deadline. Update the external call in the collection flow to use a context with
timeout (around the gh api call that produces output), and thread that context
through the command creation so hung requests are canceled; keep the fix
localized around the gh api execution path used by the collection logic.
Source: Coding guidelines
| sort.SliceStable(data, func(i, j int) bool { | ||
| if data[i][0] != data[j][0] { | ||
| return data[i][0] < data[j][0] | ||
| } | ||
| return data[i][1] < data[j][1] | ||
| }) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Validate row width before sorting to prevent index panics.
Line 262-Line 267 indexes column 0/1 directly; malformed CSV rows can cause runtime panics.
Suggested fix
header := records[0]
data := records[1:]
+ for i, row := range data {
+ if len(row) < 2 {
+ return fmt.Errorf("invalid csv row %d: expected at least 2 columns, got %d", i+2, len(row))
+ }
+ }
sort.SliceStable(data, func(i, j int) bool {
if data[i][0] != data[j][0] {
return data[i][0] < data[j][0]As per coding guidelines, missing boundary checks that can trigger out-of-bounds runtime failures are critical runtime pitfalls to address.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| sort.SliceStable(data, func(i, j int) bool { | |
| if data[i][0] != data[j][0] { | |
| return data[i][0] < data[j][0] | |
| } | |
| return data[i][1] < data[j][1] | |
| }) | |
| for i, row := range data { | |
| if len(row) < 2 { | |
| return fmt.Errorf("invalid csv row %d: expected at least 2 columns, got %d", i+2, len(row)) | |
| } | |
| } | |
| sort.SliceStable(data, func(i, j int) bool { | |
| if data[i][0] != data[j][0] { | |
| return data[i][0] < data[j][0] | |
| } | |
| return data[i][1] < data[j][1] | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@usage-metrics/collect.go` around lines 262 - 267, The sort comparator in
sort.SliceStable on data assumes every row has at least two columns, which can
panic on malformed input. Add a pre-sort validation step in the collect.go flow
to check each row’s width before accessing data[i][0] and data[i][1], and handle
invalid rows by skipping them or returning an error; use the data slice and the
sort.SliceStable comparator as the key locations to update.
Source: Coding guidelines
1 participant
What does this PR do?
Adds a complete modules usage metrics system alongside the existing core library metrics:
collect.go: unified CLI replacing the two separate scripts (collect-metrics.go/collect-module-metrics.go); runs ascollect versionsorcollect moduleswith a shared retry/CSV/sort enginedocs/usage-metrics/modules.csv: new CSV tracking monthly GitHub import counts per module (65 modules)docs/usage-metrics/modules.md: interactive dashboard with stats cards, total/trend/latest charts, and a filterable per-module chart grid (one line chart per module, sorted alphabetically)docs/usage-metrics/index.md: core library dashboard moved here so the "Usage Metrics" nav entry is a direct link.github/workflows/usage-metrics-modules.yml: monthly workflow that auto-discovers all modules undermodules/, queries GitHub Code Search, and opens a PR with the updated CSVusage-metrics/collect_test.go: unit tests for all non-network logic (error classification, CSV append/sort, collect retry logic)scripts/changed-modules.sh: updated excluded files list to include the new CSV paths and the new workflowWhy is it important?
Gives the project visibility into which individual modules are being adopted, not just the core library version. The per-module charts make it easy to spot trends, compare relative popularity, and track growth over time.
Related issues