From ed8a2c2c02fd62e197405548990387d523e3d2d2 Mon Sep 17 00:00:00 2001 From: Xinru Zhang Date: Thu, 8 Dec 2022 12:25:43 -0500 Subject: [PATCH] TEP-0121: Move metrics recorder out of reconcile() to ReconcileKind() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Prior to this commit, metrics recording durationAndCountMetrics() is in reconcile() (instead of ReconcileKind()), which only counts the taskrun numbers where the taskruns that have no preparation error / not cancelled right after scheduled / not timed out before calling reconcile() (for example, be pending for a long time.). This commit moves durationAndCountMetrics() into ReconcileKind() to address the issue above. It's worth mentioning that the recording metrics logic was originally in the ReconcileKind() function, but the commit https://github.com/tektoncd/pipeline/commit/137521906ce3e1df9440a3248f932ddb5a653e35, aming at addressing the recount issue, **moves that logic to `reconcile()`** [1] (probably for better code style? 🤔). [1] https://github.com/tektoncd/pipeline/commit/137521906ce3e1df9440a3248f932ddb5a653e35#diff-6e67e9c647bbe2a08807ff5ccbdd7dc9036df373e56b9774d3996f92ab7ceabaL138-L145 --- pkg/reconciler/taskrun/taskrun.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/reconciler/taskrun/taskrun.go b/pkg/reconciler/taskrun/taskrun.go index 8f4940adea3..b632e1b11b4 100644 --- a/pkg/reconciler/taskrun/taskrun.go +++ b/pkg/reconciler/taskrun/taskrun.go @@ -97,6 +97,7 @@ var ( // converge the two. It then updates the Status block of the Task Run // resource with the current status of the resource. func (c *Reconciler) ReconcileKind(ctx context.Context, tr *v1beta1.TaskRun) pkgreconciler.Event { + defer c.durationAndCountMetrics(ctx, tr) logger := logging.FromContext(ctx) ctx = cloudevent.ToContext(ctx, c.cloudEventClient) // By this time, params and workspaces should not be propagated for embedded tasks so we cannot @@ -452,7 +453,6 @@ func (c *Reconciler) prepare(ctx context.Context, tr *v1beta1.TaskRun) (*v1beta1 // error but it does not sync updates back to etcd. It does not emit events. // `reconcile` consumes spec and resources returned by `prepare` func (c *Reconciler) reconcile(ctx context.Context, tr *v1beta1.TaskRun, rtr *resources.ResolvedTaskResources) error { - defer c.durationAndCountMetrics(ctx, tr) logger := logging.FromContext(ctx) recorder := controller.GetEventRecorder(ctx) var err error