search

samstarling / finagle-prometheus

Provides a bridge between Finagle and Prometheus metrics
https://samstarling.co.uk/projects/finagle-prometheus/
MIT License
30 stars 18 forks source link

Cache telemetry collectors per registry #20

Open turbolent opened 6 years ago

turbolent commented 6 years ago

Currently theTelemetry caches collectors (counters, histograms, and gauges):

https://github.com/samstarling/finagle-prometheus/blob/master/src/main/scala/com/samstarling/prometheusfinagle/metrics/Telemetry.scala#L10

However, that doesn't prevent a user from creating multiple instances of Telemetry for the same registry and attempting to register an object which is already registered:

Caused by: java.lang.IllegalArgumentException: Collector already registered that provides name: test
    at io.prometheus.client.CollectorRegistry.register(CollectorRegistry.java:54)
    at io.prometheus.client.SimpleCollector$Builder.register(SimpleCollector.java:245)
    at com.samstarling.prometheusfinagle.metrics.Telemetry.$anonfun$counter$1(Telemetry.scala:28)
    at scala.collection.concurrent.TrieMap.getOrElseUpdate(TrieMap.scala:903)
    at com.samstarling.prometheusfinagle.metrics.Telemetry.counter(Telemetry.scala:28)

The caching should take the registry into account, not only the object name and namespace. It seems currently not possible to get a previously registered collection from CollectorRegistry (no method available and namesToCollectors is unfortunately not exposed)