Skip to content

Add micrometer.prometheus-proxy.connected-metrics-metadata property#68

Open
Mateo00 wants to merge 3 commits into
micrometer-metrics:mainfrom
Mateo00:connected-metrics-metadata
Open

Add micrometer.prometheus-proxy.connected-metrics-metadata property#68
Mateo00 wants to merge 3 commits into
micrometer-metrics:mainfrom
Mateo00:connected-metrics-metadata

Conversation

@Mateo00

@Mateo00 Mateo00 commented Mar 29, 2023

Copy link
Copy Markdown

Add micrometer.prometheus-proxy.connected-metrics-metadata property to enable or disable publishing Prometheus metrics metadata (lines "# HELP ..." and lines "# TYPE ...") in the payload returned by the /metrics/connected endpoint. The default value is True.

Disabling metrics metadata reduces the amount of data sent on each scrape.

In addition, some clients such as Telegarf expect the "# HELP ..." lines to be unique.
Currently, collecting metrics reported by the /metrics/connected endpoint fails with a Telegarf agent:
parsing metrics failed: reading text format failed: text format parsing error in line XX: second HELP line for metric name "XXXXX"

Disabling metrics metadata allows to work around this problem and scrapping the /metrics/connected endpoint with a Telegraf agent works well.

@pivotal-cla

Copy link
Copy Markdown

@Mateo00 Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

@pivotal-cla

Copy link
Copy Markdown

@Mateo00 Thank you for signing the Contributor License Agreement!

@Mateo00

Mateo00 commented Apr 20, 2023

Copy link
Copy Markdown
Author

Hi @shakuzen,
Is the PR compliant or is there a missing step on my end for it to be considered ?

@shakuzen

Copy link
Copy Markdown
Member

I think I've understood the problem. Losing the HELP lines seems somewhat alright, but losing the TYPE lines seems potentially more problematic. Where are you eventually storing the metrics after Telegraf scrapes them, and is there no issue for you losing all type info?

@Mateo00

Mateo00 commented May 15, 2023

Copy link
Copy Markdown
Author

After Telegraf scrapes, the metrics are stored in a VictoriaMetrics database. A Grafana connected to this database allows us to visualize the collected metrics.
In our tests, we didn't find any problems when the TYPE lines are deleted. All metrics are saved in the database and they display correctly in Grafana dashboards.

In the Prometheus documentation it's mentioned that HELP and TYPE lines are optional.
https://prometheus.io/docs/instrumenting/exposition_formats/#grouping-and-sorting :
Grouping and sorting
All lines for a given metric must be provided as one single group, with the optional HELP and TYPE lines first (in no particular order). Beyond that, reproducible sorting in repeated expositions is preferred but not required, i.e. do not sort if the computational cost is prohibitive.
Each line must have a unique combination of a metric name and labels. Otherwise, the ingestion behavior is undefined.

* Whether to enable publishing of Prometheus metadata (lines "# HELP ..." and lines "# TYPE ...") in the payload returned by the /metrics/connected endpoint.
* Disabling metadata publishing reduces the amount of data sent sent on each scrape.
*/
private boolean connectedMetricsMetadata = true;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I would rename the property to publishConnectedMetricsMetadata rather than connectedMetricsMetadata

@Mateo00 Mateo00 Mar 4, 2025

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thanks for your feedback.
I renamed the property to publishConnectedMetricsMetadata and also resynchronized the forked repo with the original.

@Mateo00 Mateo00 force-pushed the connected-metrics-metadata branch 3 times, most recently from 3457186 to 627c2aa Compare March 4, 2025 00:07
Signed-off-by: Mateo00 <mateo00@developer.org>
@Mateo00 Mateo00 force-pushed the connected-metrics-metadata branch from cf1ba90 to c761726 Compare March 4, 2025 00:17
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.

4 participants