Skip to content

Conversation

@mdelapenya
Copy link
Member

Summary

Implements an automated system to track and visualize testcontainers-go adoption across GitHub repositories. The system queries GitHub's Code Search API for usage in go.mod
files, stores historical data, and displays interactive charts on the documentation site.

Features

  • Automated Data Collection: GitHub Actions workflow that runs monthly (1st of each month at 9 AM UTC)
  • Historical Tracking: CSV-based storage of usage metrics over time
  • Interactive Dashboard: Integrated into MkDocs documentation with Chart.js visualizations
  • Version Tracking: Monitors all versions from v0.13.0 to latest (currently v0.39.0)
  • Manual Triggers: Support for on-demand collection via workflow_dispatch

Components Added

Data Collection (usage-metrics/)

  • collect-metrics.go: Go program that queries GitHub Code Search API
    • Accepts multiple -version flags for efficient batch processing
    • Built-in rate limiting (2s between requests)
    • Excludes forks and testcontainers organization repos
  • README.md: Complete documentation for the metrics system

Visualization (docs/)

  • usage-metrics.md: Dashboard page integrated into documentation
  • js/usage-metrics.js: Interactive charts powered by Chart.js
    • Usage trends over time (line chart)
    • Version comparison (bar chart)
    • Distribution by version (doughnut chart)
  • css/usage-metrics.css: Responsive styling for dashboard

Automation (.github/workflows/)

  • usage-metrics.yml: GitHub Actions workflow
    • Runs monthly on the 1st at 9 AM UTC
    • Creates pull requests for metrics updates (not direct commits)
    • Adds date to commit messages for tracking
    • Can be manually triggered with custom version list

Data Storage

  • docs/usage-metrics.csv: Historical usage data (758 initial records)
    • Format: date,version,count
    • Version-controlled for historical tracking
    • Serves as data source for dashboard

CI/CD Integration

  • Updated scripts/changed-modules.sh:
    • Added docs/usage-metrics.csv to excluded files
    • Added .github/workflows/usage-metrics.yml to excluded files
    • Prevents unnecessary CI runs for metrics-only changes

How It Works

  1. Monthly workflow triggers (or manual trigger)
  2. Script queries GitHub API for each version's usage in go.mod files
  3. Results are appended to CSV with timestamp
  4. Creates a PR with changes (format: chore(metrics): update usage metrics (YYYY-MM-DD))
  5. Dashboard automatically reflects new data when PR is merged
  6. Data is deployed with the main MkDocs documentation site

Viewing the Dashboard

Test Plan

  • Initial data collected for all versions (v0.13.0 to v0.40.0)
  • Dashboard displays correctly with all charts
  • Workflow can be manually triggered via workflow_dispatch
  • Verify monthly scheduled run creates a PR successfully
  • Confirm CI doesn't trigger for metrics-only changes
  • Test dashboard on production site after merge

Copilot AI and others added 5 commits November 20, 2025 09:56
- Create Go script to query GitHub Code Search API for usage metrics
- Add CSV storage for historical data (all versions from v0.13.0 to v0.39.0)
- Integrate usage metrics dashboard into MkDocs documentation
- Add interactive charts (trend, version comparison, distribution)
- Create GitHub Actions workflow for automated weekly collection
- Support manual workflow trigger with custom version queries

Co-authored-by: mdelapenya <[email protected]>
@mdelapenya mdelapenya self-assigned this Nov 20, 2025
@mdelapenya mdelapenya requested a review from a team as a code owner November 20, 2025 16:51
@mdelapenya mdelapenya added the documentation Docs, docs, docs. label Nov 20, 2025
@netlify
Copy link

netlify bot commented Nov 20, 2025

Deploy Preview for testcontainers-go ready!

Name Link
🔨 Latest commit 2f53de7
🔍 Latest deploy log https://app.netlify.com/projects/testcontainers-go/deploys/692588da169a310008cb2447
😎 Deploy Preview https://deploy-preview-3495--testcontainers-go.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@coderabbitai
Copy link

coderabbitai bot commented Nov 20, 2025

Summary by CodeRabbit

  • New Features

    • Added a usage metrics dashboard displaying repository usage statistics, trends, and version comparisons with interactive charts and stat cards.
    • Implemented automated monthly metrics collection and dashboard updates via scheduled workflow.
  • Chores

    • Updated Docker action dependency version.
    • Added metrics asset configuration to build system.

✏️ Tip: You can customize this high-level summary in your review settings.

Walkthrough

