Updated HAProxy Prometheus documentation #587

algchoo · 2023-07-14T18:57:11Z

Changes

Updated haproxy config, port name, and image version. Updated minimum version to match haproxy version
Changed haproxy exporter type to be 'included' and updated the documentation to fit that template
added back deployment yaml, named port, and metric verification instructions

Details
The exporter initially used has been retired and an official exporter is available as of version 2.4. The changes here remove the unofficial exporter from the documentation and shift the focus towards the official exporter.

There are instructions on building HAProxy with the Prometheus exporter module enabled and the docker images seem to have this enabled by default, I checked the latest available version of 2.4 and 2.8, both have it enabled. The metrics appear to be the same, or changes that are present don't affect the dashboard.

The documentation has been modified to fit the format for exporter_type: included.

… version to match haproxy version

yqlu · 2023-07-14T19:49:09Z

This seems like it would be a bigger change, because we want to switch the documentation from type=sidecar to type=included

With reference to the published docs, we want it to look more like
https://cloud.google.com/stackdriver/docs/managed-prometheus/exporters/airflow
or maybe https://cloud.google.com/stackdriver/docs/managed-prometheus/exporters/flink

which is being populated by https://github.com/GoogleCloudPlatform/monitoring-dashboard-samples/blob/master/integrations/airflow/documentation.yaml and https://github.com/GoogleCloudPlatform/monitoring-dashboard-samples/blob/master/integrations/flink/documentation.yaml

Which means...

switching exporter_type from sidecar -> included
adding analogous "additional_prereq_info" like what we did in flink / airflow about how HAProxy exposes Prometheus-format metrics automatically if you update the config map in this way... and run this command to verify metrics are emitted on the expected endpoint...
get rid of the "additional_install_info" which is now irrelevant
update exporter_name, exporter_pkg_name, exporter_repo_url, etc (and the relevant fields in integrations/haproxy/prometheus_metadata.yaml) to no longer point to the old exporter and github repo

…ntation to fit that template

algchoo · 2023-07-17T18:42:16Z

@yqlu Thank you for the review, changes have been pushed based on the feedback.

integrations/haproxy/documentation.yaml

yqlu · 2023-07-19T14:02:02Z

Tagging @mtabasko for additional scrutiny because this is a larger change (switching exporter types entirely)

mtabasko · 2023-07-20T02:22:35Z

