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

haproxy: enable the Prometheus Exporter service #27442

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

greed42
Copy link

@greed42 greed42 commented Jan 21, 2025

Description

haproxy: enable the Prometheus Exporter service

Type(s)
  • bugfix
  • enhancement
  • security fix
Tested on

macOS 14.7.2 23H311 arm64
Xcode 16.2 16C5032a

Verification

Have you

  • followed our Commit Message Guidelines?
  • squashed and minimized your commits?
  • checked that there aren't other open pull requests for the same change?
  • referenced existing tickets on Trac with full URL in commit message?
  • checked your Portfile with port lint?
  • tried existing tests with sudo port test?
  • tried a full install with sudo port -vst install?
  • tested basic functionality of all binary files?
  • checked that the Portfile's most important variants haven't been broken?

@macportsbot
Copy link

Notifying maintainers:
@judaew for port haproxy.
@aphor for port haproxy.

@macportsbot macportsbot added type: enhancement maintainer: open Affects an openmaintainer port labels Jan 21, 2025
variant promex description {Build with Prometheus Exporter service} {
build.args-append USE_PROMEX=1
}

Copy link
Contributor

Choose a reason for hiding this comment

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

why does this need to be a variant? It doesn't appear to have any additional dependencies, so why not always enable it?

Copy link
Author

Choose a reason for hiding this comment

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

I made it a variant because it's opt-in in "base" haproxy and opt-out in the nix package of haproxy.

But that's not a great reason; I'll make it always-on.

Copy link
Contributor

Choose a reason for hiding this comment

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

but are you sure it doesn't need an external dependency?

Copy link
Author

Choose a reason for hiding this comment

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

Yes:

  • the top-level Makefile adds only source and include paths inside the HAProxy repo
  • the #include statements in addons/promex source and header files) reference only files inside the HAProxy repo
  • and the nix package dependency list is invariant over withPrometheusExporter (I'm totally cribbing from nix because I'm testing something locally that will run on NixOS in production)

@greed42 greed42 changed the title haproxy: Add variant promex to include the Prometheus Exporter service haproxy: enable the Prometheus Exporter service Jan 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintainer: open Affects an openmaintainer port type: enhancement
Development

Successfully merging this pull request may close these issues.

4 participants