Use K8S metric wrapper #750
Conversation
|
Thanks for your PR. The following commands are available:
These commands can only be run by members of the vmware-tanzu organization. |
|
The code doesn't meet go fmt, otherwise LGTM. I found "Fixes #xx #xx" cannot close multiple issues. Instead, this might work according to https://help.github.com/en/enterprise/2.16/user/github/managing-your-work-on-github/closing-issues-using-keywords#closing-multiple-issues: |
Kubernetes implements a wrapper for Prometheus, providing several additional features such as metric deprecation and stability information. The patch is using this wrapper to expose metrics.
|
Thanks @tnqn I've addressed the above. |
|
/test-all |
|
/test-conformance |
|
|
||
| ovsStats := newOVSStatManager(ovsBridge, ofClient) | ||
| if err := prometheus.Register(ovsStats); err != nil { | ||
| if err := legacyregistry.RawRegister(ovsStats); err != nil { |
There was a problem hiding this comment.
Curious why RawRegister is used instead of Register in couple of places?
RawRegister looks to be deprecated from v0.18 k8s release.
kubernetes/kubernetes@214228f
There was a problem hiding this comment.
@ksamoray I agree with Srikar. Looks like we should be able to use Register instead of RawRegister with some minor changes (it seems that it should already work for antrea_agent_local_pod_count but that some minor changes are required for antrea_agent_ovs_flow_table). If the changes are not minor or cannot be done this week (for 0.7 release), we can punt them to a future PR and merge this as is.
|
@antoninbas @srikartati tried to get rid of this, also getting rid of using prometheus.MustNewConstMetric() as the metrics wrapper offers its own implementation. |
There was a problem hiding this comment.
@ksamoray ok let's merge it as is. Would you mind opening a new issue with your observations regarding the use of RawRegister, so we can keep track of progress on this and potential issues related to updating the k8s libraries version we use?
Kubernetes implements a wrapper for Prometheus, providing several
additional features such as metric deprecation and stability
information.
The patch is using this wrapper to expose metrics.
This also works around breakage due to upgrade of apiserver.
Fixes #734
Fixes #662