Added prometheus metrics around bundle condition transitions #300
Conversation
| blockedUpdated := updateResourceCondition(st.bundle, res.Name, &blockedCond) || bundleUpdated | ||
| inProgressUpdated := updateResourceCondition(st.bundle, res.Name, &inProgressCond) || bundleUpdated | ||
| readyUpdated := updateResourceCondition(st.bundle, res.Name, &readyCond) || bundleUpdated | ||
| errorUpdated := updateResourceCondition(st.bundle, res.Name, &errorCond) || bundleUpdated |
There was a problem hiding this comment.
Remove || bundleUpdated here and above.
| bundleUpdated = updateBundleCondition(st.bundle, &errorCond) || bundleUpdated | ||
| inProgressUpdated := updateBundleCondition(st.bundle, &inProgressCond) || bundleUpdated | ||
| readyUpdated := updateBundleCondition(st.bundle, &readyCond) || bundleUpdated | ||
| errorUpdated := updateBundleCondition(st.bundle, &errorCond) || bundleUpdated |
There was a problem hiding this comment.
Remove || bundleUpdated here and above.
There was a problem hiding this comment.
Oops - vim replacement errors
| st.bundleTransitionCounter. | ||
| WithLabelValues(st.bundle.GetNamespace(), st.bundle.GetName(), string(errorCond.Type), errorCond.Reason). | ||
| Inc() | ||
| } |
There was a problem hiding this comment.
I think all these ifs can be moved to updateBundleCondition(), right? Same with updateResourceCondition().
| WithLabelValues(st.bundle.GetNamespace(), st.bundle.GetName(), string(readyCond.Type), readyCond.Reason). | ||
| Inc() | ||
| } | ||
| if errorUpdated && errorCond.Status == smith_v1.ConditionTrue { |
There was a problem hiding this comment.
So what is the logic here? We increment when a condition becomes true. But what does that show? Why not when it becomes false?
There was a problem hiding this comment.
I left a comment on the other PR
Forgot to answer. We need to rework how we propagate errors within Smith. We have #276 for this work, lets do it in another PR. |
83296d2 to
0b5c921
Compare
I did the review, but this thing won't let me close the "requested changes"
This adds a bunch of metrics into prometheus endpoint about bundle transitions, which would be useful for alerting purposes.
Also proposal for the same PR: How about expanding the error enums?
E.g. for resources we often just spit out the error but it would be useful to categorize if the Spec was wrong or service catalog didn't respond or if the resource couldn't be created.