Refactor metrics doc generator #11467
Conversation
Signed-off-by: avlitman <alitman@redhat.com>
kubevirt-bot
added
release-note-none
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@machadovilaca you think we can avoid querying the endpoints once all refactor work is done? |
|
/retest |
1 similar comment
|
/retest |
| @@ -140,8 +140,7 @@ ${KUBEVIRT_DIR}/tools/openapispec/openapispec --dump-api-spec-path ${KUBEVIRT_DI | |||
| (cd ${KUBEVIRT_DIR}/tools/doc-generator/ && go_build) | |||
| ( | |||
| cd ${KUBEVIRT_DIR}/docs | |||
| ${KUBEVIRT_DIR}/tools/doc-generator/doc-generator | |||
| mv newmetrics.md metrics.md | |||
| ${KUBEVIRT_DIR}/tools/doc-generator/doc-generator >metrics.md | |||
There was a problem hiding this comment.
| ${KUBEVIRT_DIR}/tools/doc-generator/doc-generator >metrics.md | |
| ${KUBEVIRT_DIR}/tools/doc-generator/doc-generator > metrics.md |
There was a problem hiding this comment.
Actually this one was fixed by make generate I did use a space in the beginning
There was a problem hiding this comment.
tools/doc-generator/metrics_collector.go
There was a problem hiding this comment.
why do wee need to use a metrics collector? can you use the list methods from each component monitoring package?
There was a problem hiding this comment.
Since we query the end point and add metrics manually that's why I asked if we can avoid that.
| } | ||
| } | ||
| return "" | ||
| return virt_controller.ListMetrics() |
There was a problem hiding this comment.
since it's metrics from multiple components, you should maybe use operatormetrics.ListMetrics() directly
| All metrics documented here are auto-generated and reflect exactly what is being | ||
| exposed. After developing new metrics or changing old ones please regenerate | ||
| this document. | ||
| ` | ||
|
|
||
| func main() { | ||
| handler := domainstats.Handler(1) |
There was a problem hiding this comment.
this is only used for domainstats metrics, but this will be part of the virt-handler metric listing after the refactor, this code should also be removed IMO, and this PR only merged after that refactor
There was a problem hiding this comment.
Make sense to me also to merge only after refactor is done.
There was a problem hiding this comment.
@machadovilaca The refactor pr's that need to be merged before this one are:
#11184
#11601
#11538
Is there more?
|
/hold |
kubevirt-bot
added
the
do-not-merge/hold
kubevirt-bot
added
the
needs-rebase
|
PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
closed since handle in #11601 |
What this PR does
This pr refactor the metric doc generator to use a new common lib.
This pr is a follow-up for the monitoring code refactoring work which documented in this proposal: kubevirt/community#219
Before this PR:
The metric doc generator didn't use the operator-observability common library.
After this PR:
The metric doc generator didn't use the common library.
Jira-ticket: https://issues.redhat.com/browse/CNV-30920
Release note