eventing icon indicating copy to clipboard operation
eventing copied to clipboard

Changes on the `EventTypeSpec` type and move to v1beta3

Open matzew opened this issue 2 years ago • 6 comments
trafficstars

Feature Track document: https://docs.google.com/document/d/1hxCpQMI_qCy2CsHEv0nits4t65DExkizgg3KLvD4z2s/edit

Problem

Currently the Event Type Spec has the following fields:

  1. type: CloudEvents type
  2. source: CloudEvents source
  3. schema: CloudEvents schemaurl extension attribute
  4. schemaData: CloudEvents schema, embedded (which is a bad idea)
  5. broker (already deprecated in fav of reference)
  6. reference: the KReference to the belonging addressable
  7. description: the optional description

But there are some problems with those fields, since the first four fields are directly related to CloudEvents Spec which is the expected format of Events in Knative. However, the CloudEvents specification defines more metadata attributes.

CloudEvent attributes and extension

Here is a list of all CloudEvent attributes:

  • id (required)
  • source (required)
  • specversion (required)
  • type (required)
  • datacontenttype
  • dataschema
  • subject
  • time

In addition those there can be also extension attributes. A few of the core attributes are not reflected on the Knative EventType API, and extensions are also not honored.

Suggested Knative EventType API change

In order to address the above issue of mismatching attributes and extension, the EventTypeSpec could be changed to include a more flexible away to specify attributes and their occurance. To archive this we will remove/deprecate the existing cloud event centric fields (type, source, schema, schemaData) and will allow an array of EventAttributeDefinitions:

type EventTypeSpec struct {
	// Reference is a KReference to the belonging addressable.
	//For example, this could be a pointer to a Broker.
	// +optional
	Reference *duckv1.KReference `json:"reference,omitempty"`
	// Description is an optional field used to describe the EventType, in any meaningful way.
	// +optional
	Description string `json:"description,omitempty"`

	// CloudEvent attributes and extensions.
	Attributes []EventAttributeDefinition `json:"attributes"`
}

type EventAttributeDefinition struct {
	Name     string `json:"name,omitempty"`
	Required bool   `json:"required"`
	Value    string `json:"value,omitempty"`
}

Minimum of required EventType attributes

For Knative Eventing we should require a minimum set of attributes to be present:

  • type
  • id
  • specversion
  • source

Note: Our ingress layers are already checking if the incoming payload is a valid cloud event.

Example Resource for PingSource

This would lead us to EventType that can express a number of addition CloudEvent attributes and also extensions. For the PingSource fixed dev.knative.source.ping event we would have a CR like::

apiVersion: eventing.knative.dev/v1beta2
kind: EventType
metadata:
  name: dev.knative.sources.ping
  namespace: knative-eventing
spec:
  attributes:
    - name: type
      value: dev.knative.sources.ping
      required: true
    - name: specversion
      value: "1.0"
    - name: id
      required: true
    - name: source
      value: "/apis/v1/namespaces/$namespaceName/pingsources/$souceName"
    - name: time
      required: true
  description: CloudEvent type for fixed payloads on a specified cron schedule

The EventType is scoped on the SYSTEM_NAMESPACE (e.g. knative-eventing), and will contain no reference to a KReference (see here). This is used for general discoverability of events, potentially possible in the system.

NOTE: We would bundle a few Event type CRs for our build-in sources, and place them in the SYSTEM NAMESPACE. Affected sources would be:

  • PingSource
  • ApiServerSource

3rd party integration

Source providers are asked to do the same. If the source is installed to the SYSTEM_NAMESPACE, put a EventType as a definition to the knative-eventing namespace.

Custom Event information from workloads

Applications emitting their own events from workloads, such as deployments or batch job, are recommended to also provide EventType "definitions", while not pointing to a specific reference.

For instance:

apiVersion: eventing.knative.dev/v1beta3
kind: EventType
metadata:
  name: com.shop.domain.order.new
  namespace: my-app
spec:
  attributes:
    - name: type
      value: com.shop.domain.order.new
      required: true
    - name: specversion
      value: "1.0"
    - name: id
      required: true
    - name: source
      value: "/customer-system/node/$id/"
    - name: time
      required: true
    - name: myextension
      required: false
  description: CloudEvent type for new orders

Concrete Events

When than a concrete workload event is emitted, it will look like:

apiVersion: eventing.knative.dev/v1beta3
kind: EventType
metadata:
  name: com.shop.domain.order.new-somehash
  namespace: my-app
spec:
  attributes:
    - name: type
      value: com.shop.domain.order.new
      required: true
    - name: specversion
      value: "1.0"
    - name: id
      required: true
    - name: source
      value: "/customer-system/node/some-id-value"
    - name: time
      required: true
    - name: myextension
      required: false
  description: CloudEvent type for new orders
  reference: <my broker>

Persona: developers

Exit Criteria An updated spec and the version bump

matzew avatar Sep 19 '23 15:09 matzew

This could lead to a number of event types, like:

k get eventtypes.eventing.knative.dev -n foobar
NAMESPACE   NAME                           TYPE                       SOURCE          REFERENCE NAME   REFERENCE KIND
foobar      com.custom.event.orders        com.custom.event.orders   /system/id/$ID               
foobar      com.custom.event.orders-xyz    com.custom.event.orders   /system/id/007   prod-broker      Broker
foobar      com.custom.event.orders-UUID   com.custom.event.orders   /system/id/999   test-broker      Broker
foobar      com.custom.event.orders-SOME   com.custom.event.orders   /system/id/123   test-broker      Broker

QUESTIONS:

Question 1

On concrete workloads we know these are valid CloudEvents. Do we really care if a field is required or not? I think that's more some generic meta information about the event. Rather than a concrete event type, emitted by workloads

Answer: Since we do not track each event, just the more generic Event type, this is fine.

Question 2

Is it confusing to have same type (e.g. com.shop.domain.order.new) in one namespace? The one with a reference is a real workload. The one without a reference is a "definition" ?

Answer: Those without references can be:

  • filtered out (with some kubectl fu)
  • build-in sources can also point to the actual CRD itself (as the reference) We would treat those a generic "discovery" event types.

Question 3

Are we overloading the Eventtype for a good reason?

Answer: I guess so.

matzew avatar Sep 19 '23 15:09 matzew

Alternative: different CRD for pure definitions ((Cluster)EventDefinition). But that's something to explore if the above changes are no enough

matzew avatar Sep 19 '23 15:09 matzew

A wip PR is here: #7313 (but type bump etc is needed first)

matzew avatar Oct 17 '23 08:10 matzew

@matzew this may be a little late, but knowing whether an eventtype is a reply from a service or is a new event (e.g. from a source), is really useful when building out an eventing graph (e.g. for something like event lineage).

Do you know if there would be some way to model that with the existing spec changes, or if we could add something like isReply: true?

Cali0707 avatar Nov 28 '23 20:11 Cali0707

Why would you want to increase the complexity, that developers, in their services, need to say is reply when creating new events.

Any event produced is a new event, IMO.

A reply would it be when the same type is returned - which is a bad pattern anyways

matzew avatar Dec 01 '23 15:12 matzew

@Cali0707 we can model that differently, we can point the reference to whatever "Trigger's subscriber or Trigger" is producing that EventType

pierDipi avatar Dec 01 '23 15:12 pierDipi