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

Update FilterXByY() and ExcludeXByY() functions to return the number of excluded items #373

Closed
atc0005 opened this issue Oct 8, 2021 · 1 comment · Fixed by #376
Closed
Assignees
Labels
enhancement New feature or request output/logging output/perfdata Service Perf Data (aka, "performance data") refactor
Milestone

Comments

@atc0005
Copy link
Owner

atc0005 commented Oct 8, 2021

For example, the vsphere.FilterVMsByPowerState() function currently accepts a collection of mo.VirtualMachine values and a boolean flag indicating whether Virtual Machines in a powered off state should be evaluated. Then, a new collection is returned filtered based on that criteria.

log.Debug().Msg("Drop any VMs we've been asked to exclude from checks")
filteredVMs := vsphere.ExcludeVMsByName(vms, cfg.IgnoredVMs)
log.Debug().Msg("Filter VMs to specified power state")
filteredVMs = vsphere.FilterVMsByPowerState(filteredVMs, cfg.PoweredOff)

The function could be updated to return the number of excluded or filtered out items for use in performance data metrics and log messages.

@atc0005 atc0005 added enhancement New feature or request output/logging output/perfdata Service Perf Data (aka, "performance data") labels Oct 8, 2021
@atc0005 atc0005 added this to the Next Release milestone Oct 8, 2021
@atc0005 atc0005 self-assigned this Oct 8, 2021
@atc0005 atc0005 changed the title Update FilterXByY funcs to return the number of excluded items Update FilterXByY() and ExcludeXByY() funcs to return the number of excluded items Oct 8, 2021
@atc0005
Copy link
Owner Author

atc0005 commented Oct 8, 2021

Worth noting:

From earlier review, it looks like some of the FilterXByY() funcs are retrieving only one item from a collection. Updating those to return the number of items excluded doesn't feel like the right approach. Instead, perhaps those functions should be renamed to better reflect what we're looking for: one item back (or an error) and everything else ignored.

I believe I've seen functions of this form named RetrieveXByY, FindXByY or GetXByID. Will review the vmware/govmomi library for examples.

@atc0005 atc0005 changed the title Update FilterXByY() and ExcludeXByY() funcs to return the number of excluded items Update FilterXByY() and ExcludeXByY() functions to return the number of excluded items Oct 11, 2021
atc0005 added a commit that referenced this issue Oct 11, 2021
- Update `FilterXByY()` and `ExcludeXByY()` functions to return
  the number of excluded items

- Update applicable plugins and helper functions to use
  returned excluded counts (e.g., in detailed debug log messages)
  or to explicitly ignore the excluded count if not needed.

These changes are related to performance data metrics support
currently being added to plugins in this project.

refs GH-373
refs GH-354
atc0005 added a commit that referenced this issue Oct 12, 2021
- Update `FilterXByY()` and `ExcludeXByY()` functions to return
  the number of excluded items

- Update applicable plugins and helper functions to use
  returned excluded counts (e.g., in detailed debug log messages)
  or to explicitly ignore the excluded count if not needed.

These changes are related to performance data metrics support
currently being added to plugins in this project.

refs GH-373
refs GH-354
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request output/logging output/perfdata Service Perf Data (aka, "performance data") refactor
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant