-
Notifications
You must be signed in to change notification settings - Fork 2k
Add node_pool to blockedEval metric #26215
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
356fdae
to
6bbff40
Compare
6bbff40
to
eb68f3f
Compare
eb68f3f
to
9343575
Compare
@allisonlarson The website/content/docs/operations/metrics-reference.mdx is now website/content/docs/reference/metrics.mdx |
Adds the node_pool to the blockedEval metrics that get emitted for resource/cpu, along with the dc and node class.
9343575
to
858eecf
Compare
858eecf
to
5adb4f8
Compare
dc25db2
to
83ee53e
Compare
83ee53e
to
964f3a6
Compare
964f3a6
to
73ad69b
Compare
73ad69b
to
327abdf
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This approach looks good, but I think we're missing a few spots where we need to add the pool. I did rg -e '\&structs\.Evaluation'
and it looks like we're missing 3 spots in nomad/job_endpoint.go
, 1 in nomad/fsm.go
, 1 in nomad/alloc_endpoint
.go, 1 in nomad/plan_apply.go
, etc. It'd be worth taking another pass over these.
The ease of missing one makes me wonder if we want a NewEvaluationForJob
constructor function that takes a Job
, instead of hand-whittling the evaluation each time? That'd let us make sure all the appropriate fields are getting set.
Summary of our offline discussion: Having the node pool in the "root" of the eval isn't something we can use right now, and most consumers of it in the future will probably have the |
327abdf
to
3eb8b75
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Description
Adds the node_pool to the blockedEval metrics that get emitted for
resource/cpu, along with the dc and node class.
Docs have been updated to include the new metric label
Testing & Reproduction steps
The node_pool has been added to the automated tests, and seen when using manual tests and observing metrics emitted.
Links
Fixes #25933
Contributor Checklist
changelog entry using the
make cl
command.ensure regressions will be caught.
and job configuration, please update the Nomad website documentation to reflect this. Refer to
the website README for docs guidelines. Please also consider whether the
change requires notes within the upgrade guide.
Reviewer Checklist
backporting document.
in the majority of situations. The main exceptions are long-lived feature branches or merges where
history should be preserved.
within the public repository.