Add rule evaluation time #3491
Add rule evaluation time #3491
Conversation
rules/recording.go
Outdated
| } | ||
|
|
||
| // EvalTimeSeconds returns the time in seconds it took to evaluate the recording rule. | ||
| func (rule *RecordingRule) EvalTimeSeconds() float64 { |
There was a problem hiding this comment.
Is there a race here that warrants a lock?
There was a problem hiding this comment.
Thanks for spotting! Added locks for reading and updating both the recording and alert rule's evaluation times.
There was a problem hiding this comment.
@gouthamve Is there any further changes to be made here?
web/ui/templates/rules.html
Outdated
| <tr> | ||
| <td class="rule_cell">{{.HTMLSnippet pathPrefix}}</td> | ||
| <td>{{humanizeDuration .EvalTimeSeconds}}</td> | ||
| {{end}} |
There was a problem hiding this comment.
{{end}} should come after </tr>
rules/manager.go
Outdated
| @@ -176,6 +180,7 @@ func (g *Group) run() { | |||
| g.Eval(start) | |||
|
|
|||
| iterationDuration.Observe(time.Since(start).Seconds()) | |||
| g.EvaluationTimeSeconds = time.Since(start).Seconds() | |||
There was a problem hiding this comment.
This is another race here. When templating we are reading it without a lock.
Add a setter and accessor for it?
rules/manager.go
Outdated
| @@ -115,19 +115,24 @@ type Rule interface { | |||
| Eval(context.Context, time.Time, *promql.Engine, *url.URL) (promql.Vector, error) | |||
| // String returns a human-readable string representation of the rule. | |||
| String() string | |||
|
|
|||
| UpdateEvalTimeSeconds(float64) | |||
There was a problem hiding this comment.
Below we have GetEvaluationTimeSeconds and setEvaluationTimeSeconds while here the names are different for same functionality. Can we sync those?
There was a problem hiding this comment.
Got it. Thanks for the review! 👍
Added evaluation time to both rule groups and rules on /rules.
This makes for easier debugging when trying to identify an expensive rule or rule group.
See screenshot for example:
Any thoughts?