shotover/shotover-proxy

RedisSinkCluster - Avoid recreating `Counter`

Closed this issue · 0 comments

rukai commented

The counter!() macro is used to create a Counter instance which we can then call increment(1) on to increment a metric reported in shotover's prometheus metrics.

Counter instances can be either:

  • stored to be reused many times
  • created, used, discarded on each use.

The end result of both approaches is identical.

However, the advantages of reusing Counters is:

  • Better performance - creating a Counter can be expensive
  • Easier maintenance - the metrics labels are defined in a single place

RedisSinkCluster contains a metric named shotover_failed_requests_count, we should create that metric in RedisSinkCluster::new().
We already register the shotover_failed_requests_count counter metric in RedisSinkCluster::new, we just dont hold onto the Counter for later use.
So we should take the registration, and store the returned value in RedisSinkCluster.
To do this we will need to replace the call to sink_cluster.get_name() with just the NAME constant, which enables further cleanup in removing the get_name method. (just follow what the warnings tell you)

When implementing this PR, you can refer to how other transforms store and reuse Counter's:

out_of_rack_requests: counter!("shotover_out_of_rack_requests_count", "chain" => chain_name, "transform" => NAME),