Refactor Check Type (configuration and execution) about sensu-go HOT 6 CLOSED

OK.

I'm at an impasse and a little stuck on eventd, and I took a step back today, because I was trying to wrap up a discussion that @portertech and I had regarding Check, CheckConfig, CheckExectuion, and the /checks API. I'd like to document that in preparation for a general discussion about specifying some API behavior and getting myself back on track for working on the event flow in sensu-backend.

Problem

The /checks API endpoint allows for a simple configuration interface with a traditional CRUD API for a "Check" object, which is a specification of a check to be run by Sensu. I.e. we will refer to a Check Configuration as an object with the fields (Name, Interval, Subscriptions, Command, Handlers). The /checks API endpoint should only have a view of Checks that includes the Check Configuration fields.

The /events API endpoint, is a record of check executions and observations made about that execution including the fields (Output, Status, Issued, Executed, Duration). It is undesirable for the /checks endpoint to know about these fields, as it may lead to confusing UX if upon inspection of the objects returned by the /checks endpoint, users observe empty fields with those names. Furthermore, the /events API results include a memoization of check executions that have the fields (Executed, Status) for use in event normalization / flap detection.

How do we manage this data in the backend, and how do we represent it programmatically in a way that makes sense both semantically and programmatically?

from sensu-go.

Approach 1:

Struct Composition

I was previously working under the assumption that anonymously embedding a struct with set of fields A inside a struct with a set of fields B yielded a struct (B) with a set of fields A U B that would naturally behave like a struct with a union of those fields.

type A struct {
  FieldA int
}

type B struct {
  A
  FieldB int
}

This is only partially true. Embedding a struct in this manner will allow you to access those fields directly after initialization, but you cannot assign values to those structs directly during initialization. I.e. Initialization of embedded structs is really annoying.

b := B{
  FieldA: 1,
  FieldB: 2,
}

Will yield a compiler error unknown field 'FieldA' in struct literal of type B. To me, this isn't ideal. The resulting developer experience is kind of bad. This works:

	b := B{}
	b.FieldA = 1

Maybe this isn't a big deal. Do we just live with this? Or is there a better way.

from sensu-go.

Approach 2:

CheckConfiguration and CheckExecution

After getting really annoyed by the gauche UX of struct composition, I started thinking about API behavior, and that maybe the underlying implementation should reflect that, but then I thought that this was maybe a bad idea. Concretely:

If we divorce the two things entirely, we get two objects:

• CheckConfiguration: Name, Interval, Subscriptions, Command, Handlers
• CheckExecution: Output, Status, Issued, Executed, Duration

The immediate problem I have with this approach is that we may want /events to reflect the configuration at the time of the execution. That would require a resultant CheckExecution object to have knowledge of its initiating CheckConfiguration. This isn't strictly necessary, as our previously defined notion of Check History does not include the CheckConfiguration.

Aside:

This is something that could be implemented without storing the whole of the CheckConfiguration object alongside the CheckExecution, however. One would only need the unique identifier (i.e. Name) and a revision (assuming versioned objects, which are currently available because of Etcd--however this would require managing those stored revisions).

In general, I like this approach, but it isn't in the spirit of the Sensu 1 API which bucketed much of this into a single object (as does our current implementation).

from sensu-go.

Approach 3:

Views

Let's look at this from a different perspective.

Right now, we're defining objects in sensu-backend based on multiple sets of requirements.

1. The behavior of the /checks API endpoint
2. The behavior of the /events API endpoint.
3. The Event object

I currently consider the "Agent API" to be defined as "receive over a WebSocket an Event JSON hash with a Check hash field populated with enough information to execute a check, return a copy of this exact event object with execution fields populated".

We will call that Event JSON hash the Event object.

The /checks API endpoint currently deals only accepts as input the fields associated with CheckConfiguration. This is desirable. However, it also currently returns all of the other fields associated with the super Check object included in Events.

The /events API endpoint currently deals with the Agent API Event objects with the Super Check object that includes an array of check execution histories as previously defined.

There's no uniformity here, but all of these 3 things, in a sense, deal with a Super Check that is the union of all these fields--and each deals with a view of that object.

This approach is the most symmetrical with the /events API, but this complicates implementation in the backend the most. We would have to maintain a set of structs for each of the three views of the super object.

The /checks endpoint would have a CheckConfiguration object that it deals with those fields.

The Agent API would have a version of Check that doesn't include history.

The /events endpoint would remain the same, but have to special case all of the history information.

Storage associated with the /checks API contains duplicated information (CheckConfiguration) included in the storage for the /events API.

So, unknowingly, we have implemented this approach, and it has complicated things sufficiently that I want us to take a step back and consider approaches 1 and 2--or possibly some new approaches.

from sensu-go.

Before deciding on a solution to this problem, I think it's important that we clearly define the behavior of these APIs. This wouldn't be premature, and if we start out with a well-defined behavior with limited scope, it should be easier to increase that scope later if we must.

from sensu-go.

• Add CheckConfiguration type that has "config" fields from Check type
• Break Checks out of store into CheckStore
• Refactor Check store into CheckConfiguration store
• Make the /checks API endpoint deal only with CheckConfigurations
• Scheduler emits CheckConfigurations instead of Events
• Session should send CheckConfigurations over the checks channel (may already be happening)
• Agent should deserialize messages over the checks channel as CheckConfigurations
• CLI changes

from sensu-go.