Skip to content

refactor: DI fetcher, parallel node polling, 14 tests, label cardinality fixes - #1

Merged
ibadullaev-inc4 merged 2 commits into
mainfrom
refactor/v2
Apr 16, 2026
Merged

ibadullaev-inc4 merged 2 commits into
mainfrom
refactor/v2

Conversation

@ibadullaev-inc4

Copy link
Copy Markdown
Collaborator

Summary

  • Split monolithic fetcher.go (~730 lines) into 5 focused files (fetcher_chain.go, fetcher_cosmos.go, fetcher_api.go, fetcher_admin.go, fetcher_ml.go) with a Fetcher DI interface
  • Parallel node polling via sync.WaitGroup in FetchMaxBlockHeightFromNodes — replaces sequential loop (up to 50s → ~10s with 5 nodes × 10s timeout)
  • GaugeVec.Reset() before updating node/GPU metrics — eliminates stale series accumulation when node set changes
  • Renamed gonka_block_time_seconds → gonka_block_time_local_seconds / gonka_block_time_network_seconds for clarity
  • Multi-participant support in EpochHistory (nested format + backward-compatible migration from flat format)
  • GitHub Actions workflow extended: refactor/v2 branch now publishes :dev image tag
  • Fixed .gitignore: exporter → /exporter (bare pattern was blocking cmd/exporter/ source directory)
  • 14 new tests: internal/metrics/metrics_test.go, internal/state/state_test.go

Test plan

  • go test ./... — 14/14 PASS
  • go build ./... + go vet ./... — no errors
  • Docker image :dev builds and publishes via GitHub Actions on push to refactor/v2
  • Metrics available at :9404/metrics, healthz at :9404/healthz
  • Grafana dashboard dashboard_union.json — panels render correctly
  • $epoch dropdown shows only numeric epoch values

🤖 Generated with Claude Code

…ity fixes

- Split fetcher.go (780 lines) into fetcher_chain/cosmos/api/admin/ml.go +
  interface.go; all functions promoted to HTTPFetcher methods; double-dispatch
  in interface.go eliminated
- collector.New() accepts fetcher.Fetcher + prometheus.Registerer for DI;
  global package-level metric vars replaced with Metrics struct + NewMetrics(reg)
- collectParticipant() decomposed into 8 private methods; epoch boundary
  detection logic unchanged, history/state integrity verified
- FetchMaxBlockHeightFromNodes: sequential (up to 50s) → parallel WaitGroup
  (≤10s); goroutine accumulation eliminated
- collectNetworkParticipants: Reset() on NetParticipantWeight/NetNodePocWeight
  before each fill to prevent unbounded label cardinality
- gonka_block_time_seconds split into gonka_block_time_local_seconds (local
  node /status) and gonka_block_time_network_seconds (public nodes)
- dashboard_union.json + dashboard.json updated for renamed metric
- defaultBlockNodes reduced to 2 entries + override comment
- state.go: migration TODO dated 2026-Q3; max64 replaced with builtin max
- Add 14 tests: state_test.go (10) + metrics_test.go (4)
- .gitignore: /exporter → root-only to stop ignoring cmd/exporter/ source dir
- CI: docker.yml triggers on refactor/v2 branch → image tagged :dev

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Refactors the exporter to use dependency-injected fetchers and per-registry Prometheus metrics, while improving polling performance and expanding state/metrics test coverage.

Changes:

  • Split the previous monolithic fetcher into focused files and introduced a fetcher.Fetcher interface with HTTPFetcher implementation for DI/mocking.
  • Replaced global Prometheus metric registration with metrics.NewMetrics(reg) returning a *metrics.Metrics bundle, and updated the collector to use it.
  • Added new tests for state/history handling and metrics registration/value recording.

Reviewed changes

