Skip to content
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

prometheus best practices, TLS and basic auth support #200

Merged
merged 8 commits into from
May 24, 2022

Conversation

yeoldegrove
Copy link
Contributor

The main point was to enable TLS support.
One pre-requisite for this was a log.Logger object (reference) which is not available in the currently used logging package logrus.
I took this as an opportunity to port some best practices from e.g. HAProxy exporter and MySQL Server exporter. This includes:

  • using promlog instead of logrus for logging a38c72a
    • DEPREACTION: log format changed due to this
  • using kingpin instead if pflag to parse commandline a38c72a
    • this is completely backwards compatible for the config file and command line argument setup
      • some flags are marked as deprecated but will still work
    • includes a wrapper to parse viper config and overwrite it with kinpin flags
  • add TLS and basic auth support by using exporter-toolkit
  • build binaries with promu 945cbc1
  • test: build, run and test binary 3017a2f

In addition to that, the OBS + release CI is kind of broken at the moment. These commits tend to fix it:

@stefanotorresi stefanotorresi self-requested a review May 23, 2022 13:39
@stefanotorresi stefanotorresi self-assigned this May 23, 2022
Copy link
Member

@stefanotorresi stefanotorresi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some questions inline just to remind myself, let's have a chat face to face to discuss some of the more extensive changes.

.promu.yml Show resolved Hide resolved
Makefile Show resolved Hide resolved
ha_cluster_exporter.yaml Show resolved Hide resolved
@yeoldegrove
Copy link
Contributor Author

@stefanotorresi force pushed the changes you wanted. It is ready to merged from my point of view.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants