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

[collectd 6] Generate better names and labels for legacy metrics. #4197

Merged
merged 7 commits into from
Dec 20, 2023

Conversation

octo
Copy link
Member

@octo octo commented Dec 18, 2023

Legacy metrics, i.e. metrics reported with the value_list_t data structure, need to be upgraded to metric_family_t.

The new code puts all metrics into the collectd.v5 namespace. The metric family name is:

  • collectd.v5.${plugin}.utilization for metrics with type "percent".
  • collectd.v5.${plugin}.io for directional metrics ending in _octets.
  • collectd.v5.${type}.${data_source} for metrics with more than one data source (except directional metrics).
  • collectd.v5.${type} for metrics with one data source.

If the hostname field is set, it is translated into the host.name resource attribute. Otherwise resource attributes are omitted to trigger the default behavior (like an empty hostname field does).

The labels depend on whether plugin instance, type instance, or both are set:

  • Both: ${plugin}=${plugin_instance}, type=${type_instance}
  • Plugin instance: ${plugin}=${plugin_instance}
  • Type instance: ${plugin}=${type_instance}
  • Neither: plugin=${plugin}

This does a decent job of mapping metrics to the new schema, but I do wonder if we should simply put those fields into labels 1-to-1: collectd.v5.plugin=${plugin}, collectd.v5.plugin_instance=${plugin_instance}, collectd.v5.type_instance=${type_instance}. What do you think?

Directional metrics are metrics with two data sources called "read" and "write", or "rx" and "tx". Despite having more than one data source, the data source name is added to a direction label instead of the metric family name.

ChangeLog: collectd: The schema with which metrics are translated from v5 to v6 has been improved.

@octo octo requested review from a team as code owners December 18, 2023 23:13
@collectd-bot collectd-bot added this to the 6.0 milestone Dec 18, 2023
@octo octo added this to In progress in collectd 6 via automation Dec 18, 2023
Copy link
Contributor

@eero-t eero-t left a comment

Choose a reason for hiding this comment

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

Code looks OK. Have just small optimization suggestion(s).

src/utils/value_list/value_list.c Outdated Show resolved Hide resolved
src/utils/value_list/value_list.c Outdated Show resolved Hide resolved
@eero-t
Copy link
Contributor

eero-t commented Dec 19, 2023

This does a decent job of mapping metrics to the new schema, but I do wonder if we should simply put those fields into labels 1-to-1: collectd.v5.plugin=${plugin}, collectd.v5.plugin_instance=${plugin_instance}, collectd.v5.type_instance=${type_instance}. What do you think?

No need for hierarchical labels, metric name being such is enough.

@octo octo merged commit 9f55b8d into collectd:collectd-6.0 Dec 20, 2023
21 of 25 checks passed
collectd 6 automation moved this from In progress to Done Dec 20, 2023
@eero-t
Copy link
Contributor

eero-t commented Dec 20, 2023

This breaks Prometheus integration. Dot is invalid in metric and label names. Double colon is also invalid for labels, but could be used as separator for metric names. See: https://prometheus.io/docs/concepts/data_model/#metric-names-and-labels

EDIT: that issue was worked around in Prometheus plugin.

@octo octo added the Feature label Jan 12, 2024
@octo octo added the core label Jan 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
collectd 6
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants