Closed jspaleta closed 1 year ago
This looks good, thanks @jspaleta 👍
Just a few comments:
State should be check-based, not plugin-based. Check names must be unique (even if they share a given asset/plugin), and agent already guards against multiple concurrent executions of a given check. This should eliminate the need for locking / guarding against concurrent reads/writes.
- "Must assume the same plugin executable may be run with different settings on the same agent as different checks or check hooks, and each will need a different state file."
- State file is a single access at a time.
It would be good to standardize on JSON for all statefiles
- State file contents must be able to represent a json-like golang struct with nested maps and slices.
Should this be a sensu/sensu-go issue? We already have a type for this which already has JSON serialization/deserialization support: the Sensu Event. What if the agent just kept a copy of the previous event payload (in memory, or written to an actual statefile), and then made the previous event state available in a deterministic way (e.g. set a SENSU_PREVIOUS_EVENT
environment variable with the event contents, or a SENSU_PREVIOUS_EVENT_STATEFILE
path to the statefile)? This could provide access to the previous event entity configuration, labels, annotations, and even metrics.
I'm not sure I'm happy replacing sensu-check-log's state file with a sensu event and encoding all the file path names as arbitrarily long annotation keys and all the integer type seek offsets as strings values in the annotations. Having to encode everything as annotations means doing more work to type cast them back into the correct types.
For log check, being able to have an arbitrary golang struct instead of Sensu event is probably the better fit because the state needed there is never exposed in a sensu event.
So could you encode a sensu event as the state contnet in this implemtnation.. sure. Would it make sense to default to that in the template plugin repos that other plugins are derived from ? Sure. But I do not think Sensu event as structured right now is expressive enough for what log check functionality needs. If the SDK only implemented sensu event as state, I don't think check log would be able to use it.
I think I have to -1 this one? This is scope overreach and won't save much code in SDK consumers, if they even want the semantics we prescribe.
Rationale
Some check patterns require a small amount of state for correct operation. Lets bake that functionality into the SDK.
Check State Use Cases
Implementation requirements derived from current sensu-log-check implementation
Operational requirements/assumptions
Basic functionality needed
Advanced Functionality
If possible, implement state file with client access locking aware so that check can hold a lock on the file while the check is operating preventing a second check from access the file. This will prevent operators from accidentally using the same state file for multiple independently running checks or check hooks.
Hypothetical plugin check operation with locking