-
Notifications
You must be signed in to change notification settings - Fork 2.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Expose resource metrics using labels in boskos #13767
Expose resource metrics using labels in boskos #13767
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: stevekuznetsov 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 |
for _, resource := range resources { | ||
resourcesByState[resource] = map[string]float64{} | ||
for _, state := range states { | ||
resourcesByState[resource][state] = 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The current behavior
of producing a "0" count for type/state combinations that do not have
any record in Boskos but are requested via flag is preserved.
I guess that this line is related to the above. But I cannot see why we need 0
here as init value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we still want to be able to query empty state, without initialize I think there will be no value, instead of 0?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we can deal with no value by the query exression OR on() vector(0)
grafana/grafana#2393 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm @hongkailiu so the case that I think @krzyzacy was worried about is -- if we have a resource myThing
that has some state cleaning
but only transitively goes into that state -- say, from inUse
--> dirty
--> cleaning
--> clean
. If we do a query like sum(boskos_resources{type="myThing"}) by (state)
we will get no metric for states like cleaning
when boskos
has no resources in that state, instead of 0
. I don't see how to use your suggestion for that case, since we aren't calling out states individually.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see now. Need to fix the test?
/lgtm
LGTM label has been added. Git tree hash: 7c7ccb0997679d47d0e929e88c319ff7a2ae7c02
|
Using one metric name for boskos resources allows for simpler queries that translate across resource types and states. For instance, instead of a query for `boskos_myType_myState` the query would be for `boskos_resources{type="myType",state="myState"}`. Furthermore, queries can now also be done across states for a type, or across types for a state. For instance, to see the states for a specific type: sum(boskos_resources{type="myThings"}) by (state) This allows for simpler dashboard creation as well. The current behavior of producing a "0" count for type/state combinations that do not have any record in Boskos but are requested via flag is preserved. Metrics serving is moved to `/metrics` on `:9090` as convention dictates. Signed-off-by: Steve Kuznetsov <skuznets@redhat.com>
72f9fcc
to
143325b
Compare
/lgtm |
LGTM label has been added. Git tree hash: 901d9794fb3a138f80ef046cd5a4f0f0b1c9c73b
|
Using one metric name for boskos resources allows for simpler queries
that translate across resource types and states. For instance, instead
of a query for
boskos_myType_myState
the query would be forboskos_resources{type="myType",state="myState"}
. Furthermore, queriescan now also be done across states for a type, or across types for a
state. For instance, to see the states for a specific type:
This allows for simpler dashboard creation as well. The current behavior
of producing a "0" count for type/state combinations that do not have
any record in Boskos but are requested via flag is preserved.
Metrics serving is moved to
/metrics
on:9090
as conventiondictates.
Signed-off-by: Steve Kuznetsov skuznets@redhat.com
/assign @hongkailiu @krzyzacy