-
Notifications
You must be signed in to change notification settings - Fork 28
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
Send registry image pulls metrics to prometheus #1238
base: master
Are you sure you want to change the base?
Conversation
Change-type: patch
5da6a48
to
d06ab34
Compare
|
||
export function incrementRegistryImagePulls( | ||
value: number = 1, | ||
payload: any = {}, |
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.
Can you type this better than any
please
); | ||
|
||
export function incrementRegistryImagePulls( | ||
value: number = 1, |
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.
When you specify a default value then the type defaults to the type of that default value:
value: number = 1, | |
value = 1, |
There's nothing wrong with explicitly specifying number
but it's redundant and makes sense imo to reserve for only the cases where you want a type that is different to that of the default value
@@ -591,6 +592,17 @@ export const token: RequestHandler = async (req, res) => { | |||
authorizeRequest(req, scopes, tx), | |||
]), | |||
); | |||
|
|||
access.forEach((ac) => | |||
incrementRegistryImagePulls(1, { |
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 would name the metric differently to show that it is images that have had a token validated for access rather than image pulls (you can use the image to pull 0 or 2+ times as well so it's not 1-1)
Change-type: patch