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] Scale time metrics to seconds per second using floating point counters. #4272

Merged
merged 3 commits into from
Feb 6, 2024

Conversation

octo
Copy link
Member

@octo octo commented Feb 3, 2024

This converts the following metrics to seconds per second, in order to align with OpenTelemetry Semantic Conventions:

  • system.cpu.time (link)
  • system.disk.operation_time (link)
  • system.disk.io_time (link)
  • system.disk.weighted_io_time

The CPU plugin relies on rate_to_value() and value_to_rate(), which were lacking support for floating point counters still. Apparently I missed them in #4266. This PR fixes them.

ChangeLog: CPU and Disk plugins: Time metrics have been changed to report seconds per second.

… to `rate_to_value()` and `value_to_rate()`.
@octo octo added the Feature label Feb 3, 2024
@octo octo requested review from a team as code owners February 3, 2024 21:08
@collectd-bot collectd-bot added this to the 6.0 milestone Feb 3, 2024
@octo octo merged commit 67633b8 into collectd:collectd-6.0 Feb 6, 2024
27 checks passed
@octo octo deleted the 6/time_metrics branch February 6, 2024 08:11
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.

Looks nice, improves the plugin code.

&fam_ops_time, direction_label, write_direction,
(value_t){.derive = (derive_t)(write_time_s * 1000000.0)}, &m);
metric_family_append(&fam_ops_time, direction_label, read_direction,
(value_t){.fpcounter = (fpcounter_t)read_time_s},
Copy link
Contributor

Choose a reason for hiding this comment

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

Macro could have made lines like these nicer.

Comment on lines +524 to +533
.v0 = {.counter = 4294967238ULL},
.v1 = {.counter = 42},
.want = 10.0,
},
{
.name = "counter_t 64bit wrap-around",
.t0 = 30,
.t1 = 40,
.type = METRIC_TYPE_COUNTER,
.v0 = {.counter = 18446744073709551558ULL},
Copy link
Contributor

Choose a reason for hiding this comment

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

Wrap-around initial values would IMHO be clearer as 0xffffffff-57 and 0xffffffffffffffff-57.

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

Successfully merging this pull request may close these issues.

None yet

3 participants