Add basic auth support for Prometheus pushgateway #22548
Conversation
|
@AndrewDi Please sign the Contributor License Agreement! Click here to manually synchronize the status of this Pull Request. See the FAQ for frequently asked questions. |
|
@AndrewDi Thank you for signing the Contributor License Agreement! |
spring-projects-issues
added
the
status: waiting-for-triage
| /** | ||
| * Enable publishing via a Prometheus Pushgateway with Basic Auth. | ||
| */ | ||
| private Boolean authEnabled = false; |
There was a problem hiding this comment.
We don't have such enabled flag. What we usually do is to check that the username is set, something like StringUtils.hasText can help you with that.
There was a problem hiding this comment.
you mean check this value in PrometheusMetricsExportAutoConfigurationTests?
There was a problem hiding this comment.
No. I mean that we don't enable authentication based on a flag but based on the fact that the username property is set.
...ringframework/boot/actuate/autoconfigure/metrics/export/prometheus/PrometheusProperties.java
Outdated
Show resolved
Hide resolved
| private Boolean authEnabled = false; | ||
|
|
||
| /** | ||
| * Basic Auth Username. |
There was a problem hiding this comment.
I am not sure if "basic auth" is really important here.
There was a problem hiding this comment.
pushgateway already support basic auth, we just need to pass auth parameters to setConnectionFactory function.
There was a problem hiding this comment.
I am aware of that. The comment is on the key description.
...ringframework/boot/actuate/autoconfigure/metrics/export/prometheus/PrometheusProperties.java
Outdated
Show resolved
Hide resolved
snicoll
changed the title
snicoll
added
the
status: waiting-for-feedback
|
@AndrewDi how is it going? Did you get a chance to look at the review? |
snicoll
added
type: enhancement
|
I've decided to give this one anyway, see this polish commit. |
We hide pushgateway behind nginx, and use HTTP basic auth, so need pushgateway to implement it.