Avro Event Format for CloudEvents

[fabiojose]

Signed-off-by: Fabio José [email protected]

Closes #426

Accidentally I've closed the original PR, but open this new one with the right changes.

[NickLarsenNZ]

Original PR with comments trail: #438

[duglin]

@fabiojose can you update the README to point to this doc?

[duglin]

We tentatively approved this on today's call, but people asked for one more week to make sure. Please look it over so we can resolve it next week

[duglin]

@JemDay I believe you asked for one more week,last week. You ok with this one?

[clemensv]

I don't think the definition of a CloudEvent as a "record" satisfies our need for having extensibility, because record requires all fields to be known for encoding. I haven't yet done any deeper investigation, but I just stumbled over this post here and this schema in one of the answers seems clever:

{
    "name": "metadata",
    "type": {
        "type": "map",
        "values": [
            "null",
            "int",
            "float",
            "string",
            "boolean",
            "long",
            {
                "type": "map",
                "values": [
                    "null",
                    "int",
                    "float",
                    "string",
                    "boolean",
                    "long"
                ]
            }
        ]
    }
}

[JemDay]

I think the prior comment from @clemensv may be touching slightly on what i'd been thinking about given proposal was very structured (which in a way i like) vs the approach taken with proto which is much more minimalistic. The proto schema is more aligned with the example @clemensv cited.

As it stands with the current schema proposal a mechanism to carry 'extensions' is required.

[fabiojose]

Very clever @clemensv

I tested it and got the following schema:

{
  "namespace":"io.cloudevents",
  "type":"record",
  "name":"CloudEvent",
  "version":"0.4-wip",
  "doc":"Avro Event Format fo CloudEvents",
  "fields":[
    {
      "name":"metadata",
      "type":{
        "type":"map",
        "values":[
          "null",
          "int",
          "float",
          "string",
          "boolean",
          "long",
          "CloudEvent"
        ]
      }
    }
  ]
}

It looks more flexible than the first version.

[evankanderson]

We don't have float or boolean types defined at the moment.

We may also need to support binary types. I don't know how "binary" is spelled in Avro, possibly bytes.

[fabiojose]

Good points @evankanderson!

Done!

{
  "namespace":"io.cloudevents",
  "type":"record",
  "name":"CloudEvent",
  "version":"0.4-wip",
  "doc":"Avro Event Format for CloudEvents",
  "fields":[
    {
      "name":"attribute",
      "type":{
        "type":"map",
        "values":[
          "null",
          "int",
          "string",
          "bytes",
          "CloudEvent"
        ]
      }
    }
  ]
}

[duglin]

@clemensv @evankanderson does this look ok now?

[JemDay]

Disclaimer : Not an avro expert..

For consistency reasons with protobuf...

• Could we change the name of the definition file to cloudevent.avsc
• Does avro allow you to follow the same model as used in cloudevent.proto

To be fair i would have preferred the message in proto to have been called CloudEvent as opposed to CloudEventMap - i guess that's a different PR

Otherwise +1

[fabiojose]

Hi @JemDay

"Could we change the name of the definition file to cloudevent.avsc": Sure!

• I had used the spec.avsc to follow the same naming convention used by spec.json.

"Does avro allow you to follow the same model as used in cloudevent.proto"

• unfortunately, not

[JemDay]

Thanks ... thinking a bit more, maybe aligning around spec.format-suffix is better

Still +1

[fabiojose]

Good point! I will change the file name.

[duglin]

at this point I think consistency with the others is more important tho

[duglin]

@fabiojose is this one ready to go? it was approved to but I don't want to merge if you still have a pending change

[fabiojose]

Done, no pending changes!

The file will be spec.avsc, just like before.

[catkins-miso]

It seems there would have been a performance impact removing the first-class context attributes in favor of the attribute map approach. I understand the point of adding the attribute map for extensions, but I think the schema-on-read approach could have been isolated without impacting other use cases. In particular, if I'm listening to a firehose topic (events of many types)--or implementing an intermediary--I need to quickly evaluate the type context attribute. Not to mention Avro has good mechanics for forward and backwards compatibility which I think could have addressed concerns about introducing new context attributes.

[evankanderson]

I think the attributes may have been simplified in the interim for v1 (when this was written, they were a json-style string -> value or object mapping, rather than a string -> simple type mapping.

Unfortunately, I think the v1.0 spec ship has sailed; I'm not sure what to suggest here.

[catkins-miso]

Thanks for the response @evankanderson. I did a little preliminary benchmarking (Apache Avro 1.9 dotnetcore), and I am seeing no meaningful difference in the binary serde performance from the attributes and having distinct fields.