Skip to content

πŸ› fix(dashboard): the metrics collector reads each repo's GitHub metadata at most once an hour instead of every five minutes - #17175

Merged
hivecommons-hive[bot] merged 2 commits into
hivecommons:v6from
Danathar:fix/metrics-repo-metadata-hourly
Nov 8, 2026
Merged

hivecommons-hive[bot] merged 2 commits into
hivecommons:v6from
Danathar:fix/metrics-repo-metadata-hourly

Conversation

@Danathar

@Danathar Danathar commented Nov 8, 2026

Copy link
Copy Markdown
Contributor

Summary

The dashboard metrics collector re-read every watched repo's metadata (GET /repos/{owner}/{repo}) on every five-minute collect. Each 20-repo pass cost about 21 calls, and 177 of the 189 calls in the last hour were charged. All it uses from those reads is the outreach card's stars and forks and the Issues-disabled banner's has_issues/fork. Those change a few times a day. Each repo's metadata is now read at most once an hour, and one read of the primary repo serves both uses.

The report behind this PR guessed that the reads seldom got a 304 because repo metadata changes often. That part is wrong: the metadata barely changed. The missing 304s came from the ETag cache losing its entries, which #17172 fixes. This PR is still worth having on its own: it cuts about 170 requests an hour whether or not they would be free, and it removes the 20 charged reads that every App token refresh forces (about every 45 minutes).

What went wrong

Live reading from the projectbluefin hive (hosted-projectbluefin-knuckle, build dd6b372, fleet_status snapshot 2026-11-08T01:45:47Z, one-hour accounting window):

Caller Endpoint Requests Charged 304
hive:metrics_collector GET /repos/{owner}/{repo} 189 177 12
  • The window is one hour (restAccountingWindow), not 39 minutes as the report said.
  • The collector logged agent metrics collected every ~5 minutes, 12 times an hour. One pass reads the primary repo in collectOutreach, then all 20 watched repos again in collectIssuesDisabled (ProbeIssuesDisabled). That is 21 reads per pass, or 252 an hour.
  • The measured 189 is lower. Some passes' reads did not happen; the log shows stars: 0 for three collects while the budget was critical.

Why so few 304s. I re-read the 20 repos' metadata conditionally, with my own token and ETags (read-only, public data). 20 of 20 were unchanged after 10 minutes, and 18 of 20 after 15 minutes. With a working cache, about 97% of these reads would be free. Live, 6% were. The ETag cache was evicting them (#17172).

What else reads repo metadata. I left these on their own cadence on purpose:

  • The advisory issue's create path calls ProbeIssuesDisabled immediately before it creates. That is a correctness check, so it stays fresh.
  • contribute_standby_dispatch.go, forge/github_forge.go, main_eval_cycle.go and rfcnotify_wire.go read on demand for their own purposes and are not on the collector's loop.

The banner is informational only, and a failed read keeps its last verdict, so an hour of staleness can't cause wrong behaviour. At worst, the banner appears or clears up to an hour late after someone flips a repo's Issues setting.

What changes

  • MetricsCollector.repoMetadata holds each repo's successful GET /repos/{owner}/{repo} answer for repoMetadataRefreshInterval (one hour). The key is the repo name, lower-cased.
  • collectOutreach (stars and forks) and collectIssuesDisabled (has_issues/fork) both read through it, so the primary repo is read once for both.
  • A failed read is not held. The next collect asks again, and the banner keeps its previous verdict meanwhile, as before.
  • New github.IssuesDisabledFor classifies metadata that has already been read. ProbeIssuesDisabled uses it too, so both name the same cause and remedy.
  • Saving: up to about 230 requests an hour at the nominal 20-repo cadence (252 β†’ about 20), and about 170 at today's measured rate. The charged saving today is about 157 an hour until πŸ› fix(github): the ETag cache keeps the hive's repeat reads, so they come back as free 304s instead of being chargedΒ #17172 deploys. After that, it is the token-refresh misses, about 27 an hour.

Files

  • src/pkg/dashboard/metrics_collector.go: repoMetadata, the held metadata, and stars/forks read through it.
  • src/pkg/dashboard/metrics_issues_disabled.go: the banner check reads through repoMetadata.
  • src/pkg/github/advisory.go: IssuesDisabledFor.
  • src/docs/fleet-health.md: the banner clears within the hour of the fix.
  • changelog.d/fixed-metrics-repo-metadata-hourly.md

Tests

  • TestMetricsCollector_RepoMetadataReadHourly runs twelve collects five minutes apart and asserts that each repo is read once, that stars, forks and the banner verdict come from the held read, and that after an hour each repo is read again and a changed has_issues shows. On v6 tip it fails: the primary repo was read 24 times and each other repo 12 times in 55 minutes. It passes with the fix.
  • TestMetricsCollector_RepoMetadataFailureRetriesNextCollect checks that a failed read is retried on the next collect while the previous verdict stays up. It passes on tip and with the fix; it guards the fail-open behaviour.
  • The existing TestCollectIssuesDisabled_* tests still pass.

Full packages run: ./pkg/dashboard/, ./pkg/github/. Also run: go vet, check-docs-links.py, check-docs-citations.py.

How to verify live

After deploying, top_consumers should show hive:metrics_collector GET /repos/{owner}/{repo} at about 20 requests an hour (one per watched repo) instead of 189.

Refs #17172

πŸ€– Generated with Claude Code

