Closed mylesw42 closed 4 years ago
I think this rewrite is much cleaner than the first attempt. It piggy backs off the native event.Metric types. It also now works together with checks that actually have metrics, meaning these checks will also have their status captured as a measurement.
@mylesw42 take a look at https://github.com/sensu/sensu-wavefront-handler for some examples on how to implement Github Actions and modules!
Oh! I didn't notice your last comment @nikkixdev. All the while I was just figuring out golangci-lint, heh. Okay I'll look a bit more in to GitHub stuff with the example you provided. Thanks.
Hmm, I have mixed feelings about this. I think that moving this code to a mutator would be a more correct approach than tying it to this specific handler implementation. On the other hand, that would be less efficient and more complex for users to set up.
@flowerysong If you take a look at the community sensu-go-graphite-handler, you can see it has a similar behavior in the handler, adding metrics for the check state and status. I do see your point that some logic intended for mutators can tend to leak over into handlers. However I think for the purpose of TSDB-specific implementations, that's alright because sometimes the metrics have to be special cased for each TSDB.
@nikkixdev Right, but the fact that other handlers want to do the same thing is kind of my point. By doing the transformation from check to metrics in a mutator you theoretically only have to implement it once and then all of the handlers that consume the Sensu Metric Format can support this use case without n slowly diverging implementations.
@flowerysong I guess I like the simplicity of flipping the "-c" switch to enable this measurement. Running a mutator process just to capture the status, and then run a handler again for every check feels a bit heavy. @nikkixdev - So I was going to fit in golangci-lint in the Github actions, but it was only supported on Linux (first test failed). Then I noticed this repo is actually integrated with GoLangCI, so I ended up copying the wavefront config.
@nikkixdev I think that should be about it. The last commit actually closes off pr-32.
@nikkictl / @mylesw42 - Any change you guys have an example of how to use this for events ?
A first stab at implementing this feature in issue-27
I've tested locally for InfluxDB successfully.
I updated the example config, and usage also. This feature is intended to be used "stand-alone" from the existing metric processing workflow. Ideally, a new handler would be setup dedicated for collecting the check status results.