Adds an automated usage-metrics pipeline: a Go collector that queries GitHub Code Search, a scheduled/manual GitHub Actions workflow to run it, CSV storage at docs/usage-metrics.csv, MkDocs dashboard assets (JS/CSS/MD), a usage-metrics submodule, and small CI/config updates.

Changes

Cohort / File(s) Summary
CI workflow & build rules
**.github/workflows/usage-metrics.yml**, **scripts/changed-modules.sh**, **.gitignore**
New scheduled/manual "Update Usage Metrics" workflow that runs the collector, generates/updates docs/usage-metrics.csv, commits changes to a chore branch and opens a PR; added workflow and CSV to exclusion list; ignored usage-metrics/scripts/collect-metrics binary.
Metrics collector (Go)
usage-metrics module: **usage-metrics/collect-metrics.go**, **usage-metrics/go.mod**, **usage-metrics/README.md**
New Go module and CLI accepting multiple -version flags and -csv path; queries GitHub code search via gh with retry/backoff and rate-limit handling, aggregates date/version/count, and appends rows to docs/usage-metrics.csv; README documents usage.
Website dashboard assets
**docs/js/usage-metrics.js**, **docs/css/usage-metrics.css**, **docs/usage-metrics.md**
Client-side dashboard: CSV loading (Papa Parse), aggregation, stat cards, three Chart.js visualizations, responsive CSS, and static markup for the metrics page.
Docs configuration
**mkdocs.yml**
Registered usage-metrics page and added assets: local JS/CSS and external libraries (Chart.js, date adapter, PapaParse); added nav entry for the usage-metrics page.
Internal config types
**modulegen/internal/mkdocs/types.go**
Added ExtraJavascript []string (yaml:"extra_javascript") and UsageMetrics string (yaml:"Usage Metrics,omitempty") fields to MkDocs config types.
Misc CI tweak
**.github/workflows/ci-test-go.yml**
Updated docker/setup-docker-action reference to a newer commit hash.
Makefile include
**usage-metrics/Makefile**
Included shared test Makefile ../commons-test.mk.

Sequence Diagram(s)

sequenceDiagram
    participant Scheduler as GitHub Scheduler
    participant Workflow as Actions workflow
    participant Collector as collect-metrics.go
    participant GH as GitHub API (gh)
    participant CSV as docs/usage-metrics.csv
    participant Git as Git (branch/PR)

    Scheduler->>Workflow: Trigger (monthly or manual)
    Workflow->>Workflow: Checkout, setup Go
    Workflow->>Collector: Run with resolved -version flags
    Collector->>GH: Code search queries (retry/backoff)
    GH-->>Collector: Return counts
    Collector->>CSV: Append rows (date, version, count)
    Workflow->>Git: Create branch, commit & push CSV
    Workflow->>Git: Open PR to main
Loading
sequenceDiagram
    participant User as Browser
    participant Page as usage-metrics page
    participant JS as usage-metrics.js
    participant CSV as docs/usage-metrics.csv
    participant Chart as Chart.js

    User->>Page: Load
    Page->>JS: Init
    JS->>CSV: Fetch CSV
    CSV-->>JS: Rows
    JS->>JS: Parse & aggregate (by date/version)
    JS->>Chart: Render trend, version, latest charts
    JS->>Page: Show stats & update time
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • Pay extra attention to:
    • Retry/backoff and rate-limit handling in usage-metrics/collect-metrics.go
    • CSV append behavior, header creation, and absolute path resolution in the collector
    • Workflow logic for version resolution, branch naming, and PR creation via gh
    • Chart.js instance lifecycle and CSV parsing/error handling in docs/js/usage-metrics.js
    • MkDocs integration (mkdocs.yml + modulegen/internal/mkdocs/types.go) to ensure assets/nav are injected correctly

Possibly related PRs

Suggested reviewers

  • stevenh

Poem

🐰 I hopped through code and counted each crate,

Versions tallied, dates aligned by fate.
Charts sprout like carrots, bright and neat,
CSV crumbs lead to dashboards sweet.
Hooray — metrics gathered, a tasty treat!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 15.38% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately reflects the main objective: automating usage metrics collection and publishing results in the documentation site.
Description check ✅ Passed The description is comprehensive and directly related to the changeset, detailing the automated metrics system, components added, workflow automation, and test plan.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (5)
usage-metrics/README.md (1)

7-7: Wrap bare URLs in markdown link syntax.

Lines 7, 98, and 99 contain bare URLs that should be wrapped in markdown link format for better compliance and accessibility. For example:

