[haproxy_otel] Haproxy OpenTelemetry assets - #17189
Conversation
ritalwar
added
documentation
|
@claude for detailed initial review. |
|
Claude finished @lalit-satapathy's task in 3m 28s —— View job Detailed Initial ReviewI've completed the detailed review and submitted my feedback via the PR review system.
Review Summary: The HAProxy OpenTelemetry content pack is well-structured overall. I found 2 medium-severity and 4 low-severity items to address:
Good practices observed:
See the full review for inline comments and suggested fixes. --- | Branch: Why is Claude responding? | Type Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not. |
![github-actions[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
This is a well-structured HAProxy OpenTelemetry content pack with good dashboard coverage. I've added a few suggestions that can improve the quality and consistency with other OTel content packs in this repository.
Summary of key feedback:
-
Screenshots are commented out - The screenshot images already exist in the
/imgfolder but thescreenshotssection inmanifest.ymlis commented out. Enabling these would improve discoverability in Kibana. -
OTel config example incomplete - The example configuration in the README is missing the exporter in the service pipeline, and the
batchprocessor is used but not defined. Users following this example literally would not get data into Elasticsearch. -
Minor documentation improvements - The HAProxy config example should be in a code block, and a brief metrics reference table (like in nginx_otel) would be helpful.
Good practices observed:
- Proper CODEOWNERS entry
- Changelog follows correct format with PR link
- Dashboard naming is consistent (
[Metrics HAProxy Otel]prefix) - Appropriate Kibana version constraint (
^9.2.0) - Uses
type: contentcorrectly for OTel content packs - Dataset discovery properly configured
Nitpick comments (1)
💬 NITPICK Icon title could be more descriptive — packages/haproxy_otel/manifest.yml:28
Why: Other packages use more descriptive icon titles (e.g. 'logo HAProxy' or 'Logo MySQL OTEL')
icons:
- src: /img/logo_haproxy.svg
title: HAProxy logo
size: 32x32
type: image/svg+xmlWhy is Claude responding? | Type @claude to interact further
Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.
| version: 0.1.0 | ||
| source: | ||
| license: "Elastic-2.0" | ||
| description: "Haproxy stats metrics from OpenTelemtry Collector" |
There was a problem hiding this comment.
⚪ LOW Typo in description
Why: Typo 'OpenTelemtry' should be 'OpenTelemetry' for consistency
| description: "Haproxy stats metrics from OpenTelemtry Collector" | |
| description: "Haproxy stats metrics from OpenTelemetry Collector" |
Why is Claude responding? | Type @claude to interact further
Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.
| discovery: | ||
| datasets: | ||
| - name: haproxyreceiver.otel | ||
| # screenshots: |
There was a problem hiding this comment.
🟡 MEDIUM Screenshots are commented out
Why: Other OTel content packs include screenshots which improve discoverability in Kibana. The images already exist in /img folder
| # screenshots: | |
| screenshots: | |
| - src: /img/haproxy-metrics-overview-otel.png | |
| title: HAProxy OTel Metrics Overview | |
| size: 600x600 | |
| type: image/png | |
| - src: /img/haproxy-metrics-frontend-otel.png | |
| title: HAProxy OTel Frontend | |
| size: 600x600 | |
| type: image/png | |
| - src: /img/haproxy-metrics-backend-otel.png | |
| title: HAProxy OTel Backend | |
| size: 600x600 | |
| type: image/png | |
| - src: /img/haproxy-metrics-http-frontend-otel.png | |
| title: HAProxy OTel HTTP Frontend | |
| size: 600x600 | |
| type: image/png | |
| - src: /img/haproxy-metrics-http-backend-otel.png | |
| title: HAProxy OTel HTTP Backend | |
| size: 600x600 | |
| type: image/png | |
| - src: /img/haproxy-metrics-http-server-otel.png | |
| title: HAProxy OTel HTTP Server | |
| size: 600x600 | |
| type: image/png |
Why is Claude responding? | Type @claude to interact further
Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.
|
|
||
|
|
||
| service: | ||
| pipelines: |
There was a problem hiding this comment.
🟡 MEDIUM Missing exporter in service pipeline
Why: The service pipeline references processors and receivers but doesn't include exporters, which would prevent data from being sent to Elasticsearch
| pipelines: | |
| service: | |
| pipelines: | |
| metrics: | |
| receivers: [haproxy] | |
| processors: [batch] | |
| exporters: [elasticsearch/otel] |
Why is Claude responding? | Type @claude to interact further
Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.
| 2. Permissions required: The collector requires access to HAProxy metrics, which are exposed via either the stats socket or the HTTP stats endpoint. Ensure that the OTEL Collector can read from the socket or query the HTTP endpoint. For example, if using the stats socket, the user running the collector must have read permissions on the socket file. | ||
|
|
||
| 3. HAProxy configuration: HTTP stats endpoint: If using the HTTP stats page, configure HAProxy to allow access from the collector host. For example: | ||
|
|
There was a problem hiding this comment.
⚪ LOW HAProxy config should be in code block
Why: The HAProxy configuration example should be formatted as a code block for better readability, consistent with other config examples in the doc
| 3. HAProxy configuration: HTTP stats endpoint: If using the HTTP stats page, configure HAProxy to allow access from the collector host. For example: | |
| \`\`\` | |
| listen stats | |
| bind *:8404 | |
| mode http | |
| stats enable | |
| stats uri /metrics | |
| stats refresh 10s | |
| stats auth admin:password | |
| \`\`\` |
Why is Claude responding? | Type @claude to interact further
Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.
|
|
||
| - Export to Elasticsearch | ||
|
|
||
| Example: otel-collector-config.yaml: |
There was a problem hiding this comment.
⚪ LOW Missing processors definition
Why: The example config uses 'batch' processor in the pipeline but doesn't define it in the processors section. Similar OTel integrations include a resourcedetection processor too.
| Example: otel-collector-config.yaml: | |
| Example: otel-collector-config.yaml: | |
| \`\`\` | |
| receivers: | |
| haproxy: | |
| endpoint: http://localhost:8404/stats | |
| collection_interval: 10s | |
| metrics: | |
| haproxy.sessions.total: | |
| enabled: true | |
| # ... other metrics ... | |
| processors: | |
| batch: {} | |
| resourcedetection: | |
| detectors: ["system", "ec2"] | |
| exporters: | |
| debug: | |
| verbosity: normal | |
| elasticsearch/otel: | |
| endpoints: https://localhost:9200 | |
| # ... |
Why is Claude responding? | Type @claude to interact further
Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.
| 4. Enable [Optional Haproxy metrics](https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/receiver/haproxyreceiver/documentation.md#optional-metrics) in config file: HAProxy exposes a set of default metrics automatically. The OpenTelemetry HAProxy receiver has a list of optional metrics that are not enabled by default. If you want these optional metrics to appear in Elasticsearch, you must explicitly enable them in the OTel Collector HAProxy receiver configuration like in the example above. | ||
|
|
||
|
|
||
| ## Metrics reference |
There was a problem hiding this comment.
⚪ LOW Consider adding metrics reference table
Why: Other OTel integrations like nginx_otel include a metrics reference table which helps users understand what metrics are available. Currently this section only links to external documentation.
Why is Claude responding? | Type @claude to interact further
Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.
mykola-elastic
left a comment
mykola-elastic
left a comment
There was a problem hiding this comment.
Quick visuals review
Dashboards
- Adding a link panel can (or even shall) be considered to switch between dashboards easily. https://www.elastic.co/docs/explore-analyze/visualize/link-panels
- Markdown "dashboard overview" panels may be added with a brief description of the dashboard.
Logo
Most of the OTel assets CPs have a small OpenTelemetry logo in the corner, though there is no strict rule.
Screenshots
The dashboard screenshots are good, just I'd try to load the haproxy a little bit so that graphs look a bit more alive.
For example, a simple script that I used for nginx:
nginx-test.sh
- generate some GET, HEAD, OPTIONS requests (10 to 20 per second)
- generate GET requests at /not-found to trigger 404 errors (1-2 per second)
- generate GET requests at /throw503 (I configured the endpoint to always respond with 503) (1-2 requests every 5 seconds)
Of course it will be a bit different with Haproxy, maybe not worth it, up to you to decide.
Co-authored-by: Ishleen Kaur <102962586+ishleenk17@users.noreply.github.com>
|
I ran Results
Upgrade and post-upgrade compile succeeded with 0.4.0. Suggested commands: uvx --from kb-dashboard-cli==0.4.0 kb-dashboard upgrade --input-dir packages/haproxy_otel/_dev/shared --write
uvx --from kb-dashboard-cli==0.4.0 kb-dashboard compile --input-dir packages/haproxy_otel/_dev/shared --output-dir packages/haproxy_otel/kibana/dashboard --format elastic-integrations |
|
/test |
![macroscopeapp[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
💚 Build Succeeded
History
cc @ritalwar |
|
Package haproxy_otel - 0.1.0 containing this change is available at https://epr.elastic.co/package/haproxy_otel/0.1.0/ |
7 participants
Proposed commit message
This PR adds Haproxy Otel content pack which include assets like dashboards, alert_rule_templates and slo_template.
Checklist
changelog.ymlfile.Author's Checklist
How to test this PR locally
Related issues
Screenshots