Custom Attributes

[grepory]

Blocks #414 and #458

[palourde]

Okay so I thought this issue would be the easiest thing ever but turns out it’s a nightmare. I naively added the line map<string, Any> custom_attributes = 12 [(gogoproto.nullable) = false]; to message Entity. From that point, I'm able to generate the *.pb.go files (even if I receive some warnings) but now, I have absolutely no clue how we can use that in our code.

I wasn't able to find a lot of resources on the subject but ironically, found out https://github.com/golang/protobuf/issues/60 haha. I'll definitively need some help here!

[grepory]

Yeah. I forgot about Any.

[grepory]

ptypes.MarshalAny and UnmarshalAny seem reasonable. Is there something preventing you from using those?

[grepory]

Ah JSON marshaling unmarshaling. I think I just answered my own question.

[palourde]

Yeah, I basically ended up using JSON to serialize the custom attributes into Any but that feels really weird to me. A bit like this: https://github.com/containerd/typeurl/blob/master/types.go

[grepory]

Yeah, and that breaks the API we had wanted originally which was that CustomAttributes is as close to a Ruby Hash as possible, but there's really no easy way to do that in Go.

[grepory]

I think the easiest route is to say that we only support one type of key and value: string.

In govaluate expressions, comparisons will have to take this into account, which is a bit annoying, but we could own some of the complexity inside of query parsing eventually--converting non-string arguments into strings.

[grepory]

But then that breaks anything besides equality comparisons.

[calebhailey]

Yikes. This seems like a really hard problem! I've been wondering if this would sneak up on us...

[echlebek]

It's pretty hard to handle arbitrary data structures in Go without writing piles of reflection. I've thought about embedding Lua to deal with it, but it's a relatively heavy dependency.

[portertech]

[grepory]

I was just looking through k8s metadata to see what they do, but they do exactly what I suggested: map<string, string>. Also, they only support equality comparisons in selectors.

We could solve this with an uglier API.

entity.GetInt('key') entity.GetFloat('key')

[portertech]

@grepory "I think the easiest route is to say that we only support one type of key and value: string" - for attributes used in filter statements or all custom attributes?

[grepory]

@portertech It would have to be for all custom attributes.

[grepory]

@echlebek reminds me of json.RawMessage with this blog post.

What we are primarily concerned about here is the JSON API and giving the Agents/Users ability to specify custom attributes for their checks and entities. If we decouple the JSON and Protobuf APIs, then this becomes sort of easier to solve.

CustomAttributes becomes an internal storage mechanism for a map<string, Any> and we own the complexity of using Any. During serialization/deserialization, we first unmarshal into a Check/Entity as is appropriate, but then do a second deserialization, stripping any keys we know exist within the original types' set of fields into a map<string, Any>. The reverse, I actually do not know how to do off the top of my head. That blog post only works with real, actual types all the way down.

This is not pretty--and it does mean that we're doing two marshal/unmarshal phases. It also introduces a... more copious bounty of bugs that could be unearthed. It is an idea, though.

[portertech]

This would be unfortunate. As a user, I would like to be able to write event handler plugins that utilize custom attributes that have values with various types. Are we also losing the ability to have nested maps etc? So flat map of strings?

[portertech]

I just wrote an EC2 discovery tool for a 1.x customer that creates Sensu Proxy Clients for EC2 instances matching certain criteria (e.g. running, tagged with "governance"). The Proxy Clients contain a custom attribute "ec2": {} that contains various EC2 instance attributes. There are also nested hashes and array values, e.g "tags": {}. I am thinking about how I would approach this with 2.0, given the most severe limitation (string=string), e.g. "ec2_tags_tagname"="tagvalue".

[palourde]

Disclaimer:

I got inspired by what I've found in https://github.com/containerd/typeurl/blob/master/types.go and started played around that code. Here's the result:

In entity.proto, we could have something like this:

message Entity {
  Any custom_attributes = 1 [(gogoproto.nullable) = false];
}

Then, we could do something like this:

// marshal the custom attributes with json
customAttributes := map[string]interface{}{"foo": "bar", "qux": map[string]string{"qux": "baz"}}
bytes, _ := json.Marshal(customAttributes)
e.CustomAttributes = types.Any{
  TypeUrl: "github.com/sensu/sensu.types.Any",
  Value:   bytes,
}

// marshal to simulate going on the wire
serialized, _ := proto.Marshal(e)
if err != nil {
  logger.Fatal("could not serialize anything")
}

// unmarshal to simulate coming off the wire
var entity types.Entity
if err := proto.Unmarshal(serialized, &entity); err != nil {
  logger.Fatal("could not deserialize anything")
}

// unmarshal the custom attributes with json
var customAttributes2 map[string]interface{}
_ = json.Unmarshal(entity.CustomAttributes.GetValue(), &customAttributes2)

// The following statement would print something like this: map[foo:bar qux:map[qux:baz]]
fmt.Printf("%+v\n", customAttributes2)

Does it even makes sense? Would it be usable? 🤷‍♀️

[grepory]

So, serialized, this means that an Entity JSON would look like this:

{
  "id": "entity_id",
  "custom_attributes": {
    "type_url": "sensu.types.Any",
    "value": "{\"key\": \"value\"}"
  }
}

Right? Which I think not what we want.

[echlebek]

Oops, I opened a new issue instead of using this one. This is now a dupe of #586