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

Metric name "_info" suffix gets trimmed #982

Open
iagotomas opened this issue Jun 29, 2024 · 11 comments
Open

Metric name "_info" suffix gets trimmed #982

iagotomas opened this issue Jun 29, 2024 · 11 comments
Assignees

Comments

@iagotomas
Copy link

Hi,
trying to implement a ruleset for some service, running jmx_exporter 1.0.1 as standalone I'm facing a weird issue which I'm not sure I understand.
I have the following rule for which the "_info" suffix in the name gets dropped. If I employ another suffix or add additional characters to the suffix "_info" it remains.

rules:
  #metrics:name=jvmInfo.runtime.{runtime}.vendor.{vendor}.version.{version},type=gauges
  - pattern: 'metrics<name=jvmInfo\.runtime\.(.+)\.vendor\.(.+)\.version\.(.*), type=gauges><>Value'
    labels:
      runtime: $1
      vendor: $2
      version: $3
    type: UNTYPED
    name: jvm_runtime_info
    help: "JVM info"

Resulting metric name:
jvm_runtime{runtime="OpenJDK_Runtime_Environment",vendor="Amazon.com_Inc.",version="17.0.7+7-LTS"} 1.0

Why does "_info" get removed from the name?

@dhoard dhoard self-assigned this Jul 1, 2024
@dhoard
Copy link
Collaborator

dhoard commented Jul 1, 2024

The relevant code is here...

PrometheusNaming.sanitizeMetricName(this.name),

@fstab, do you remember the specifics around why we sanitize the name?

@dhoard
Copy link
Collaborator

dhoard commented Oct 31, 2024

@fstab The code change was introduced as part of metric name collision work.

Do you remember the specifics?
Is it necessary?

@doxsch
Copy link
Contributor

doxsch commented Nov 11, 2024

We do have the same issue:

Beans:

kafka.schema.registry:type=avro-schemas-created
kafka.schema.registry:type=avro-schemas-deleted
kafka.schema.registry:type=json-schemas-created
kafka.schema.registry:type=json-schemas-deleted
kafka.schema.registry:type=protobuf-schemas-created
kafka.schema.registry:type=protobuf-schemas-deleted

Rule:

rules:
  - pattern: "kafka.schema.registry<type=(.+)-schemas-(.+)>([^:]+):"
    name: "kafka_schema_registry_schemas_$2"
    labels:
      schemaType: $1

returned metrics:

# HELP kafka_schema_registry_schemas Number of registered Avro schemas kafka.schema.registry:name=null,type=avro-schemas-created,attribute=avro-schemas-created
# TYPE kafka_schema_registry_schemas untyped
kafka_schema_registry_schemas{cluster="e1-kafka-pfnet-lab",schemaType="avro"} 0.0
kafka_schema_registry_schemas{cluster="e1-kafka-pfnet-lab",schemaType="json"} 0.0
kafka_schema_registry_schemas{cluster="e1-kafka-pfnet-lab",schemaType="protobuf"} 0.0

# HELP kafka_schema_registry_schemas_deleted Number of deleted Protobuf schemas kafka.schema.registry:name=null,type=protobuf-schemas-deleted,attribute=protobuf-schemas-deleted
# TYPE kafka_schema_registry_schemas_deleted untyped
kafka_schema_registry_schemas_deleted{cluster="e1-kafka-pfnet-lab",schemaType="avro"} 0.0
kafka_schema_registry_schemas_deleted{cluster="e1-kafka-pfnet-lab",schemaType="json"} 0.0
kafka_schema_registry_schemas_deleted{cluster="e1-kafka-pfnet-lab",schemaType="protobuf"} 0.0

The deleted metrics works but the created metric is trimmed.

@fstab
Copy link
Member

fstab commented Nov 11, 2024

Thanks for reporting this.

The reason is that the _info suffix is reserved for metrics of type INFO, but in the example the metric is

type: UNTYPED

Unfortunately I think type: INFO is not implemented in jmx_exporter. @dhoard would it be possible to add the INFO type? That would be the cleanest way to implement this.

@dhoard
Copy link
Collaborator

dhoard commented Dec 3, 2024

@fstab @iagotomas I will need to investigate the change after the pending release. The MatchedRule is used before any of the snapshot builds.

@doxsch Your issue is similar, but not easily solved since the metric is a counter, but the suffix _created is reserved for a value representing a timestamp.

@dhoard
Copy link
Collaborator

dhoard commented Dec 5, 2024

@fstab @zeitlinger Looking at the OpenMetrics specification...

Exposers SHOULD avoid names that could be confused with the suffixes that text format sample metric names use.

Suffixes for the respective types are:
Counter: '_total', '_created'
Summary: '_count', '_sum', '_created', '' (empty)
Histogram: '_count', '_sum', '_bucket', '_created'
GaugeHistogram: '_gcount', '_gsum', '_bucket'
Info: '_info'
Gauge: '' (empty)
StateSet: '' (empty)
Unknown: '' (empty)

Given the specifier SHOULD, the code to sanitize the name is overly strict...

PrometheusNaming.sanitizeMetricName(this.name),

The client_java library enforces the requirement, effectively changing the statement from SHOULD to MUST

I feel we need to investigate if removing the restriction in client_java is possible.

Thoughts?

@fstab
Copy link
Member

fstab commented Dec 5, 2024

This isn't easy, because there are many corner cases. For example, if a user registers a Gauge named my_os_info, and an Info metric named my_os, then there would be a name collision at scrape time. I think it's easier to prevent reserved suffixes at registration time so that users can be sure nothing bad can happen at scrape time.

@dhoard
Copy link
Collaborator

dhoard commented Dec 5, 2024

@doxsch can you create a new issue around the _created?

@fstab
Copy link
Member

fstab commented Dec 6, 2024

So, we just discussed this in the Prometheus Java community call, and we decided that we should support _info and _created suffixes. We'll work on a way to enable these.

@offermannu
Copy link

We have also encountered this issue and it has broken some of our Grafana dashboards. From our point of view, this is an incompatible change as it still worked with version 0.17.2. We cannot change the dashboards as otherwise the historical data would also be gone.

@dhoard
Copy link
Collaborator

dhoard commented Dec 15, 2024

@offermannu We are discussing possible changes.

For your rules, what metric names and types (GAUGE, COUNTER or UNTYPED) are you using that are having issues?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants