-
Notifications
You must be signed in to change notification settings - Fork 69
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
Add faillint paths to reject global prometheus metrics #18
Conversation
Signed-off-by: Tyler Reid <[email protected]>
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! But we need to get CI passes fixing issues before merging.
Signed-off-by: Tyler Reid <[email protected]>
Vendor went off, let me fix that |
This reverts commit 052a84b.
Signed-off-by: Tyler Reid <[email protected]>
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 think it looks as if there are some typos, dupes in the faillint
config?
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.
How are all the functions comments changes related to this PR?
Don't understand why unrelated comments have changed in this PR.
The comments were failing golang ci lint, so I wanted to fix that with the other linter issues as I went. But you're right, I'll pull it to a different PR since it doesn't make sense here. |
This reverts commit 64bc82d.
Signed-off-by: Tyler Reid <[email protected]>
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 think there's duplication in the lint rules.
Signed-off-by: Tyler Reid <[email protected]>
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
Let there be Lint! |
This covers metrics registered with
prometheus.{NewCounter,NewCounterVec,NewCounterVec,NewGauge,NewGaugeVec,NewGaugeFunc,\ NewHistorgram,NewHistogramVec,NewSummary,NewSummaryVec}
and will suggest to usepromauto.With()
to register metrics to prevent registering of global metrics. We will also fail onprometheus.MustRegister()
calls as metrics should be registered automatically with promauto.Signed-off-by: Tyler Reid [email protected]