atc0005 / todo

A collection of TODO items not specific to any one project
MIT License
0 stars 0 forks source link

Surface `plugin.AddPerfData()` error, exit with UNKNOWN exit state #53

Closed atc0005 closed 1 year ago

atc0005 commented 1 year ago

Overview

Example of code block where the issue is not surfaced:

    pd := getPerfData(allAssertions, fileAssertions, registryAssertions)
    if err := plugin.AddPerfData(false, pd...); err != nil {
        log.Error().
            Err(err).
            Msg("failed to add performance data")
    }

Until now the approach has been just to log the error and continue.

At a minimum I believe that I should surface the error by adding to the error collection; an error "collection" is a relatively new feature of the nagios plugin library, so if I had to guess (without checking my memory) I'd say that I opted to log the error vs collecting it as another error (earlier or later) might be more significant and not something that could be skipped.

Example of the modified code block:

    pd := getPerfData(allAssertions, fileAssertions, registryAssertions)
    if err := plugin.AddPerfData(false, pd...); err != nil {
        log.Error().
            Err(err).
            Msg("failed to add performance data")

        // Surface the error in plugin output.
        plugin.AddError(err)

        // Abort plugin execution with UNKNOWN status. This should not fail,
        // but if it does, then it's *very* likely a logic bug that should be
        // surfaced.
        plugin.ExitStatusCode = nagios.StateUNKNOWNExitCode
        plugin.ServiceOutput = fmt.Sprintf(
            "%s: Failed to process performance data metrics",
            nagios.StateUNKNOWNLabel,
        )

        return
    }

TODO

Review these projects and apply changes where applicable:

atc0005 commented 1 year ago

Example diff from the check-cert project:

diff --git a/cmd/check_cert/main.go b/cmd/check_cert/main.go
index b119daf..fa13b79 100644
--- a/cmd/check_cert/main.go
+++ b/cmd/check_cert/main.go
@@ -391,7 +391,13 @@ func main() {
        // Surface the error in plugin output.
        plugin.AddError(perfDataErr)

-       // TODO: Abort plugin execution with UNKNOWN status?
+       plugin.ExitStatusCode = nagios.StateUNKNOWNExitCode
+       plugin.ServiceOutput = fmt.Sprintf(
+           "%s: Failed to generate performance data metrics",
+           nagios.StateUNKNOWNLabel,
+       )
+
+       return
    }

    if err := plugin.AddPerfData(false, pd...); err != nil {
@@ -402,7 +408,11 @@ func main() {
        // Surface the error in plugin output.
        plugin.AddError(err)

-       // TODO: Abort plugin execution with UNKNOWN status?
+       plugin.ExitStatusCode = nagios.StateUNKNOWNExitCode
+       plugin.ServiceOutput = fmt.Sprintf(
+           "%s: Failed to process performance data metrics",
+           nagios.StateUNKNOWNLabel,
+       )
    }

    switch {
atc0005 commented 1 year ago

NOTE: Checking off TODO items if project doesn't currently emit performance data.