Metrics (4/4) #572
Metrics (4/4) #572
Conversation
kuritka
requested review from
donovanmuller,
k0da,
somaritane and
ytsarev
as code owners
kuritka
force-pushed
the
prometheus-metrics-add
branch
from
c96953b to
b0c81e6
Compare
kuritka
force-pushed
the
prometheus-metrics-add
branch
4 times, most recently
from
41468b4 to
5fb1cdb
Compare
|
After today talk with @somaritane , @k0da I extended this PR with new infoblox metrics + tests:
@somaritane , naming changed by your recommandations |
controllers/gslb_controller.go
Outdated
| return result.RequeueError(fmt.Errorf("error reading the object (%s)", err)) | ||
| } | ||
|
|
||
| err = r.DepResolver.ResolveGslbSpec(ctx, gslb, r.Client) | ||
| if err != nil { | ||
| m.ErrorIncrement(gslb) |
There was a problem hiding this comment.
Consider using VerbNoun naming for the function in order to retain the consistency with other functions in the code (e.g. RequeError()):
| m.ErrorIncrement(gslb) | |
| m.IncrementError(gslb) |
There was a problem hiding this comment.
@somaritane , than would be nice to follow RequeueError patter, will rename all of them?
ErrorIncrement -> IncrementError
ReconciliationIncrement -> IncrementReconciliation
InfobloxZoneUpdateIncrement -> InfobloxIncrementZoneUpdate
InfobloxZoneUpdateErrorIncrement -> InfobloxIncrementZoneUpdateError
InfobloxHeartbeatIncrement -> InfobloxIncrementHeartbeat
InfobloxHeartbeatErrorIncrement -> InfobloxIncrementHeartbeatError
There was a problem hiding this comment.
@somaritane , amended for all increment funcs
kuritka
force-pushed
the
prometheus-metrics-add
branch
from
5fb1cdb to
dd119bc
Compare
There was a problem hiding this comment.
@kuritka looks good, just a question about strategy related status metrics
| m.metrics.K8gbGslbFailoverStatus = prometheus.NewGaugeVec( | ||
| prometheus.GaugeOpts{ | ||
| Name: K8gbGslbFailoverStatus, | ||
| Help: "K8GB Failover status.", |
There was a problem hiding this comment.
Perhaps we can find a better description and maybe a name as well for this metric.
Because "K8GB Failover status." sounds a bit misleading to me, we're not talking about k8gb operator status here, also this is GaugeVec metric, not Scalar.
Maybe it should be "Gslb status count for Failover strategy"?
With "K8gbGslbStatusCountForFailover" and "k8gb_gslb_status_count_for_failover" respectively?
Same goes for Roundrobin and GeoIP metrics below
There was a problem hiding this comment.
@somaritane , amended, thx.
related to #124 This PR contains several specific metrics ```gaolang Health records *prometheus.GaugeVec StatusPerIngressHosts *prometheus.GaugeVec StatusFailover *prometheus.GaugeVec StatusRoundRobin *prometheus.GaugeVec StatusGeoIP *prometheus.GaugeVec ZoneUpdateTotal *prometheus.CounterVec ErrorTotal *prometheus.CounterVec ReconciliationTotal prometheus.Counter ``` ```shell make destroy-full-local-setup deploy-full-local-setup upgrade-candidate deploy-prometheus make stop-test-app make start-test-app ```         Infoblox metrics are: - k8gb_infoblox_heartbeats_total - k8gb_infoblox_heartbeat_errors_total - k8gb_infoblox_zone_updates_total - k8gb_infoblox_zone_update_errors_total Signed-off-by: kuritka <[email protected]>
kuritka
force-pushed
the
prometheus-metrics-add
branch
from
dd119bc to
2fd01ee
Compare
related to #124
This PR contains several specific metrics
Demo
k8gb_gslb_error_total
k8gb_gslb_healthy_records
k8gb_gslb_reconciliation_total
k8gb_gslb_status_failover
k8gb_gslb_status_roundrobin
k8gb_gslb_status_per_ingress_hosts
Infoblox metrics:
Signed-off-by: kuritka [email protected]