reuse metrics response

[ahmadmuhamadi]

Fixes #139

Problem

In server mode, probeLiveAgencies fetches /api/where/metrics.json at the
top of the tick and keeps only entry.AgencyIDs. The per-agency loop then
reaches FetchObaAPIMetrics, which fetches the same URL again for each
agency. The endpoint takes no agency parameter, so all N+1 responses are
identical.

Change

probeLiveAgencies now returns the parsed response asw . It's held in
collectForServerScope and threaded through collectAgencyChecks into
FetchObaAPIMetrics, where a new prefetch *OBAMetrics parameter skips the
fetch when non-nil. Agency mode passes nil and is unchanged.

Emission logic is untouched. Removes the TODO(server-mode dedup) and the
matching doc-comment paragraph.

Result: /metrics.json requests per server-mode tick go from N+1 to 1.

Tests

TestServerScopeFetchesMetricsOncePerTick runs one server-mode tick over two
live agencies and asserts one request reaches the endpoint, and that both
agencies emit their own values from that response. The stub returns an empty
agencyIDs after the first call, so any refetch would trip the guard and emit
nothing. Verified failing on the old behaviour.

Summary by CodeRabbit

• Performance

  • Server-wide metrics collection now fetches shared metrics data once per collection interval, reducing repeated endpoint requests.
  • Existing agency-specific collection behavior remains unchanged.

• Bug Fixes

  • Metrics for each agency are now populated consistently from the shared response during server-scope collection.

• Tests

  • Added coverage confirming metrics are fetched only once per interval and correctly recorded for each agency.

[CLAassistant]

All committers have signed the CLA.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: f9b363e5-abbe-45d4-80db-682bfe235f83

📥 Commits

Reviewing files that changed from the base of the PR and between ac3a175 and fd2540a.

📒 Files selected for processing (1)

• internal/metrics/oba_rest_api_metrics_test.go

🚧 Files skipped from review as they are similar to previous changes (1)

• internal/metrics/oba_rest_api_metrics_test.go

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

Server-scope collection now reuses the parsed /api/where/metrics.json response for all live agencies in one tick. Agency mode retains direct fetching. Tests verify request deduplication and resulting gauge values.

Changes

Server metrics prefetch

Layer / File(s)	Summary
Metrics prefetch contract and processing internal/metrics/metrics_service.go, internal/metrics/oba_rest_api_metrics.go	FetchObaAPIMetrics accepts optional prefetched metrics. The fetch implementation reuses that data and preserves common metric recording.
Server-scope response wiring internal/app/metrics_collector.go	probeLiveAgencies returns parsed metrics. Server-scope agency checks receive the response, while agency mode passes nil.
Prefetch behavior validation internal/app/metrics_collector_test.go, internal/app/test_helpers.go, internal/metrics/oba_rest_api_metrics_test.go	Tests verify one metrics request per tick, gauge values from the prefetched response, and existing fetch behavior with the expanded function signature.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Merge Risk: ⚪ Minimal · up to fd254

Server-mode collection reuses a single metrics response across agencies, reducing redundant requests while agency-mode fetching remains unchanged. No merge-blocking risk is currently identified.

Sequence Diagram(s)

sequenceDiagram
  participant ServerScope
  participant probeLiveAgencies
  participant collectAgencyChecks
  participant FetchObaAPIMetrics
  ServerScope->>probeLiveAgencies: fetch metrics.json once
  probeLiveAgencies-->>ServerScope: live agencies and parsed OBAMetrics
  ServerScope->>collectAgencyChecks: pass prefetched OBAMetrics
  collectAgencyChecks->>FetchObaAPIMetrics: process each live agency
  FetchObaAPIMetrics-->>collectAgencyChecks: record agency metrics

Loading

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 43.75% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 16 functions across 6 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the main change: reusing the metrics response during server-mode collection.
Linked Issues check	✅ Passed	The changes satisfy issue #139. The implementation passes the parsed metrics response from probeLiveAgencies through the collection layers to FetchObaAPIMetrics, avoids repeated server-mode requests, …
Out of Scope Changes check	✅ Passed	All changes support the linked objective in issue #139. The production changes implement response reuse, and the test changes verify request deduplication and metric population. No unrelated changes a…

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coveralls]

coverage: 66.749% (+0.3%) from 66.46% — ahmadmuhamadi:fix/reuse-metrics-response into OneBusAway:main

[0xaboomar]

Hey @ahmadmuhamadi, The prefetch optimization looks good, great work!

There are just a few leftover comments and docs from the old behavior that should be cleaned up:

1. internal/app/metrics_collector.go:226-246: Remove the entire TODO(server-mode dedup) block. It describes the old N+1 fetch problem and proposes the same solution that is now implemented. The existing loop comment around lines 218-225 is still accurate and can stay.

2. internal/metrics/oba_rest_api_metrics.go:53-56: Update the stale N+1 documentation. It currently says that server mode performs the same /api/where/metrics.json request once per live agency and points to the TODO above. This is no longer true. Something like this would reflect the new behavior:

   // In server-mode this function is called once per live agency per tick,
   // but reuses the response prefetched by probeLiveAgencies instead of
   // issuing another HTTP request. Agency-mode callers pass nil and fetch
   // their own response.

3. internal/metrics/oba_rest_api_metrics.go:40-70: The function documentation also needs to be updated. It currently says the function always performs an HTTP GET and that client must be non-nil, which is no longer always true. It should explain that when prefetch is nil, the function fetches and decodes the endpoint itself; when prefetch is provided, it uses that response and skips the HTTP request. The prefetch parameter should also be documented, and client should be described as required only when prefetch == nil.

4. internal/app/metrics_collector.go:135: Small whitespace typo:

   // Treat as "no agencies live" for this tick; static bundles st    ay

   should be:

   // Treat as "no agencies live" for this tick; static bundles stay

5. internal/app/metrics_collector_test.go:126-127: The test comment is incomplete and still describes the old N+1 behavior. Update it to describe what the test now verifies:

   // Server mode should fetch /api/where/metrics.json once per tick.
   // The parsed response is reused by each live agency's
   // FetchObaAPIMetrics call.

6. internal/app/metrics_collector.go:270-271: This comment reads more like a commit note than a code comment:

   // nil prefetch: agency-mode has no /metrics.json probe ahead of this call,
   // so FetchObaAPIMetrics fetches the endpoint itself. Behavior unchanged.

   I would either remove it or shorten it to:

   // nil: agency-mode fetches its own response.

One optional improvement: prefetch works, but something like prefetchedMetrics or cachedMetrics would make its purpose clearer. If you decide to rename it, the relevant usages are in metrics_collector.go:133, metrics_collector.go:307, metrics_service.go:66, and oba_rest_api_metrics.go:74.

Moreover, the PR is conflicting with the main branch consider resolve this conflict.

[ahmadmuhamadi]

Okay, Tmrw i will work on them