Skip to content

Conversation

c-julin
Copy link
Contributor

@c-julin c-julin commented Aug 29, 2025

Summary

  • Enables injection of custom Prometheus registry for Kafka client metrics
  • Replaces global registry usage with per-registry metric management
  • Maintains all existing metrics functionality including active client tracking

Changes

  • Modified client_hooks.go to support per-registry metrics instead of global promauto
  • Updated CachedClientProvider to accept and use injected Prometheus registry
  • Fixed /admin/metrics endpoint to use injected registry
  • Updated test files to pass registry parameter

This allows enterprise repos to inject their own Prometheus registry while preserving all Kafka metrics collection.

@c-julin c-julin requested review from weeco and sago2k8 September 2, 2025 07:04
Remove MaxStaleAge and AutoCleanInterval for simpler cache behavior.
@c-julin c-julin force-pushed the jc/metrics-hook-used branch from 4b3bf54 to 8d1a1ba Compare September 2, 2025 07:06
@c-julin c-julin merged commit d7ec450 into master Sep 4, 2025
5 checks passed
@c-julin c-julin deleted the jc/metrics-hook-used branch September 4, 2025 13:34
@@ -546,7 +547,7 @@ func (api *API) routes() *chi.Mux {
// Private routes - these should only be accessible from within Kubernetes or a protected ingress
router.Group(func(r chi.Router) {
api.Hooks.Route.ConfigInternalRouter(r)
r.Handle("/admin/metrics", promhttp.Handler())
r.Handle("/admin/metrics", promhttp.HandlerFor(api.PrometheusRegistry.(*prometheus.Registry), promhttp.HandlerOpts{}))
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't have a compiler at hand with this branch at the moment, but is the type assertion necessary here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we use the interface in the constructor which probably isn't necessary would be simpler to use the type. can update this

Copy link
Contributor

Choose a reason for hiding this comment

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

https://pkg.go.dev/github.com/prometheus/client_golang/prometheus/promhttp#HandlerFor

This suggests that it also accepts the interface though :D

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Different interfaces Registerer and Gatherer i think its better to just use the type which implements both interfaces. instead we store it as a registerer and then use it as both.

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