✨ Add new metrics and testing documentation

[Karthik-K-N]

Description

This PR adds few metrics into the controller.

Contains only metrics part from PR: #185

Related Issue

Type of Change

/kind feature

Testing

Checklist

• make test passes
• make lint passes

Does this PR introduce a user-facing change?

Add new metrics and testing documentation

Doc #(issue)

[netlify]

✅ Deploy Preview for node-readiness-controller canceled.

Name	Link
🔨 Latest commit	398a91c
🔍 Latest deploy log	https://app.netlify.com/projects/node-readiness-controller/deploys/6a212d9f3908a20008fb78db

[Karthik-K-N]

/cc @AvineshTripathi

[AvineshTripathi]

Thanks for the PR @Karthik-K-N. I have put some comments! PTAL

[AvineshTripathi]

looking at this, I was wondering if we should club this and  https://github.com/kubernetes-sigs/node-readiness-controller/pull/271/changes#diff-6e56d7c057f6a96124b78f2b669dd7dface2718c304aece205017d114b3a7088R43?? What do you think?

I feel both share the same purpose

[Karthik-K-N]

which one are you referring to? I am not able to trace.

[AvineshTripathi]

apologies, here is the link https://github.com/Karthik-K-N/node-readiness-controller/blob/398a91c694977524361d55de0d9997f1e1c5275b/internal/metrics/metrics.go#L43

[Karthik-K-N]

No Issues thanks.

EvaluationDuration is an internal profiling metric, it tells us how fast our Go code executes a single rule evaluation (usually in milliseconds). ReconciliationLatency is our external Service Level Indicator (SLI). It measures the total end-to-end reaction time from the moment the node changed state in the real world to the moment we successfully patched the taint (which includes API server lag, rate limiting, and queue wait times, usually in seconds).
Keeping them separate allows us to instantly know if a slowdown is caused by inefficient code vs. Kubernetes API queueing.

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Karthik-K-N
Once this PR has been reviewed and has the lgtm label, please ask for approval from ajaysundark. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[ajaysundark]

cc @rawadhossain

[rawadhossain]

Thanks for looping me into this PR (somehow missed it earlier😅). I went through the changes, and this aligns quite well with the Headlamp project.

The separation between EvaluationDuration and ReconciliationLatency makes sense, and the additional rule-level metrics would be helpful for the Grafana dashboards and Headlamp views we'll be building.

Also nice to see nodes_by_state being updated from both reconciler paths so the state remains accurate regardless of where the reconciliation is triggered.

Looking forward to building on top of this once it lands.

cc @ajaysundark @Karthik-K-N

[ajaysundark]

In a high level, metric shapes look good to me. But I'm not sure if updating state / metric per event is scalable for the controller. We may need to take a second look at the compute cost for these

[Karthik-K-N]

May be can we get this in to unblock or not to overlap with other work and based on the scale test results we can revisit and update the metrics?

[ajaysundark]

May be can we get this in to unblock or not to overlap with other work and based on the scale test results we can revisit and update the metrics?

Could we create separate feature branches for the LFX projects? it'd help with the velocity of the projects as well.