fix: Custom metrics are not recorded for DAG tasks Fixes #3872 #3886
Conversation
workflow/controller/dag.go
Outdated
| // Collect the completed task metrics | ||
| tmpl := woc.wf.GetTemplateByName(task.Template) | ||
| if tmpl != nil && tmpl.Metrics != nil { | ||
| if prevNodeStatus, ok := woc.preExecutionNodePhases[node.ID]; ok && !prevNodeStatus.Fulfilled() { | ||
| localScope, realTimeScope := woc.prepareMetricScope(node) | ||
| woc.computeMetrics(tmpl.Metrics.Prometheus, localScope, realTimeScope, false) | ||
| } | ||
| } |
There was a problem hiding this comment.
Not too sure this is the correct approach here. All metrics should be emitted within executeTemplate. If you notice for DAG or Steps, metrics are never emitted outside of it. Can you investigate why the current metric emission code in executeTemplate is not getting called in this case?
There was a problem hiding this comment.
In DAG flow, the Code is checking the node.Fulfilled() in early-stage (executeDAGTask) which prevents to execute the executeTemplate for completed the tasks.
https://github.com/argoproj/argo/blob/5b5d2359ef9f573121fe6429e386f03dd8652ece/workflow/controller/dag.go#L316-L326
Options:
- We can remove the
returnstatement on the above code. So, Completed node will travel toexecutetemplateBut I am not sure whether it will break any DAG flow - Above fix, Add metrics emission code in
node.Fulfilled()check
There was a problem hiding this comment.
On second thought, I think we can keep this current fix for this issue.
workflow/controller/dag.go
Outdated
| // Collect the completed task metrics | ||
| tmpl := woc.wf.GetTemplateByName(task.Template) | ||
| if tmpl != nil && tmpl.Metrics != nil { | ||
| if prevNodeStatus, ok := woc.preExecutionNodePhases[node.ID]; ok && !prevNodeStatus.Fulfilled() { | ||
| localScope, realTimeScope := woc.prepareMetricScope(node) | ||
| woc.computeMetrics(tmpl.Metrics.Prometheus, localScope, realTimeScope, false) | ||
| } | ||
| } |
There was a problem hiding this comment.
On second thought, I think we can keep this current fix for this issue.
|
Thank you for taking care of the issue that fast! Highly appreciated |
|
Backported to 2.10 and 2.11 |
Checklist:
"fix(controller): Updates such and such. Fixes #1234". Fixes Custom metrics are not recorded for DAG tasks. #3872