Basic auth: add metrics #24
Conversation
roidelapluie
force-pushed
the
metrics1
branch
from
e1d4cf1
to
4fc67c9
Compare
|
Ready for a second review, once that is in I will add metrics for the TLS part (or I can do in the same PR if you want) |
| u.failuresCounter = prometheus.NewCounter( | ||
| prometheus.CounterOpts{ | ||
| Namespace: "prometheus_toolkit", | ||
| Subsystem: "https", |
There was a problem hiding this comment.
https and basic auth are not the same thing
In general, it's better to have the metric name as a single string literal as it makes grepping and determining what the actual metric name is easier.
There was a problem hiding this comment.
I have renamed the metric. However I think that keeping namespace separated is better, it that's fine for you.
There was a problem hiding this comment.
I preferred it named based on the package name.
There was a problem hiding this comment.
Should we change the package name?
Signed-off-by: Julien Pivotto <[email protected]>
Signed-off-by: Julien Pivotto <[email protected]>
roidelapluie
force-pushed
the
metrics1
branch
from
4fc67c9
to
c652415
Compare
|
Would renaming the package to http help, like this? |
roidelapluie
force-pushed
the
metrics1
branch
from
e633264
to
c652415
Compare
Signed-off-by: Julien Pivotto <[email protected]>
|
I did not like the "http" package rename as it is too close with go's net/http. I have reworked this to have a metric name: |
Signed-off-by: Julien Pivotto [email protected]