Does it make sense to add a units attribute to <metric/>?

[rmohr]

I think it would be great if vhostmd reports the units of the values which it report.

I could think about the following, when vhostmd runs with the default vhostmd.conf on fedora:

<metrics>
  <metric type="string" context="host">
    <name>HostName</name>
    <value>node01</value>
  </metric>
  <metric type="string" context="host">
    <name>HostSystemInfo</name>
    <value>linux</value>
  </metric>
  <metric type="real64" context="vm" unit="s">
    <name>TotalCPUTime</name>
    <value>98.820000</value>
  </metric>
  <metric type="uint64" context="vm" unit="KiB">
    <name>PhysicalMemoryAllocatedToVirtualSystem</name>
    <value>1000000</value>
  </metric>
  <metric type="uint64" context="vm" unit="KiB">
    <name>ResourceMemoryLimit</name>
    <value>1000000</value>
  </metric>
  <metric type="int64" context="host">
    <name>NumberOfPhysicalCPUs</name>
    <value>6</value>
  </metric>
  <metric type="real64" context="host" unit="s">
    <name>TotalCPUTime</name>
    <value>26779.730000</value>
  </metric>
  <metric type="uint64" context="host" unit="KiB">
    <name>FreePhysicalMemory</name>
    <value>131700</value>
  </metric>
  <metric type="uint64" context="host" unit="KiB">
    <name>FreeVirtualMemory</name>
    <value>131700</value>
  </metric>
  <metric type="uint64" context="host" unit="KiB">
    <name>AllocatedToVirtualServers</name>
    <value>3745584</value>
  </metric>
  <metric type="uint64" context="host" unit="KiB">
    <name>UsedVirtualMemory</name>
    <value>3745584</value>
  </metric>
  <metric type="uint64" context="host" unit="KiB">
    <name>PagedInMemory</name>
    <value>0</value>
  </metric>
  <metric type="uint64" context="host" unit="KiB">
    <name>PagedOutMemory</name>
    <value>0</value>
  </metric>
  <metric type="int64" context="host" unit="s">
    <name>Time</name>
    <value>1619002945</value>
  </metric>
  <metric type="string" context="host">
    <name>VirtualizationVendor</name>
    <value>kubevirt.io</value>
  </metric>
</metrics>

vm-dump-metrics does not seem to care about the exact xml format, so should be backwardcompatibel and much clearer. What do you think?

[fabiand]

This would be a valuable addition.

[jfehlig]

Every year or two (or more) there is a vhostmd-related query that reminds me the project still exits :-). I agree adding a units attribute is a useful addition, and also agree it would be a backward compatible change.

Do you have time/energy/desire to implement that? If not, no problem. I can get to it next week.

[rmohr]

Every year or two (or more) there is a vhostmd-related query that reminds me the project still exits :-). I agree adding a units attribute is a useful addition, and also agree it would be a backward compatible change.

I guess it just does its job well within the defined scope :)

Do you have time/energy/desire to implement that? If not, no problem. I can get to it next week.

Would be awesome if you could do that. I am implementing host metrics exposure in kubevirt/kubevirt#5502 for kubevirt. For architectural reasons we can not use vhostmd, but it would be great if I can just add unit there and know that it is still in sync with vhostmd :)

[jfehlig]

Sorry for the delay. I finally found time to work on this issue and thought it would be pretty easy, but unfortunately not the case. The problem is, vhostmd may not even know the unit. E.g. consider the TotalPhyMem metric in the upstream configuration

https://github.com/vhostmd/vhostmd/blob/master/vhostmd.xml#L80

In this case we know the unit would be MiB since libvirt reports in KiB and we divide by 1024. But the user could overwrite the action with their own action that produces TotalPhyMem in bytes. And custom actions, like many in your fedora example, would produce values with units unknown to vhostmd. We could add a units attribute to the definition, but it could only be optional and without it we'd be in the same boat.

That said, I'm pretty rusty with vhostmd so might be overlooking something and am open to suggestions.

[rmohr]

In this case we know the unit would be MiB since libvirt reports in KiB and we divide by 1024. But the user could overwrite the action with their own action that produces TotalPhyMem in bytes. And custom actions, like many in your fedora example, would produce values with units unknown to vhostmd. We could add a units attribute to the definition, but it could only be optional and without it we'd be in the same boat.

Yes, that would have been my assumption that the person which writes the vhostmd.xml can optionally specify the resulting unit.
The person which writes vhostmd.xml has far more knowledge about the unit than the person querying the metrics from inside a guest I would suppose.

[jfehlig]

Perhaps you have a few minutes to take a peek at the above PR? Thanks!

BTW, looking at that old code makes my eyes hurt. It is in bad need of modernization, but I'm surprised the project is still alive and has users :-).