- Line 7: [GitHub Code Search](https://github.com/search?q=...)
- Line 98: [Production](https://golang.testcontainers.org/usage-metrics/)
- Line 99: [Local](http://localhost:8000/usage-metrics/)

Also applies to: 98-98, 99-99

.github/workflows/usage-metrics.yml (1)

32-60: Add defensive quoting to shell variable expansion.

The shellcheck warning about SC2086 is valid here. While the current code works (word splitting is intentional), adding quotes provides better robustness against unexpected input:

- go run collect-metrics.go $VERSION_FLAGS -csv "../../docs/usage-metrics.csv"
+ go run collect-metrics.go $VERSION_FLAGS -csv "../../docs/usage-metrics.csv"

Actually, for maximum safety with multiple flags, consider using an array:

# Earlier in script:
VERSION_FLAGS=()
for version in "${VERSION_ARRAY[@]}"; do
  version=$(echo "$version" | xargs)
  if [ -z "$version" ]; then
    continue
  fi
  VERSION_FLAGS+=("-version" "$version")
done

# Then invoke:
go run collect-metrics.go "${VERSION_FLAGS[@]}" -csv "../../docs/usage-metrics.csv"

This eliminates word-splitting concerns entirely.

docs/js/usage-metrics.js (1)

64-103: Handle empty datasets to avoid “Latest Version: undefined” UI

If the CSV is ever empty (or filtered to no rows), versions becomes empty, which makes latestVersion undefined while still rendering the stats grid and latest‑usage doughnut (with zero data). Consider an upfront guard that detects versions.length === 0, shows a “no data available” message, and skips stats + charts, so the UI degrades more cleanly.

Also applies to: 262-321

usage-metrics/collect-metrics.go (2)

142-164: Avoid shell‑constructed gh api command; use exec.Command arguments instead

queryGitHubUsage builds a single shell string and runs sh -c with interpolated endpoint. Even though version strings are expected to be well‑formed tags, this is brittle (quoting/escaping issues) and opens the door to command injection if a malformed version ever sneaks in. Consider switching to something like:

cmd := exec.Command(
	"gh", "api",
	"-H", "Accept: application/vnd.github+json",
	"-H", "X-GitHub-Api-Version: 2022-11-28",
	endpoint,
)
output, err := cmd.Output()

This keeps arguments properly separated and avoids shell interpretation entirely.


54-96: Optional robustness: dedupe versions before querying and consider UTC date

Two small, non‑blocking robustness tweaks you might consider in collectMetrics:

  • If callers accidentally pass the same -version multiple times, you’ll query and append duplicate metrics for that version/date. A simple map[string]struct{} dedupe step before the loop would avoid redundant GitHub calls and duplicate CSV rows.
  • date := time.Now().Format("2006-01-02") uses the host’s local timezone; for scheduled runs in CI you likely want a stable notion of “collection day” (e.g., time.Now().UTC()) so dates don’t skew if the runner’s locale changes.

Neither is critical, but both make behavior a bit more predictable over time.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 00f2020 and 9c57543.

⛔ Files ignored due to path filters (1)
  • docs/usage-metrics.csv is excluded by !**/*.csv
📒 Files selected for processing (10)
  • .github/workflows/usage-metrics.yml (1 hunks)
  • .gitignore (1 hunks)
  • docs/css/usage-metrics.css (1 hunks)
  • docs/js/usage-metrics.js (1 hunks)
  • docs/usage-metrics.md (1 hunks)
  • mkdocs.yml (2 hunks)
  • scripts/changed-modules.sh (2 hunks)
  • usage-metrics/README.md (1 hunks)
  • usage-metrics/collect-metrics.go (1 hunks)
  • usage-metrics/go.mod (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-18T08:24:27.479Z
Learnt from: mdelapenya
Repo: testcontainers/testcontainers-go PR: 3254
File: .github/dependabot.yml:21-21
Timestamp: 2025-09-18T08:24:27.479Z
Learning: In the testcontainers-go repository, submodules like atlaslocal that are part of a parent module (e.g., mongodb) share the same go.mod file and should not have separate Dependabot entries. They are already monitored through the parent module's Dependabot configuration entry.

Applied to files:

  • usage-metrics/go.mod
  • usage-metrics/README.md
  • mkdocs.yml
🪛 actionlint (1.7.8)
.github/workflows/usage-metrics.yml

32-32: shellcheck reported issue in this script: SC2086:info:28:27: Double quote to prevent globbing and word splitting

(shellcheck)

🪛 LanguageTool
usage-metrics/README.md

[uncategorized] ~31-~31: The official name of this software platform is spelled with a capital “H”.
Context: ... mobile and desktop ### 🤖 Automation (.github/workflows/usage-metrics.yml) - Runs mo...

(GITHUB)


[style] ~180-~180: Consider using a different verb for a more formal wording.
Context: ...le ## Contributing To add features or fix issues: 1. Test changes locally with `...

(FIX_RESOLVE)

🪛 markdownlint-cli2 (0.18.1)
usage-metrics/README.md

7-7: Bare URL used

(MD034, no-bare-urls)


98-98: Bare URL used

(MD034, no-bare-urls)


99-99: Bare URL used

(MD034, no-bare-urls)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
  • GitHub Check: lint (modules/yugabytedb) / lint: modules/yugabytedb
  • GitHub Check: lint (modules/solace) / lint: modules/solace
  • GitHub Check: lint (modules/socat) / lint: modules/socat
  • GitHub Check: lint (modules/scylladb) / lint: modules/scylladb
  • GitHub Check: lint (modules/dockermcpgateway) / lint: modules/dockermcpgateway
  • GitHub Check: lint (modules/meilisearch) / lint: modules/meilisearch
  • GitHub Check: lint (modules/nats) / lint: modules/nats
  • GitHub Check: lint (modules/neo4j) / lint: modules/neo4j
  • GitHub Check: lint (modules/k3s) / lint: modules/k3s
  • GitHub Check: lint / lint:
  • GitHub Check: lint (modules/ollama) / lint: modules/ollama
  • GitHub Check: lint (modules/nebulagraph) / lint: modules/nebulagraph
  • GitHub Check: lint (modules/inbucket) / lint: modules/inbucket
  • GitHub Check: lint (modules/memcached) / lint: modules/memcached
  • GitHub Check: lint (modules/compose) / lint: modules/compose
  • GitHub Check: lint (modules/milvus) / lint: modules/milvus
  • GitHub Check: lint (modules/gcloud) / lint: modules/gcloud
  • GitHub Check: lint (modules/localstack) / lint: modules/localstack
  • GitHub Check: lint (modules/databend) / lint: modules/databend
  • GitHub Check: Analyze (go)
🔇 Additional comments (11)
usage-metrics/go.mod (1)

1-3: Module definition looks correct.

The usage-metrics submodule is properly scoped and Go version is current. Dependencies will be managed by go.mod when the collect-metrics.go program is compiled.

.gitignore (1)

27-28: Verify binary output path.

The .gitignore entry references usage-metrics/scripts/collect-metrics, but based on the workflow (which runs from usage-metrics directory and builds collect-metrics.go), the binary should be at usage-metrics/collect-metrics (without the scripts/ subdirectory). Confirm the correct path to ignore.

scripts/changed-modules.sh (1)

89-89: Exclusions properly aligned with system design.

Both additions—the usage-metrics workflow and the generated CSV file—are appropriately excluded from triggering module builds, consistent with the established pattern for documentation and automation files.

Also applies to: 102-102

mkdocs.yml (1)

22-27: Configuration additions are well-structured.

The externally sourced libraries are pinned to specific versions for stability, and the local assets are properly referenced. Navigation entry is correctly positioned.

Also applies to: 157-157

docs/usage-metrics.md (1)

1-30: Static structure is well-designed for dynamic population.

The HTML markup provides appropriate hooks (element IDs, semantic structure) for the JavaScript runtime to populate with data. Loading and error states are properly provisioned.

.github/workflows/usage-metrics.yml (2)

36-43: Verify git tag extraction logic handles edge cases.

The tag extraction pipeline uses several transformations (grep, sed, sort, awk). While the logic appears sound, test these scenarios:

  1. When no tags exist matching the pattern
  2. When v0.13.0 doesn't exist (will awk still work correctly?)
  3. When there are pre-release tags (e.g., v0.40.0-rc1) in the repo

The current grep pattern v0\.[0-9]+\.[0-9]+$ will correctly exclude pre-releases, which is good. However, verify that the awk line /v0.13.0/,0 correctly handles the case where v0.13.0 might not exist.


62-85: PR creation flow is solid.

The branch naming convention is clear, commit messages are descriptive, and the PR body provides context. The workflow correctly checks for staged changes before committing to avoid empty commits.

docs/css/usage-metrics.css (1)

1-111: CSS is well-structured and responsive.

The stylesheet provides coherent styling across multiple components with appropriate responsive breakpoints. Grid layout for stats, proper shadow and radius use for depth, and mobile considerations are all in place. Color contrast and accessibility appear adequate.

docs/js/usage-metrics.js (2)

10-33: Solid data loading, processing, and init flow

The CSV fetch, Papa.parse usage, sorting/grouping, and single‑run init guard form a clean, resilient pipeline. Error handling with showError plus resetting isInitialized on failure is a good touch, and the page‑presence check via trendChart keeps this script from doing work on unrelated pages.

Also applies to: 45-63, 323-362


105-175: Verify Chart.js time‑scale adapter is wired correctly

Both the trend and total‑repositories charts rely on type: 'time' for the x‑axis and x: d.date values. This assumes the Chart.js time adapter is loaded/registered in the page and can parse your YYYY-MM-DD strings. Please double‑check the docs page includes the appropriate adapter bundle/registration for the Chart.js major version you’re using so these axes render correctly.

Also applies to: 177-260

usage-metrics/collect-metrics.go (1)

166-195: CSV append strategy is straightforward and safe for single-writer use

appendToCSV correctly resolves an absolute path, creates the file if missing, conditionally writes the header, and appends a single row per metric. For the current usage pattern (single CI job appending once per version per run) this is perfectly adequate and keeps the implementation simple.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (2)
modulegen/internal/mkdocs/types.go (2)

25-27: ExtraJavascript field wiring looks correct; naming nit only

The YAML tag extra_javascript matches MkDocs’ config key, so this will correctly round‑trip additional JS assets. If you care about Go identifier style, you might consider renaming the field to ExtraJavaScript for conventional capitalization, but behavior is already correct as‑is.


30-41: Adding Usage Metrics nav: confirm addModule’s hard‑coded indices still point at the right items

addModule assumes c.Nav[3].Modules and c.Nav[4].Examples are the “Modules” and “Examples” entries. With a new Usage Metrics nav item added in mkdocs.yml, this will break if the new entry is inserted before either of those and shifts their indices.

Two follow‑ups to consider:

  • Verify that in mkdocs.yml the new Usage Metrics item is appended after the existing “Modules”/“Examples” entries so indices 3 and 4 are unchanged.
  • Longer term, avoid magic indices by locating the correct nav entry by key (e.g., scanning c.Nav for the item where len(Modules) > 0 or len(Examples) > 0) before mutating it, so future nav additions don’t silently misplace generated links.

Also applies to: 48-79

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9c57543 and 2ea4802.

📒 Files selected for processing (1)
  • modulegen/internal/mkdocs/types.go (2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
  • GitHub Check: lint (modules/artemis) / lint: modules/artemis
  • GitHub Check: lint (modules/mariadb) / lint: modules/mariadb
  • GitHub Check: lint (modules/dockermodelrunner) / lint: modules/dockermodelrunner
  • GitHub Check: lint (modules/influxdb) / lint: modules/influxdb
  • GitHub Check: lint (modules/vault) / lint: modules/vault
  • GitHub Check: lint (modules/pinecone) / lint: modules/pinecone
  • GitHub Check: lint (modules/mssql) / lint: modules/mssql
  • GitHub Check: lint (modules/elasticsearch) / lint: modules/elasticsearch
  • GitHub Check: Analyze (go)

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🧹 Nitpick comments (1)
usage-metrics/collect-metrics.go (1)

83-86: Lexicographic sort produces incorrect version ordering

String comparison doesn't handle semantic versioning correctly. For example, "v0.9.0" < "v0.10.0" is false lexicographically, so v0.10.0 would sort before v0.9.0.

Consider using a semver-aware comparison or parsing version components numerically:

sort.Slice(metrics, func(i, j int) bool {
    // Simple approach: pad version numbers for correct string sort
    // Or use a semver library like golang.org/x/mod/semver
    return semver.Compare(metrics[i].Version, metrics[j].Version) < 0
})

Alternatively, if adding a dependency is undesirable, parse and compare major/minor/patch components as integers.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f9b5a8d and 1ed86fa.

📒 Files selected for processing (3)
  • scripts/changed-modules.sh (3 hunks)
  • usage-metrics/Makefile (1 hunks)
  • usage-metrics/collect-metrics.go (1 hunks)
🧰 Additional context used
🪛 checkmake (0.2.2)
usage-metrics/Makefile

[warning] 1-1: Missing required phony target "all"

(minphony)


[warning] 1-1: Missing required phony target "clean"

(minphony)


[warning] 1-1: Missing required phony target "test"

(minphony)

🪛 Shellcheck (0.11.0)
scripts/changed-modules.sh

[warning] 133-133: Quote to prevent word splitting/globbing, or split robustly with mapfile or read -a.

(SC2206)


[warning] 133-133: Quote to prevent word splitting/globbing, or split robustly with mapfile or read -a.

(SC2206)


[warning] 133-133: Quote to prevent word splitting/globbing, or split robustly with mapfile or read -a.

(SC2206)

🔇 Additional comments (2)
scripts/changed-modules.sh (2)

89-89: Exclusions for usage-metrics automation are well-placed.

Adding .github/workflows/usage-metrics.yml and docs/usage-metrics.csv to excluded files prevents metrics-only updates from triggering unnecessary module builds — consistent with the PR objective to suppress CI for metrics changes.

Also applies to: 102-102


129-133: Usage-metrics module integrated into build logic correctly.

The new usageMetricsModule follows the same pattern as modulegen (manually defined, not auto-discovered), which is appropriate for a root-level module. Its inclusion in allModules ensures that changes to the Go tool itself will trigger builds.

Please confirm that the usage-metrics directory has a valid Go module structure (e.g., usage-metrics/go.mod) so that the module is discoverable by downstream build logic.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
usage-metrics/collect-metrics.go (1)

89-95: Consider batching CSV writes to reduce file I/O overhead.

The current implementation opens and closes the CSV file for each metric record. While functionally correct and acceptable for the expected monthly run with ~30 versions, opening the file once and writing all records would be more efficient.

-	// Write all metrics to CSV
-	for _, metric := range metrics {
-		if err := appendToCSV(csvPath, metric); err != nil {
-			log.Printf("Warning: Failed to write metric for %s: %v", metric.Version, err)
-			continue
-		}
-		fmt.Printf("Successfully recorded: %s has %d usages on %s\n", metric.Version, metric.Count, metric.Date)
-	}
+	// Write all metrics to CSV in a single batch
+	if err := appendMetricsToCSV(csvPath, metrics); err != nil {
+		return fmt.Errorf("write metrics to CSV: %w", err)
+	}
+	for _, metric := range metrics {
+		fmt.Printf("Successfully recorded: %s has %d usages on %s\n", metric.Version, metric.Count, metric.Date)
+	}

This would require refactoring appendToCSV to accept a slice and write all records within a single file handle.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1ed86fa and 2f53de7.

📒 Files selected for processing (2)
  • usage-metrics/collect-metrics.go (1 hunks)
  • usage-metrics/go.mod (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • usage-metrics/go.mod
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: lint (modules/yugabytedb) / lint: modules/yugabytedb
  • GitHub Check: lint (examples/nginx) / lint: examples/nginx
  • GitHub Check: Analyze (go)
🔇 Additional comments (2)
usage-metrics/collect-metrics.go (2)

100-140: LGTM!

The retry logic with exponential backoff is now correctly implemented. All 5 backoff intervals are reachable, and the initial attempt plus retries are properly tracked.


142-168: LGTM!

The command injection vulnerability has been properly addressed by using exec.Command with explicit arguments instead of shell interpolation. Error handling with ExitError stderr extraction provides useful diagnostics.

Comment on lines +176 to +178
_, err = os.Stat(absPath)
fileExists := !os.IsNotExist(err)

Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Edge case in file existence check.

If os.Stat returns an error other than os.IsNotExist (e.g., permission denied on parent directory), fileExists will be true, potentially skipping the header write for a new file. While unlikely in practice (the subsequent OpenFile would likely fail too), consider handling this edge case explicitly.

 	_, err = os.Stat(absPath)
-	fileExists := !os.IsNotExist(err)
+	fileExists := err == nil

This ensures fileExists is only true when stat succeeds, and the header will be written for any stat failure scenario where the file is subsequently created.

🤖 Prompt for AI Agents
In usage-metrics/collect-metrics.go around lines 176–178, the current file
existence check treats any non-IsNotExist error as the file existing; change the
logic so fileExists is true only when os.Stat returned nil (e.g., fileExists :=
err == nil), and treat any non-nil stat error as "file does not exist" for the
purpose of writing the header (optionally log or return the stat error if you
want to surface permission/other errors separately before attempting OpenFile).

@mdelapenya mdelapenya merged commit f748575 into testcontainers:main Nov 25, 2025
215 checks passed
@mdelapenya mdelapenya deleted the copilot/automate-testcontainers-usage-metrics branch November 25, 2025 11:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Docs, docs, docs.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant