balancer/rls: Add cache metrics

[zasweq]

After some discussion, decided to just emit gauge metrics inline using synchronous gauges with cache mutex held. OpenTelemetry just copies this data over to another store for exporters to use, so this shouldn't introduce a performance hit/lock contention, especially since the accesses cache creates lock contention on the operations even before metrics.

As suggested in #7484 (comment).

RELEASE NOTES:

• balancer/rls: Add cache metrics

[codecov]

Codecov Report

Attention: Patch coverage is 87.09677% with 4 lines in your changes missing coverage. Please review.

Project coverage is 81.68%. Comparing base (7b9e012) to head (7b17b83).
Report is 9 commits behind head on master.

Files	Patch %	Lines
internal/testutils/stats/test_metrics_recorder.go	55.55%	2 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #7495      +/-   ##
==========================================
+ Coverage   81.60%   81.68%   +0.07%     
==========================================
  Files         357      359       +2     
  Lines       27294    27532     +238     
==========================================
+ Hits        22274    22490     +216     
- Misses       3816     3822       +6     
- Partials     1204     1220      +16     

Files	Coverage Δ
balancer/rls/balancer.go	85.81% <100.00%> (+0.04%)	⬆️
balancer/rls/cache.go	89.07% <100.00%> (+1.11%)	⬆️
internal/testutils/stats/test_metrics_recorder.go	72.35% <55.55%> (-0.27%)	⬇️

... and 43 files with indirect coverage changes

[easwars]

LGTM, modulo minor comments.

[easwars]

Is this really required? The grpc.lb.rls.server_target is empty here. So, will this measurement even be useful?

[zasweq]

I think so. This gives a measurement at the beginning that the cache is current of 0 (vs unset and not showing up). This is a gauge, so just the most recent state of the system, but I think it makes sense to at construction time give the state that it's empty).

[zasweq]

Discussed this offline with Doug and Yash, at first we thought leave it but then they mused about the same point you brought up, which is this gauge as written will live around the lifetime of the binary with an empty target. So this is actually a valid correctness issue. Thanks for catching this.

[zasweq]

I brought this up in the Observability thread alongside another one of my concerns, but this case is the main one that is seen as a correctness issue. If the rls server target changes over the lifetime of the balancer, it's up to exporter to have logic for the same uuid gauge with a different rls server target. Yash mentioned dashboards can group on uuid, and then see the rls server target change over time, so WAI.

[easwars]

gofmt is crazy that it allows { and } to be on the same line for some of the methods and not for other methods. I've noticed this when making changes too.

You could get rid of the _ parameter names for all the methods though, since none of the parameters are used.

[zasweq]

Ah I see. Will switch to one line, and remove _ parameter name.