[abhijeet-dhumal]

What this PR does / why we need it

Adds six upstream Trainer Prometheus metrics for runtime inventory, TrainJob creation, completion, failure, lifetime, and requested training-node counts proposed in #3429:

Metrics are registered only by the elected controller leader and collected from informer-backed snapshots without Kubernetes API calls during scraping. The implementation also handles duplicate terminal events, runtime updates/deletions, and leader transitions.

TLS, authentication, and ServiceMonitor configuration are handled separately by PR #4102.

• kubeflow_trainer_runtime_info: current TrainingRuntime and ClusterTrainingRuntime inventory.
• kubeflow_trainer_trainjob_created_total: TrainJobs observed after the initial informer cache listing.
• kubeflow_trainer_trainjob_finished_total: terminal outcomes by result, reason, and runtime.
• kubeflow_trainer_trainjob_lifetime_seconds: time from TrainJob creation to terminal state.
• kubeflow_trainer_trainjob_requested_training_nodes_per_job: requested training-node size.

The controller uses its existing informer caches instead of calling the Kubernetes API during metric collection. Runtime information is kept in an in-memory snapshot and reused when processing TrainJobs.
Only the elected controller leader processes metric events and registers the Trainer metrics. When leadership changes, the old leader removes its metrics and the new leader registers them. This prevents duplicate metrics from multiple controller replicas.

The implementation also:

• avoids counting TrainJobs already present when the cache starts;
• records terminal metrics only once per TrainJob;
• avoids high-cardinality labels such as TrainJob names and UIDs;
• includes unit and race-test coverage;
• documents metric behavior, PromQL examples, and limitations.

Validation

• Unit tests for runtime inventory, TrainJob events, runtime policy resolution, terminal-state handling, and metric lifecycle.
• Race tests for the metrics package.
• go vet ./pkg/metrics/...
• Formatting and git diff --check.
• Existing repository code-quality, generation, and language test checks.
• Prometheus metrics architecture and monitoring documentation.

Follow-up

Final CI, documentation build, approver review, and cluster E2E validation remain required before merge.

Checklist

• Docs included for the user-facing metrics and monitoring changes
• Unit tests added
• Leader ownership and registration lifecycle covered

[google-oss-prow]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign tenzen-y for approval. For more information see the Kubernetes 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

[robert-bell]

Thanks @abhijeet-dhumal! I've left some inline feedback.

[abhijeet-dhumal]

Thanks @robert-bell .. I’ve tried to address the metric scope and test coverage updates.

[robert-bell]

@abhijeet-dhumal I ran the new PR review skill over this - can you check if these any of these comments are valid?

[andreyvelich]

Thanks @abhijeet-dhumal for the work on this! Review done using AI tools; inline comments below.

[andreyvelich]

/milestone v2.4

[robert-bell]

Thanks @abhijeet-dhumal this is looking pretty close!

[andreyvelich]

We don't need to add this in the phase 1, right @robert-bell @abhijeet-dhumal ?

Suggested change

Name: "trainjob_requested_training_nodes_per_job",

[robert-bell]

I'd be keen to keep this in phase 1. It gives admins info about job sizes which is useful for investing in optimising network configs.

[andreyvelich]

Why do we need a separate informer handler @abhijeet-dhumal @robert-bell?
In JobSet, we directly call the metrics counter from the controller loop: https://github.com/kubernetes-sigs/jobset/blob/main/pkg/controllers/failure_policy.go#L432

[robert-bell]

Using the informer completely decouples the metrics instrumentation from the reconciliation logic keeping both clean and easier to evolve independently.

[andreyvelich]

Do we prefer approach to decouple it?

Would love to hear feedback from the Kueue maintainer on this too @kannon92 @tenzen-y!
https://github.com/kubernetes-sigs/kueue/tree/main/pkg/metrics

[abhijeet-dhumal]

I prefer keeping the informer-based observer... it keeps metrics independent of reconciliation and records lifecycle events without adding metrics work to the reconcile path.

[andreyvelich]

@abhijeet-dhumal we talked about this today at the Trainer call with @robert-bell and @tenzen-y.
Ref: https://youtu.be/nsE0a4T9LFM?t=959

Given the trade-offs of using separate watchers for events, could we explore how other Kubernetes projects handle event processing, particularly since our current implementation differs from Kueue and Training Operator v1?

My main concern with your proposed approach is that metric updates would rely solely on eventual consistency, as the metrics might not be updated at the time the corresponding event is processed.

[abhijeet-dhumal]

@andreyvelich I’ve tried to address the review comments in the latest changes, including the documentation updates. Could you please take another look when you have a chance and let me know if anything else needs work?

[andreyvelich]

/milestone clear

[abhijeet-dhumal]

Need to discuss/align and sign off on structural LLD for this implementation!
cc: @robert-bell