Thanks, YQ. How urgent is this? If urgent, I will try to look at it tomorrow (Thurs, 20 july). (Next week would be much better -- I'm really swamped right now. Sorry!)

…uctions

yqlu · 2023-07-20T13:26:00Z

Not urgent -- next week is fine!

integrations/haproxy/documentation.yaml

mtabasko

Sorry this took me so long to get to. YQ, I've staged a version of the HAProxy doc that incorporates the changes I've suggested here, too.

integrations/haproxy/documentation.yaml

mtabasko · 2023-07-31T00:37:56Z

integrations/haproxy/documentation.yaml

+  Most of the official docker images for versions greater than or equal to 2.4 are built with this service enabled.
+  The following example was built referencing [HAProxy Enterprise documentation](https://www.haproxy.com/documentation/hapee/latest/observability/metrics/prometheus/).
+  It works with the community edition and can be modified to suit specific needs:
+  <pre>


Is this intended to be included inline (and not extracted into the an external file (which is what we do with "config_mods" As is, this will just be an inline block.

@mtabasko Sorry for not getting back sooner, I hope you're doing well!

I believe that this being inline is appropriate, based on other included exporters. I wasn't sure that including a config_mods section in this file made sense and if it does I'd be happy to make changes accordingly.

Hi, Austin! Hope you're doing well, too!

I don't have a strong opinion about this -- I realize that you don't have great insight into what we do with this downstream, so I just wanted to make sure that "inline" reflected your intent. So I have no objection. If you and YQ are good with it, that's good enough for me!

I'm good with this, if everyone else is.

Yep, I think this makes sense -- https://cloud.google.com/stackdriver/docs/managed-prometheus/exporters/flink is the one we want to model this after.

integrations/haproxy/documentation.yaml

mtabasko · 2023-07-31T00:38:10Z

integrations/haproxy/documentation.yaml

@@ -81,6 +85,9 @@ podmonitoring_config: |
    selector:
      matchLabels:
        app.kubernetes.io/name: haproxy
+additional_podmonitoring_info: |
+  Ensure that the values of the `port` and `matchLabels` fields match those of the {{app_name_short}} pods you want to monitor.


Not clear if this is one paragraph or two. line 89 and 90 are "thematically" different.

Opted to make this two separate paragraphs, but if it doesn't look right we do something different.

I think two paragraphs is good here. Thanks!

mtabasko · 2023-08-02T15:29:48Z

integrations/haproxy/documentation.yaml

@@ -81,6 +85,9 @@ podmonitoring_config: |
    selector:
      matchLabels:
        app.kubernetes.io/name: haproxy
+additional_podmonitoring_info: |
+  Ensure that the values of the `port` and `matchLabels` fields match those of the {{app_name_short}} pods you want to monitor.


I think two paragraphs is good here. Thanks!

yqlu

Two small wording changes, then LGTM. Thanks Austin!!

yqlu · 2023-08-02T20:45:20Z

integrations/haproxy/documentation.yaml

+  Most of the official docker images for versions greater than or equal to 2.4 are built with this service enabled.
+  The following example was built referencing [HAProxy Enterprise documentation](https://www.haproxy.com/documentation/hapee/latest/observability/metrics/prometheus/).
+  It works with the community edition and can be modified to suit specific needs:
+  <pre>


Yep, I think this makes sense -- https://cloud.google.com/stackdriver/docs/managed-prometheus/exporters/flink is the one we want to model this after.

yqlu · 2023-08-02T20:45:29Z

integrations/haproxy/documentation.yaml

@@ -81,6 +85,9 @@ podmonitoring_config: |
    selector:
      matchLabels:
        app.kubernetes.io/name: haproxy
+additional_podmonitoring_info: |
+  Ensure that the values of the `port` and `matchLabels` fields match those of the {{app_name_short}} pods you want to monitor.


yqlu · 2023-08-02T21:28:20Z

integrations/haproxy/documentation.yaml

-exporter_name: the HAProxy exporter
-exporter_pkg_name: haproxy_exporter
-exporter_repo_url: https://github.com/prometheus/haproxy_exporter
+exporter_name: the HAProxy promex exporter


Suggested change

exporter_name: the HAProxy promex exporter

exporter_name: the PROMEX service for HAProxy

Reading through https://github.com/haproxy/haproxy/blob/master/addons/promex/README and I think that's a more accurate name (they don't ever use "promex" in lowercase).

yqlu · 2023-08-02T21:28:31Z

integrations/haproxy/documentation.yaml

+  {{app_name_short}} exposes Prometheus-format metrics only when it is 
+  [built with the service enabled](https://github.com/haproxy/haproxy/blob/master/addons/promex/README){:class="external"} and an appropriate `frontend` is included in the configuration.
+
+  Most of the official docker images for versions greater than or equal to 2.4 are built with this service enabled.


Suggested change

Most of the official docker images for versions greater than or equal to 2.4 are built with this service enabled.

Most of the official Docker images for versions greater than or equal to 2.4 are built with this service enabled.

* Updated haproxy config, port name, and image version. Updated minimum version to match haproxy version * Changed haproxy exporter type to be 'included' and updated the documentation to fit that template * added back deployment yaml, named port, and metric verification instructions * added class to links, added new lines * small wording changes

Updated haproxy config, port name, and image version. Updated minimum…

e86b475

… version to match haproxy version

github-actions bot requested a review from yqlu July 14, 2023 18:57

Changed haproxy exporter type to be 'included' and updated the docume…

bbbc4fc

…ntation to fit that template

yqlu reviewed Jul 19, 2023

View reviewed changes

integrations/haproxy/documentation.yaml Show resolved Hide resolved

integrations/haproxy/documentation.yaml Show resolved Hide resolved

integrations/haproxy/documentation.yaml Outdated Show resolved Hide resolved

added back deployment yaml, named port, and metric verification instr…

cf69b5b

…uctions

mtabasko reviewed Jul 31, 2023

View reviewed changes

integrations/haproxy/documentation.yaml Show resolved Hide resolved

mtabasko reviewed Jul 31, 2023

View reviewed changes

integrations/haproxy/documentation.yaml Outdated Show resolved Hide resolved

mtabasko suggested changes Jul 31, 2023

View reviewed changes

added class to links, added new lines

388c757

mtabasko approved these changes Aug 2, 2023

View reviewed changes

yqlu approved these changes Aug 2, 2023

View reviewed changes

small wording changes

9d7bdd6

yqlu approved these changes Aug 2, 2023

View reviewed changes

yqlu merged commit 18f8aa1 into GoogleCloudPlatform:master Aug 2, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Updated HAProxy Prometheus documentation #587

Updated HAProxy Prometheus documentation #587

algchoo commented Jul 14, 2023 •

edited

Loading

yqlu commented Jul 14, 2023

algchoo commented Jul 17, 2023

yqlu commented Jul 19, 2023

mtabasko commented Jul 20, 2023

yqlu commented Jul 20, 2023

mtabasko left a comment

mtabasko Jul 31, 2023

algchoo Aug 2, 2023

mtabasko Aug 2, 2023

algchoo Aug 2, 2023

yqlu Aug 2, 2023

mtabasko Jul 31, 2023

algchoo Aug 2, 2023

mtabasko Aug 2, 2023

yqlu Aug 2, 2023

mtabasko Aug 2, 2023

yqlu left a comment

yqlu Aug 2, 2023

yqlu Aug 2, 2023

yqlu Aug 2, 2023

yqlu Aug 2, 2023

	exporter_name: the HAProxy promex exporter
	exporter_name: the PROMEX service for HAProxy

	Most of the official docker images for versions greater than or equal to 2.4 are built with this service enabled.
	Most of the official Docker images for versions greater than or equal to 2.4 are built with this service enabled.

Updated HAProxy Prometheus documentation #587

Updated HAProxy Prometheus documentation #587

Conversation

algchoo commented Jul 14, 2023 • edited Loading

yqlu commented Jul 14, 2023

algchoo commented Jul 17, 2023

yqlu commented Jul 19, 2023

mtabasko commented Jul 20, 2023

yqlu commented Jul 20, 2023

mtabasko left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yqlu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

algchoo commented Jul 14, 2023 •

edited

Loading