fix: cache PrometheusMetrics to prevent duplicate registration panic

[ankurs]

Summary

• NewPrometheusMetrics(namespace) now caches instances per namespace
• Uses sync.RWMutex with double-checked locking — reads (cache hits) use read lock only
• Calling multiple times with the same namespace returns the cached instance instead of panicking on duplicate Prometheus registration

Test plan

• go test -race ./... passes
• make lint — 0 issues

Summary by CodeRabbit

• Documentation

  • Clarified metrics initialization docs to state caching behavior and guidance on using a small set of static namespaces.

• Bug Fixes

  • Metrics initialization now caches instances per namespace and is concurrency-safe, preventing duplicate metric registrations.

• Tests

  • Added tests verifying cached instances by namespace and safe concurrent initialization.

[coderabbitai]

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

Adds a process-global cache for NewPrometheusMetrics(namespace) so repeated calls with the same namespace return the same *prometheusMetrics instance. Uses a package-level map with a read-write mutex and double-checked locking; README docs and tests updated to assert caching and concurrency safety.

Changes

Cohort / File(s)	Summary
Prometheus Metrics Implementation metrics.go	Adds a package-level promMetricsCache map[string]*prometheusMetrics with sync.RWMutex. Implements double-checked locking: read-lock lookup, write-lock construction on miss, stores cached instance to avoid duplicate creation/registration per namespace.
Prometheus Metrics Tests metrics_test.go	Adds tests asserting same-instance returns for identical namespaces, different instances for different namespaces, and concurrent-safe initialization via 100 goroutines.
Documentation Update README.md	Clarifies NewPrometheusMetrics(namespace string) Metrics is safe to call multiple times with the same namespace, returns a process-global cached instance, and recommends using a small set of static namespaces rather than per-request/tenant dynamic values.

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant NewPrometheusMetrics
  participant Cache
  participant PromRegistry as PrometheusRegistry

  Caller->>NewPrometheusMetrics: request(namespace)
  NewPrometheusMetrics->>Cache: RLock & lookup(namespace)
  Cache-->>NewPrometheusMetrics: hit? (instance or nil)
  alt cache hit
    NewPrometheusMetrics-->>Caller: return cached instance
  else cache miss
    NewPrometheusMetrics->>Cache: RUnlock, Lock (write)
    NewPrometheusMetrics->>Cache: re-check(namespace)
    alt still missing
      NewPrometheusMetrics->>PromRegistry: register vectors/gauges
      NewPrometheusMetrics->>Cache: store new instance
    end
    NewPrometheusMetrics->>Cache: Unlock
    NewPrometheusMetrics-->>Caller: return new instance
  end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I hopped through code with mutex might,

One cache per name keeps metrics tight,
No racing threads, no doubles seen,
A tidy map where values convene.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: implementing caching for PrometheusMetrics to prevent duplicate registration panics.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/prometheus-metrics-cache

---

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 and usage tips.}

[Copilot]

Pull request overview

This PR prevents panics from duplicate Prometheus collector registration by making NewPrometheusMetrics(namespace) return a cached, per-namespace singleton instance, and updates the public docs to reflect the new behavior.

Changes:

• Added a package-level cache (guarded by sync.RWMutex with double-checked locking) to reuse *prometheusMetrics instances per namespace.
• Updated NewPrometheusMetrics documentation to state it’s safe to call multiple times with the same namespace.
• Updated README API docs accordingly.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
README.md	Documents the new “safe to call multiple times” behavior for NewPrometheusMetrics.
metrics.go	Introduces a per-namespace cache + locking to avoid duplicate Prometheus registrations.

---

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