Add system.cpu.frequency metric.

[ChrsMark]

Fixes #130

Changes

This PR adds the remaining of the CPUINFO data described at #130. The respective CPU frequency is added as a metric, called system.cpu.frequency.max based on open-telemetry/opentelemetry-collector-contrib#26532 (comment). See also #337 (comment) for more details.

Example:

On a Linux machine execute the following:

cat /proc/cpuinfo | grep MHz
cpu MHz		: 2252.777
cpu MHz		: 3000.000
cpu MHz		: 1217.024
cpu MHz		: 3000.000
cpu MHz		: 3000.000
cpu MHz		: 3000.000
cpu MHz		: 3000.000
cpu MHz		: 3000.000

As we can see the provided frequencies are per CPU and hence we distinguish this by using the system.cpu.logical_number metric attribute.

Collector's implementation PR: open-telemetry/opentelemetry-collector-contrib#27445

Merge requirement checklist

• CONTRIBUTING.md guidelines followed.
• CHANGELOG.md updated for non-trivial changes.
• schema-next.yaml updated with changes to existing conventions.

[frzifus]

Is here something missing?

[mx-psi]

Is there something missing?

I think we need to discuss whether this definition is practical. The library we use on the hostmetrics receiver does not support getting the real-time frequency, only the maximum frequency. See open-telemetry/opentelemetry-collector-contrib#26532 (comment)

I have not had time to check whether an alternative exists (the problem seems to be Windows).

Let's figure out the practicality of implementation first

[ChrsMark]

Hey folks! So since it seems that its not quite straight-forward to collect the current cpu's frequency in Windows maybe we can introduce the system.cpu.frequency.max instead (based on https://github.com/shirou/gopsutil/blob/2fabf15a16dca198f735a5de2722158576e986a9/cpu/cpu_linux.go#L145-L148).

This would give us a meaningful attribute for now which comes from cpufreq/cpuinfo_max_freq in Linux and is also available for other OS distros, but at the same time gives us the flexibility to add system.cpu.frequency.min and/or system.cpu.frequency.current in the future if we find the need.

@mx-psi @frzifus what do you think?

[mx-psi]

@mx-psi @frzifus what do you think?

Makes sense to me. I think the name should end in .limit instead based on https://github.com/open-telemetry/semantic-conventions/blob/main/docs/general/metrics.md#instrument-naming ?

[ChrsMark]

@mx-psi @frzifus what do you think?

Makes sense to me. I think the name should end in .limit instead based on https://github.com/open-telemetry/semantic-conventions/blob/main/docs/general/metrics.md#instrument-naming ?

That would be fine but not sure how we would potentially refer to the min's equivalent?
BTW I got the idea from the already existent DB metrics -> https://github.com/open-telemetry/semantic-conventions/blob/main/model/metrics/database-metrics.yaml#L37-L53

[mx-psi]

That would be fine but not sure how we would potentially refer to the min's equivalent?

Right... and taken literally the max frequency is not the 'total amount' of frequency in any sense.

BTW I got the idea from the already existent DB metrics -> https://github.com/open-telemetry/semantic-conventions/blob/main/model/metrics/database-metrics.yaml#L37-L53

Ok then I would go with .max 👍

[frzifus]

@mx-psi @frzifus what do you think?

Makes sense to me. I think the name should end in .limit instead based on https://github.com/open-telemetry/semantic-conventions/blob/main/docs/general/metrics.md#instrument-naming ?

That would be fine but not sure how we would potentially refer to the min's equivalent? BTW I got the idea from the already existent DB metrics -> https://github.com/open-telemetry/semantic-conventions/blob/main/model/metrics/database-metrics.yaml#L37-L53

Ive no strong preference. But from the outside I think min & max is more expressive.

an instrument that measures the constant, known total amount of something should be called entity.limit. For example, system.memory.limit for the total amount of memory on a system.

Can there be a limit on the frequency which is lower then max? Or is there no way to recognize this and a limit would always be max?

[ChrsMark]

Can there be a limit on the frequency which is lower then max? Or is there no way to recognize this and a limit would always be max?

So based on https://www.kernel.org/doc/Documentation/cpu-freq/user-guide.txt cpuinfo_max_freq is the maximum operating frequency the processor can run at while scaling_max_freq is the current "policy limits". By echoing new values into these files, you can change these limits.

