feat(xo-server-perf-alert): don't compute RAM usage when no tools

[MelissaFrncJrg]

See #7831

Screenshot

Fixes first part of https://kanban.vates.fr/b/jnfjuip4eBARBNuv9/xo-releases/jHtPoS4TKqZB7QL7Z

Description

No memory usage is collected without the guest tools. That means that the selected VMs won't be monitored by the xo-server-perf-alert plugin

Checklist

• Commit
  • Title follows commit conventions
  • Reference the relevant issue (Fixes #007, See xoa-support#42, See https://...)
  • If bug fix, add Introduced by
• Changelog
  • If visible by XOA users, add changelog entry
  • Update "Packages to release" in CHANGELOG.unreleased.md
• PR
  • If UI changes, add screenshots
  • If not finished or not tested, open as Draft

[b-Nollet]

I don't know if you agree with me @MathieuRA but I feel like it would be more clear to use the return statement of a function instead of defining this maybeUpdateListItem function, which essentially does "return value if no value was returned before" (if I understood well). It would also prevent some code to execute when it is not useful.

It could be something like this : (I did not test it)

const getListItem = () => {

  if (result.value === undefined) {
    return "**Can't read performance counters**"
  }

  if (lcObjectType === 'vm' && definition.variableName === 'memoryUsage') {
    const checkManagementAgent = (vm, guestMetrics) => {
      if ((vm.power_state !== 'Running' && vm.power_state !== 'Paused') || guestMetrics === undefined) {
        return
      }

      const { major, minor } = guestMetrics.PV_drivers_version
      const hasPvVersion = major !== undefined && minor !== undefined
      return hasPvVersion || guestMetrics.other['feature-static-ip-setting'] === '1'
    }

    const vm = result.object
    const guestMetrics = this._xo.getXapi(uuid).getObject(vm.guest_metrics)

    const managementAgentDetected = checkManagementAgent(vm, guestMetrics)

    if (managementAgentDetected === undefined) {
      return "**Can't read performance counters**"
    }
    if (managementAgentDetected === false) {
      return '**Guest tools must be installed**'
    }
  }

  return result.value?.toFixed(1) + typeFunction.unit

}

result.listItem = `  * ${result.objectLink}: ${getListItem()}\n`

[MelissaFrncJrg]

@b-Nollet actually maybeUpdateListItem manages the link with ${result.objectLink} so imo it's better to keep is as it is ?
I tested with

if (managementAgentDetected === undefined) {
                    listItem = `  * ${result.objectLink}: **Can't read performance counters**\n`
                  }

and it seems to work but isn't Mathieu's version more readable ?

[b-Nollet]

@b-Nollet actually maybeUpdateListItem manages the link with ${result.objectLink} so imo it's better to keep is as it is ? I tested with

if (managementAgentDetected === undefined) {
                    listItem = `  * ${result.objectLink}: **Can't read performance counters**\n`
                  }

and it seems to work but isn't Mathieu's version more readable ?

I may be misunderstanding something, but as result.objectLink is not modified or created in this part of the code, I don't think we particularly need to use it before assigning the value of result.listItem

[MathieuRA]

@b-Nollet Yes, I think it would be better.

[pdonias]

You can also link to the other PR so we can easily edit both sides if needed.