Wrapper that adds the cluster name as a label. #8961
Conversation
| append(labels, Label{"cluster", m.ClusterName})) | ||
| } | ||
|
|
||
| var globalClusterMetrics *ClusterMetricSink |
There was a problem hiding this comment.
Every time I introduce a global I always wind up regretting it eventually (and I wish go-metrics did not rely on one). Are you sure this can't just be a field in Core instead?
There was a problem hiding this comment.
It seems to me like we will want to introduce some metrics in places that are not in Core, such as individual secret engines. The new counts could probably all be handled by adding a member to Core, but then we would end up also passing the wrapper down to TokenStore and ExpirationManager and PassthroughBackend.
On the plus side, having it in core would give a more sensible way of initializing the cluster name. But I don't understand right now how it would get to all the other places.
One of the longer-term goals here is to be able to convert existing go-metrics calls (if we can find a sensible way to do so) so that they can start including the cluster ID too, which pushed me in the direction of more closely matching how go-metrics does things.
I'll give this some more thought.
There was a problem hiding this comment.
TokenStore and ExpirationManager have their own Core objects already, so I'm not sure why they need to be passed the wrapper.
What about adding something to the system view for use by secrets engines?
There was a problem hiding this comment.
Good point, they can access it through Core if it's exported.
It seems like the parallel for backends would be to provide it in BackendConfig like the Logger.
There was a problem hiding this comment.
I would really not like to introduce a metrics sink here and then come back and do it all over again with the event system broker,, but I guess that's probably the best option.
There was a problem hiding this comment.
Removed global variable; I'll do something with SystemView or BackendConfig when I am adding a real use case.
The new telemetry metrics all use cluster as a label; this helper adds it automatically rather than specifying it in each location.