So from what I can understand scaling_max_freq is the mutable limit that can be set while cpuinfo_max_freq is the actual hardware limit the vendor defines.

If that's correct then maybe this is not a metric but more a host.cpu resource attribute?

Update:
I had a look into Prometheus Node exporter (as a reference) and I see that both cpuinfo and scaling frequencies are provided:

1. https://github.com/prometheus/node_exporter/blob/8ff3c439221f1906ee9b0b2362e490014286c7e7/collector/cpufreq_common.go#L35
2. https://github.com/prometheus/node_exporter/blob/8ff3c439221f1906ee9b0b2362e490014286c7e7/collector/cpufreq_common.go#L50

Based on this I would suggest using system.cpu.frequency.max for now and if needed in the future we can add system.cpu.frequency.scaling.* attributes as well.

[ChrsMark]

Hey @frzifus @mx-psi ! The collector's implementation PR is open for review: open-telemetry/opentelemetry-collector-contrib#27445.

It seems that we can add support for reporting the current cpu in Linux systems and provide the option for adding support for more platforms in the future accordingly. Hence I have updated this PR to introduce the system.cpu.frequnecy (current frequnecy) so it's back to its original state. Let me know what you think.

[mx-psi]

From the WG meeting: how does this metric compare with https://github.com/open-telemetry/semantic-conventions/blob/main/docs/system/hardware-metrics.md#hwcpu---physical-processor-metrics ? In particular:

• Are they the same? If they are the same, we should have one of them (I would say this one). If they are different, we should clarify how they are different.
• Should we use Hz instead of MHz for this metric?

For reference, the metrics do not seem to be used https://github.com/search?q=org%3Aopen-telemetry%20hw.cpu&type=code. cc @arminru as the original author of the document in case you have more context.

[ChrsMark]

Thanks for summarizing this @mx-psi!

One problem I see with hw.cpu.speed is that we cannot distinguish it per cpu.logical_number. The current frequencies provided from /proc/cpuinfo are different per processor and this means that if we would decide to use hw.cpu.speed we would need to attach a metric attribute as well to distinguish from which processor the metric is coming from.
Right now we have system.cpu.logical_number which corresponds to this but I'm not sure if we could use it with hw.cpu.speed 🤔 . If we can use it then I think we are fine to go for hw.cpu.speed.
Alternatively since hw.cpu.speed is not used at the moment what would be our chances to deprecate it and use system.cpu.frequency one? @arminru your feedback would help here since you introduced the field back then :).

[arminru]

Hey @mx-psi @ChrsMark! I reviewed the PR back then and suggested changes which were applied and thus show up in the co-authored-by field but the original proposal came from @bertysentry, so I'll forward the questions to him instead.

[ChrsMark]

@bertysentry do you have any content to share here?

Would that make sense to add system.cpu.logical_number attribute to the hw.cpu.speed and use it instead of introducing the system.cpu.frequency?
Or should we introduce the system.cpu.frequency since we are stabilizing the system.* namespace and deprecate the hw.cpu.speed one? To my mind it's cleaner to have system.cpu.frequency along with the system.cpu.logical_number attribute.

@open-telemetry/semconv-system-approvers thoughts on this?

[ChrsMark]

From the WG meeting(Oct 18th):

1. We are aligned that system.cpu.frequency should be introduced since it refers to logical cores as well and using just the hw.cpu.speed would be limiting. Also we need to re-use the system.cpu.logical_number in any case.
2. As a base frequency Hz should be used.

I will update the PR accordingly.

[bertysentry]

Sorry for the late reply (I'm on vacation!)

It is true that hw.cpu.speed.limit has been defined for the entire CPU package and doesn't specify a logical processor number, which is a limitation and needs to be fixed (I will take care of this: hw.cpu. metrics should represent individual cores, when possible).

WRT the deprecation of hw.cpu.speed.*, I think we should keep both system.cpu.frequency and this one, because logical processors seen by the operating system are not always the same as the real processors. Think of hyper-threading, and virtual cores allocated to virtualization systems (VMware, qemu, etc.).

NB: The challenge with processors is that the technology evolves quite rapidly, and some metrics that sound logical today will be meaningless in a few years. Example: Intel's RAPL.