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

Add optional support for data type and description for metrics #48

Closed
jlosito opened this issue Apr 20, 2023 · 12 comments
Closed

Add optional support for data type and description for metrics #48

jlosito opened this issue Apr 20, 2023 · 12 comments
Assignees
Labels
enhancement New feature or request TBD To Be Done

Comments

@jlosito
Copy link

jlosito commented Apr 20, 2023

Prometheus metrics generally list a data type and a description of the metric. Some agents even look for this data. It generally looks like something below.

# HELP http_requests_total Total number of http api requests
# TYPE http_requests_total counter
http_requests_total{api="add_product"} 4633433

I believe this library doesn't add the data type nor the description which makes it difficult for some agents to scrape the data points.

Would it be possible to add the ability to denote a description and data type for each metric?

@cristaloleg
Copy link

cristaloleg commented Apr 20, 2023

which makes it difficult for some agents to scrape the data points.

Just curious, what agents fail to scrape?

@jlosito
Copy link
Author

jlosito commented Apr 20, 2023

@cristaloleg here's a link to prometheus docs describing the format. The # TYPE lines actually matter and are consumed as tokens.

https://github.com/prometheus/docs/blob/main/content/docs/instrumenting/exposition_formats.md#comments-help-text-and-type-information

@jlosito
Copy link
Author

jlosito commented Apr 21, 2023

I also found that the openmetrics standard also follows this format.

https://github.com/OpenObservability/OpenMetrics/blob/main/specification/OpenMetrics.md#metricfamily-metadata

@saez0pub
Copy link

saez0pub commented Jul 4, 2023

@cristaloleg datadog agent drops the metrics if TYPE is not set, I have to map each metric to the correct type.

@tenmozes
Copy link
Contributor

tenmozes commented Jul 4, 2023

https://cloud.google.com/stackdriver/docs/managed-prometheus/troubleshooting - same applicable for GCP, so it's reasonable to add these annotations and option to control it

@hagen1778 hagen1778 changed the title List data type and description for each metric Add optional support for data type and description for metrics Jul 4, 2023
@hagen1778 hagen1778 added enhancement New feature or request TBD To Be Done labels Jul 4, 2023
@hagen1778
Copy link
Contributor

We should provide an option for user to decide whether TYPE needs to be added to the metrics exposition format. The option should make metrics lib compatible with DD agent and Google Managed Prometheus.

@cristaloleg
Copy link

@hagen1778 Is there any reason why TYPE should not be provided at all?

@hagen1778
Copy link
Contributor

Is there any reason why TYPE should not be provided at all?

Yes, VictoriaMetrics ignores this information on ingestion. Since the main user of the metrics lib is VictoriaMetrics itself, it was decided to omit extra complexity.

@cristaloleg
Copy link

Ok, so just not implemeted instead of unachievable, thanks

@valyala
Copy link
Contributor

valyala commented Jul 4, 2023

Is there any reason why TYPE should not be provided at all?

The TYPE and HELP comments aren't provided by github.com/VictoriaMetrics/metrics package by default because of the following reasons:

  • These comments are ignored by VictoriaMetrics and vmagent, as @hagen1778 said above.
  • These comments require additional network bandwidth for transferring to metrics scraper such as Prometheus, VictoriaMetrics or vmagent. This may result in network bandwidth bottleneck in some setups.
  • These comments may complicate usage of the github.com/VictoriaMetrics/metrics package, since the user need to pass proper HELP and TYPE values in the code somehow.
  • These comments may contain invalid or misleading information. For example, the HELP line may contain irrelevant description for the metric, or the TYPE comment may contain invalid metric type (e.g. counter instead of gauge). The consumer of the /metrics page (either a human or metrics scraper) has no any protection against this, so it may break in various ways when consuming invalid data from these comments.

It is pity that some third-party scrapers require the TYPE or HELP comments given considerations mentioned above. The solution is to provide some kind of optional API at github.com/VictoriaMetrics/metrics, which could be used to instruct the package to generate some kind of TYPE and HELP comments per each exposed metric.

@mattmalec
Copy link

To anyone who's stuck on this like we were, we implemented support for HELP and TYPE so we could use the Datadog Agent to scrape Erigon metrics. It's built to be a drop-in replacement, so hopefully it shouldn't be hard for anyone else to use. Replacing the current implementation with ours was as simple as replacing the dependency in the go.mod file.

Hope this helps!

repo: https://github.com/ourzora/erigon-metrics
see implementation: ourzora@fa6fa51

hagen1778 added a commit that referenced this issue Dec 15, 2023
New public method `ExposeMetadata` allows enabling exposition
of dummy meta-info for all exposed metrics across all Sets.

This feature is needed to improve compatibility
with 3rd-party scrapers that require meta information to be present.

This commit doesn't update exposition of default system/process
metrics to keep the list of changes small. This change should
be added in a follow-up commit.

#48
@hagen1778 hagen1778 self-assigned this Dec 15, 2023
valyala added a commit that referenced this issue Dec 19, 2023
* allow exposing meta information for registered metrics

New public method `ExposeMetadata` allows enabling exposition
of dummy meta-info for all exposed metrics across all Sets.

This feature is needed to improve compatibility
with 3rd-party scrapers that require meta information to be present.

This commit doesn't update exposition of default system/process
metrics to keep the list of changes small. This change should
be added in a follow-up commit.

#48

* cleanup

* wip

* wip

* wip

* wip

---------

Co-authored-by: Aliaksandr Valialkin <valyala@victoriametrics.com>
@valyala
Copy link
Contributor

valyala commented Dec 19, 2023

Support for optional exposition of TYPE and HELP metadata has been added in the commit 9dc7358 . By default this metadata isn't exposed, since it isn't used by VictoriaMetrics components as explained in this comment. This commit adds an ability to enable exposing the TYPE and HELP metadata across all the metrics by calling metrics.ExposeMetadata(true):

  • The TYPE metadata contains metric name plus metric type
  • The HELP metadata contains metric name

This commit is included in the tag v1.28.0. Closing the feature request as done.

@valyala valyala closed this as completed Dec 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request TBD To Be Done
Projects
None yet
Development

No branches or pull requests

7 participants