-
Notifications
You must be signed in to change notification settings - Fork 874
feat: add prebuilds metrics collector #17547
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
2431bbe
to
45a0672
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.
Just pointing out a TODO that we still need to look into. Otherwise, looks good.
45a0672
to
80fba7d
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.
A few small suggestions/nits, otherwise LGTM
8c3e53e
to
9676b65
Compare
a69ee41
to
7791f50
Compare
d43c13d
to
00ebdde
Compare
Signed-off-by: Danny Kopping <dannykopping@gmail.com> Reverting unnecessary changes that crept in Signed-off-by: Danny Kopping <dannykopping@gmail.com>
Signed-off-by: Danny Kopping <dannykopping@gmail.com>
Signed-off-by: Danny Kopping <dannykopping@gmail.com>
Signed-off-by: Danny Kopping <dannykopping@gmail.com>
Signed-off-by: Danny Kopping <dannykopping@gmail.com>
00ebdde
to
1d66ade
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.
One minor nit/observation, otherwise LGTM!
{"coderd_prebuilt_workspaces_eligible", ptr.To(0.0), false}, | ||
}, | ||
templateDeleted: []bool{false}, | ||
eligible: []bool{false}, |
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.
I'm not seeing any test-cases where we'd need slices for deleted/eligible and it probably isn't too compatible with the way expected metrics are defined. Consider simplifying (helps reduce complexity of test body too)?
Closes coder/internal#509