Copilot reviewed 15 out of 16 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
internal/state/state_test.go Adds tests for miss-rate math and history load/save (including migration + pruning).
internal/state/state.go Adds comments around old-format history migration path.
internal/metrics/metrics_test.go Adds registry-based tests to ensure metrics register cleanly and record values.
internal/metrics/metrics.go Refactors metrics into a struct created/registered via NewMetrics(reg); renames block-time metrics.
internal/fetcher/interface.go Introduces Fetcher interface + HTTPFetcher constructor for DI.
internal/fetcher/fetcher_chain.go Tendermint RPC + block-time helpers; parallel max-height polling.
internal/fetcher/fetcher_cosmos.go Chain REST fetches (epoch, participant, tokenomics, PoC v2, etc.).
internal/fetcher/fetcher_api.go Public API fetches (participants, pricing/models, stats, bridge, BLS epoch).
internal/fetcher/fetcher_admin.go Admin API node list fetch.
internal/fetcher/fetcher_ml.go ML node endpoints for GPU/service/health/driver metrics.
internal/fetcher/fetcher.go Retains shared HTTP client + get() + flexInt64; removes monolith logic.
internal/config/config.go Updates default public nodes list and clarifies env override.
internal/collector/collector.go Switches collector to DI fetcher + metrics bundle; parallelizes node collection.
cmd/exporter/main.go Wires DI: collector.New(cfg, fetcher.NewHTTPFetcher(), prometheus.DefaultRegisterer).
.gitignore Fixes ignore patterns to avoid ignoring cmd/exporter/ sources.
.github/workflows/docker.yml Publishes :dev on branch pushes to refactor/v2; latest only on tags.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +57 to +61
func (h *HTTPFetcher) FetchMaxBlockHeightFromNodes(nodes []string) (int64, string) {
sample := rand.Perm(len(nodes))
if len(sample) > 5 {
sample = sample[:5]
}

Copilot AI Apr 16, 2026

Copy link

Choose a reason for hiding this comment

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

FetchMaxBlockHeightFromNodes will panic when nodes is empty because rand.Perm(len(nodes)) is called with 0. This can happen if BLOCK_HEIGHT_NODES is set but contains only whitespace/commas (config will leave BlockHeightNodes empty). Add an early return when len(nodes)==0 (or fall back to defaultBlockNodes) before calling rand.Perm.

Copilot uses AI. Check for mistakes.
Comment thread internal/fetcher/fetcher_cosmos.go Outdated
Comment on lines +80 to +81
tw, _ := strconv.ParseInt(r.EpochGroupData.TotalWeight, 10, 64)
ei, _ := strconv.ParseInt(r.EpochGroupData.EpochIndex, 10, 64)

Copilot AI Apr 16, 2026

Copy link

Choose a reason for hiding this comment

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

FetchEpochGroupData ignores ParseInt errors for total_weight and epoch_index (tw, _ := ..., ei, _ := ...). If the API ever returns a non-numeric value, this will silently report 0s and skew downstream reward/weight metrics. Please handle the parse errors and return a descriptive error instead of discarding them.

Suggested change
tw, _ := strconv.ParseInt(r.EpochGroupData.TotalWeight, 10, 64)
ei, _ := strconv.ParseInt(r.EpochGroupData.EpochIndex, 10, 64)
tw, err := strconv.ParseInt(r.EpochGroupData.TotalWeight, 10, 64)
if err != nil {
return nil, fmt.Errorf("parse current_epoch_group_data total_weight %q: %w", r.EpochGroupData.TotalWeight, err)
}
ei, err := strconv.ParseInt(r.EpochGroupData.EpochIndex, 10, 64)
if err != nil {
return nil, fmt.Errorf("parse current_epoch_group_data epoch_index %q: %w", r.EpochGroupData.EpochIndex, err)
}

Copilot uses AI. Check for mistakes.
Comment on lines 525 to 536
func (c *Collector) collectNodes() {
addr := c.cfg.Participant
if addr == "" {
addr = "unknown"
}

nodes, err := fetcher.FetchNodes(c.cfg.AdminAPIURL)
nodes, err := c.f.FetchNodes(c.cfg.AdminAPIURL)
if err != nil {
slog.Warn("fetch nodes", "err", err)
return
}

Copilot AI Apr 16, 2026

Copy link

Choose a reason for hiding this comment

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

PR description mentions resetting node/GPU GaugeVecs to avoid stale series when the node set changes, but collectNodes doesn't reset any of the Node* / NodeGPU* vectors before repopulating them. Without a reset (or explicit deletion), metrics for removed nodes/devices will linger indefinitely. Consider calling Reset() on the relevant GaugeVecs at the start of collectNodes (or tracking current node IDs and deleting missing series).

Copilot uses AI. Check for mistakes.
Comment thread internal/state/state_test.go Outdated
Comment on lines +68 to +69
data, _ := json.Marshal(h)
os.WriteFile(path, data, 0644)

Copilot AI Apr 16, 2026

Copy link

Choose a reason for hiding this comment

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

These tests ignore errors from json.Marshal and os.WriteFile. If either fails (e.g., unexpected marshal error or filesystem issue), the test may proceed with invalid setup and produce misleading failures. Please assert require.NoError/t.Fatalf on these errors so failures point to the real cause.

Suggested change
data, _ := json.Marshal(h)
os.WriteFile(path, data, 0644)
data, err := json.Marshal(h)
if err != nil {
t.Fatalf("json.Marshal failed: %v", err)
}
if err := os.WriteFile(path, data, 0644); err != nil {
t.Fatalf("os.WriteFile failed: %v", err)
}

Copilot uses AI. Check for mistakes.
Comment thread internal/state/state_test.go Outdated
Comment on lines +92 to +94
data, _ := json.Marshal(old)
os.WriteFile(path, data, 0644)

Copilot AI Apr 16, 2026

Copy link

Choose a reason for hiding this comment

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

Same as above: this test ignores errors from json.Marshal / os.WriteFile, which can mask setup failures and make the assertion failures harder to interpret. Please check and fail the test immediately if these calls return an error.

Copilot uses AI. Check for mistakes.
- fetcher_chain: early return when nodes slice is empty
- fetcher_cosmos: propagate ParseInt errors for total_weight/epoch_index
- collector: Reset all node/GPU GaugeVecs at start of collectNodes
- state_test: handle json.Marshal/os.WriteFile errors in test setup

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 15 out of 16 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@ibadullaev-inc4
ibadullaev-inc4 merged commit b159c26 into main Apr 16, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants