Metrics: Add MetricBuilder to standardize metrics #3743
Conversation
rna-afk
changed the title
openshift-ci-robot
added
the
do-not-merge/work-in-progress
rna-afk
force-pushed
the
installer_telemetry_metric_builder
branch
from
15d1fac
to
1ac4cbc
Compare
rna-afk
force-pushed
the
installer_telemetry_metric_builder
branch
2 times, most recently
from
7ea4e15
to
8e36a95
Compare
There was a problem hiding this comment.
Regarding the initializer, perhaps we should consider something similar to this pattern: https://play.golang.org/p/VGl9olSFnNm
This would require setters for label key values and value. Curious if @abhinavdahiya has thoughts.
I think the semantics of this pattern has some nice benefits.
|
Did see it thanks. Every time I try to solve it, it causes more problems. I will manually fix the entries now. |
rna-afk
force-pushed
the
installer_telemetry_metric_builder
branch
5 times, most recently
from
210452b
to
0bb52ad
Compare
openshift-ci-robot
added
needs-rebase
rna-afk
force-pushed
the
installer_telemetry_metric_builder
branch
from
3353975
to
2aa53f6
Compare
|
/retest |
1 similar comment
|
/retest |
rna-afk
changed the title
openshift-ci-robot
removed
the
do-not-merge/work-in-progress
| }) | ||
| } | ||
| } | ||
| func TestNewMetricBuilder(t *testing.T) { |
There was a problem hiding this comment.
Can we add a comment:
TestNewMetricBuilder tests the Metric Builder initializer.
Although I see now that this tests more than the iniitializer. I'm not sure if these are the most useful test cases. They are fine, but they don't test realistic errors/failure modes so I'm not sure if these add much value.
There was a problem hiding this comment.
Hmm.. do you have any kind of tests in mind? I thought that since this is just a struct with the metric type, there can only be some kind of validation tests and the significant tests can be done only when the API system is in place.
rna-afk
force-pushed
the
installer_telemetry_metric_builder
branch
from
2aa53f6
to
033e2f1
Compare
|
/test e2e-aws-upgrade |
2 similar comments
|
/test e2e-aws-upgrade |
|
/test e2e-aws-upgrade |
In order to make all the metrics uniform, the MetricBuilder struct type is created with the option for storage, value setting, and a type field. A build function is added which will create the respective Prometheus Collector object based on the type of the MetricBuilder and will automatically set the values and labels to the collector object which can be used for pushing. It also has a field for the description and the full name of the metric for providing information about the metric generated. The field bucket is used only for histogram.
Added the dependencies for prometheus in the vendor folder along with some indirect dependencies.
rna-afk
force-pushed
the
installer_telemetry_metric_builder
branch
from
c2fc9c5
to
7ccc7cb
Compare
|
@rna-afk: The following tests failed, say
Full PR test history. Your PR dashboard. 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. I understand the commands that are listed here. |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: abhinavdahiya The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci-robot
added
the
approved
|
/lgtm |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
In order to make all the metrics uniform, the MetricBuilder
struct type is created with the option for storage, value setting,
and a type field. A build function is added which will create the
respective Prometheus Collector object based on the type of the
MetricBuilder and will automatically set the values and labels to
the collector object which can be used for pushing.
It also has a field for the description and the full name of the
metric for providing information about the metric generated. The field
bucket is used only for histogram.