https://claude.ai/code/session_01BQYnPGFJx81c8JXR3o1sg1

…ost hourly

collectOutreach (stars/forks) and collectIssuesDisabled (has_issues) read
GET /repos/{owner}/{repo} for the primary repo and every watched repo on
every five-minute collect: 189 calls an hour on the 20-repo projectbluefin
hive, 177 charged. Hold each successful read for an hour and share it
between both uses; a failed read is retried on the next collect. The
advisory create path keeps its fresh ProbeIssuesDisabled check.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BQYnPGFJx81c8JXR3o1sg1
Signed-off-by: Danathar <Danathar@users.noreply.github.com>
@kubestellar-prow kubestellar-prow Bot added the dco-signoff: yes Indicates the PR's author has signed the DCO. label Nov 8, 2026
@kubestellar-prow

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign kellyaa for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kubestellar-prow kubestellar-prow Bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Nov 8, 2026
@github-actions

github-actions Bot commented Nov 8, 2026

Copy link
Copy Markdown
Contributor

Open PR overlap check

Base branch: v6

Conflicts are checked with every PR merged onto the current tip of v6, so a PR that only needs a rebase is not reported as conflicting with its siblings.

No conflicting open PRs or same-file overlaps were found.

@hivecommons-hive hivecommons-hive Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Read metrics_collector.go, metrics_issues_disabled.go and github/advisory.go at the v6 merge-base alongside the diff. This looks correct to me.

What I checked:

  • repoMetadata (metrics_collector.go:748) only stores a successful, non-nil read and releases the mutex before the network call, so a failed read is retried on the next collect and a slow GitHub call doesn't hold the lock. The key is lower-cased owner/name, matching how collectIssuesDisabled already de-dupes via seen[full].
  • IssuesDisabledFor (advisory.go:80) is a pure extraction of the old body of ProbeIssuesDisabled (v6 advisory.go:75-78): same has_issues/fork logic, nil for nil metadata. ProbeIssuesDisabled still exists for the advisory create path, so that fresh check is unchanged.
  • Fail-open behaviour in collectIssuesDisabled is preserved: on error the previous verdict for that repo is carried forward exactly as before.
  • mc.clock() (metrics_collector.go:907) is the existing injectable time source, so the hour window is testable and the new test drives it.
  • collectOutreach already called gh.GetRepo on mc.client() without the inner-client nil guard ProbeIssuesDisabled had, and it runs on the same tick, so routing the banner through GetRepo adds no new exposure.

One observation, not a defect: since the primary repo is also in gh.Repositories(), the banner and the outreach card now share one read per hour for it, which is what the body describes.

β€” hive: agent=reviewer backend=copilot model=claude-fable-5.1 copilot=1.0.88

@hivecommons-hive

hivecommons-hive Bot commented Nov 8, 2026 •

Copy link
Copy Markdown
Contributor

Hive status β€” updated in place; edits do not notify. Full history is in the Activity Monitor.

Reviewed by reviewer-2 β€” no findings.

Reviewer notes

Read this against the v6 tree; it looks correct to me, and it is a smaller change than the +225 suggests (most of it is the test).

What I checked:

  • repoMetadata (metrics_collector.go) is the only new state. The map is guarded by repoMetaMu across both the read and the write, and the GitHub call happens outside the lock, so the one-per-hour guarantee holds for the sequential collect() path and would still be safe if collectOutreach/collectIssuesDisabled ever ran concurrently (worst case: two reads in the same instant, never a torn entry). Only a successful read is held, which is what the failure test asserts.
  • collectIssuesDisabled keeps the fail-open contract exactly: the err != nil β†’ keep prev[full] branch is unchanged, and ghpkg.IssuesDisabledFor is a pure extraction of the !GetHasIssues() check that ProbeIssuesDisabled still uses (advisory.go:72-85), so createAdvisoryIssue's fresh probe at advisory.go:212 is unaffected. One deliberate nuance worth knowing: a nil, nil answer from GetRepo used to mean "not disabled"; it now surfaces as an error and keeps the previous verdict. That is the safer direction and will not occur with a real GitHub response.
  • Key is lower-cased owner/name, so the primary repo read by collectOutreach (projectScope()) and the same repo from gh.Repositories()/SplitRepo share one entry β€” the "one read serves both" claim in the body holds, and the test's /repos/myorg/open count of 1 across both callers confirms it.
  • mc.clock()/mc.now (metrics_collector.go:907), issuesDisabledTestCollector, ErrNoGitHubClient and NewClientForTest all exist in v6 with the signatures the diff relies on; fmt/strings were already imported.
  • Docs: fleet-health.md item 7 and the changelog fragment describe the new hourly refresh and the fail-open retry accurately.

No findings.

Evidence bundle `#17175e3581c3` sha256:056b11b5394a3cd39f11fcb1fdbdb4fcbd789a1e314bcedda83b566f401f5d9e β€” download from the dashboard or `hivectl review evidence #17175`.

@hivecommons-hive
hivecommons-hive Bot merged commit 58581b6 into hivecommons:v6 Nov 8, 2026
69 of 70 checks passed
@hivecommons-hive

Copy link
Copy Markdown
Contributor

merged by Hive trusted-author auto-merge: author @Danathar holds role owner and GitHub permission maintain on repo; CI green at e3581c3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dco-signoff: yes Indicates the PR's author has signed the DCO. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant