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

Add initial support for collecting and formatting performance data #81

Closed
atc0005 opened this issue Sep 4, 2021 · 8 comments
Closed
Assignees
Labels
documentation Improvements or additions to documentation enhancement New feature or request performance data
Milestone

Comments

@atc0005
Copy link
Owner

atc0005 commented Sep 4, 2021

Extend current type to allow collecting performance data and returning it to the Nagios console.

One or more new fields, one or more new methods.

Providing and emitting performance data should remain optional.

@atc0005 atc0005 added documentation Improvements or additions to documentation enhancement New feature or request performance data labels Sep 4, 2021
@atc0005 atc0005 added this to the Next Release milestone Sep 4, 2021
@atc0005 atc0005 self-assigned this Sep 4, 2021
@atc0005 atc0005 modified the milestones: v0.7.0, v0.8.0 Sep 7, 2021
@atc0005
Copy link
Owner Author

atc0005 commented Sep 7, 2021

Extend current type to allow collecting performance data and returning it to the Nagios console.

Looking at the https://assets.nagios.com/downloads/nagioscore/docs/nagioscore/3/en/pluginapi.html coverage for "PERFDATA", we have the option of providing the formatted performance data on the same line as the initial first line of output (ServiceOutput), at the end of the output (LongServiceOutput) or both.

Emitting the output at the very end may be the simplest approach, but one potential problem is that if the LongServiceOutput content is too long it could result in the PerfData content being lost.

Perhaps we could stub in basic length checks to help determine where the PerfData output is emitted.

It's worth noting that I've been occasionally reviewing the one-line ServiceOutput text via CLI execution as one of my iterative testing steps prior to deploying a new service check. If we mix in the PerfData on the initial line it will "add noise" to the brief output.

@atc0005
Copy link
Owner Author

atc0005 commented Sep 7, 2021

I've made some light progress on this, but still have a lot of work left to do. I'm currently going over the https://nagios-plugins.org/doc/guidelines.html#AEN200 doc and trying to get a sense of how to handle validation for the specific values that makes up a PerfData entry.

@atc0005
Copy link
Owner Author

atc0005 commented Sep 10, 2021

Stubbed out some proof of concept scratch work. Hope to implement soon.

@atc0005
Copy link
Owner Author

atc0005 commented Sep 10, 2021

Thought: function chaining. The first method returns a type that has a method which takes the second piece of data and so on. The net result is that we build the value we want in pieces.

This was clearer in my mind before I was woke up, so I am sure I am leaving something out.

atc0005 added a commit that referenced this issue Sep 14, 2021
- Add `PerformanceData` type to represent a single, complete
  Performance Data entry

- Extend `ExitState` type to hold Performance Data entries

- Modify `ExitState.ReturnCheckResults()` method to process
  Performance Data entries if present

- Add `ExitState.AddPerfData()` method with optional flag
  to skip validation
  - intended to allow client code full control of validation

This is a first draft and has not undergone sufficient testing
to be considered stable.

refs GH-81
atc0005 added a commit that referenced this issue Sep 14, 2021
- Add `PerformanceData` type to represent a single, complete
  Performance Data entry

- Extend `ExitState` type to hold Performance Data entries

- Modify `ExitState.ReturnCheckResults()` method to process
  Performance Data entries if present

- Add `ExitState.AddPerfData()` method with optional flag
  to skip validation
  - intended to allow client code full control of validation

This is a first draft and has not undergone sufficient testing
to be considered stable.

Documentation updates have not been made yet to illustrate
use of the new functionality.

refs GH-81
atc0005 added a commit that referenced this issue Sep 14, 2021
- Add `PerformanceData` type to represent a single, complete
  Performance Data entry

- Extend `ExitState` type to hold Performance Data entries

- Modify `ExitState.ReturnCheckResults()` method to process
  Performance Data entries if present

- Add `ExitState.AddPerfData()` method with optional flag
  to skip validation
  - intended to allow client code full control of validation

This is a first draft and has not undergone sufficient testing
to be considered stable.

Documentation updates have not been made yet to illustrate
use of the new functionality.

refs GH-81
@atc0005
Copy link
Owner Author

atc0005 commented Sep 14, 2021

TODO: Update documentation to reflect changes from GH-87.

atc0005 added a commit to atc0005/check-vmware that referenced this issue Sep 15, 2021
Add initial, prototype support for Performance Data
output to the `check_vmware_datastore` plugin.

This commit relies on untagged support in the
atc0005/go-nagios project for handling
Performance Data. The state of that initial implementation
and the changes provided by this commit are still subject
to change based on testing/feedback.

refs GH-321
refs atc0005/go-nagios#81
refs atc0005/go-nagios@1db1acf
@atc0005
Copy link
Owner Author

atc0005 commented Sep 16, 2021

Looks like the label naming practice is to use underscores. See atc0005/check-vmware#321 (comment) and previous for additional notes.

I'll extend the doc comments for the Label field to help make that clear.

atc0005 added a commit that referenced this issue Sep 16, 2021
Stress using underscores to match community naming convention.

refs GH-81
atc0005 added a commit that referenced this issue Sep 16, 2021
- Fix skipValidation flag handling behavior
- Skip appending any performance data if a validation failure
  occurs
- Update doc comments to make expected behavior clear

refs GH-81
atc0005 added a commit that referenced this issue Sep 29, 2021
Update format for emitted performance data metrics to use single space
as separator instead of newline.

While the multi-line format used previously is recognized and
supported by Nagios, Icinga did not interpret the performance data in
the same way. Testing confirms that this updated format works with
both monitoring platforms.

This commit also updates doc comments and ensures that a trailing
newline is retained as the final line in generated output to satisfy
the Nagios plugin output format.

- refs #81
- refs atc0005/check-vmware#321
atc0005 added a commit that referenced this issue Sep 29, 2021
- Add minor mention/coverage of Performance Data support, link to
  current GitHub issue for implementation and "next step" issue for
  better documentation updates to reflect a (more) stable form of the
  support

- Add additional reference links to better note what sources were used
  to define the Performance Data implementation

- refs #81
- refs #92
- refs atc0005/check-vmware#321
@atc0005
Copy link
Owner Author

atc0005 commented Sep 30, 2021

See atc0005/check-vmware#321 for recent discussion and troubleshooting of the performance data output format. As of commit bb945df, initial support is in place and ready for further use/testing. #92 is intended to refresh the documentation (such as it is) to cover the new support for emitting performance data.

The hope is that after some further use the better way forward will be clearer.

For example, here is a mockup of what method chaining might look like:

	nagiosExitState.
		PerformanceData().
		Label().
		Value().
		UnitOfMeasurement().
		Warn().
		Crit().
		Add()

	nagiosExitState.
		PerformanceData().
		Label().
		Value().
		UnitOfMeasurement().
		Max().
		Min().
		Add()

Ultimately I'm going to need to give this some time/thought before making further changes.

@atc0005 atc0005 closed this as completed Sep 30, 2021
@atc0005 atc0005 changed the title Add support for collecting and formatting performance data Add initial support for collecting and formatting performance data Sep 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request performance data
Projects
None yet
Development

No branches or pull requests

1 participant