search

boynux / squid-exporter

Squid Prometheus Exporter
https://www.boynux.com/squid-exporter
MIT License
134 stars 53 forks source link

Adopt use of prometheus/exporter-toolkit #95

Closed dswarbrick closed 4 months ago

dswarbrick commented 4 months ago

exporter-toolkit provides easy implementation of TLS support and other niceties such as generation of a uniform landing page.

Bump prometheus/client_golang and prometheus/common dependencies to more recent versions.

Fixes: #86

dswarbrick commented 4 months ago

@boynux I have deliberately kept using the standard lib flag package for this PR, but I propose migrating to kingpin in a subsequent PR, like the majority of other exporters already use. This is largely because the exporter-toolkit package relies on it for maximum efficacy, e.g. for specifying log format / level in a standardized way.

boynux commented 4 months ago

Thank you, I'll review this and make a test build. I appreciate your contribution, also it'd be good if you can add option in the README.md

dswarbrick commented 4 months ago

... also it'd be good if you can add option in the README.md

What did you have in mind? The (quite extensive) documentation for the web config file is hosted by the exporter-toolkit package, via the URL displayed in the -web.config.file flag default / hint.

boynux commented 4 months ago

... also it'd be good if you can add option in the README.md

What did you have in mind? The (quite extensive) documentation for the web config file is hosted by the exporter-toolkit package, via the URL displayed in the -web.config.file flag default / hint.

perhaps you can add it to this list https://github.com/prometheus/exporter-toolkit/blob/master/docs/web-configuration.md ?

a one liner would be sufficient

dswarbrick commented 4 months ago

I'm going to assume that you did not mean for me to add something to web-configuration.md, since that lives in a different project and would require a different PR.

Can you clarify what list you are referring to?

boynux commented 4 months ago

I'm going to assume that you did not mean for me to add something to web-configuration.md, since that lives in a different project and would require a different PR.

Can you clarify what list you are referring to?

doh! of course, my bad, wrong paste naturally i meant here: https://github.com/boynux/squid-exporter?tab=readme-ov-file#usage

boynux commented 4 months ago

I tested the code and it seems everything is working as expected. I'll happy to merge this, please update the readme and I'll bump the version once this is merged.

Thank you

boynux commented 4 months ago

Never mind, I have to update the version tag anyway, I'm merging this and will update the readme with the version.

Thank you!