[Source] Deprecate key-type label and replace it with a spec field

[pierDipi]

Problem

A label for a configuration is fine if you don't have control over the CRD or for an experimental feature.

Specifying a key type is a core feature of KafkaSource, so I propose to introduce a new spec field as an alternative to the key-type label:

spec:
  key:
    sourceType: <int | string | byte-array | float>
    targetType: <https://github.com/cloudevents/spec/blob/v1.0.1/spec.md#type-system>

A spec field has many benefits over an annotation or label like validation since if I mistype the annotation key or label key I won't get any error or warning.

Persona:
Which persona is this feature for?

• event consumer

Exit Criteria
A measurable (binary) test that would indicate that the problem has been resolved.

Time Estimate (optional):
How many developer-days do you think this may take to resolve?
1

[lionelvillard]

+1 with the condition that all Kafka primitive types are supported.

Related to #897

[lionelvillard]

also why not keyType? Do you expect something else under key ?

[pierDipi]

also why not keyType?

I'm ok with keyType too.

Do you expect something else under key?

No, I don't atm.

[pierDipi]

+1 with the condition that all Kafka primitive types are supported.

Related to #897

I'm not entirely sure on supporting all those types, those are Kafka protocol types so I'm not sure how common they are as key of a message, in fact there is no serializer for INT8 or BOOLEAN in the Java client library.

[lionelvillard]

those are Kafka protocol types

what is keyType then, if not protocol types?

[pierDipi]

Kafka protocol types are types used by the Kafka protocol for communications between Kafka Clients and Kafka Server.

keyType is the type hint used to deserialize the key of a Consumer record (composed by key, value, and headers).

[lionelvillard]

Sorry you didn't quite answer the question. Records are encoded using the Kafka Protocol, which only encoded values, not the types. Consequently it's up to the application to indicate (directly or indirectly) what's the type of the encoded record (the key in this particular case) in order to decode it.

keyType can either be the Kafka protocol type or the target type (CloudEvent types). The latter seems to be the logical choice since the purpose of keyType is to help setting partitionKey, and it fits well into the Knative/CloudEvent world. Note that the opposite argument can be made when consuming events produced by a non-Knative/CloudEvent component (for instance a Java-based producer).

Bottom line, I would argue that the keyType value space should include all CloudEvent types and that we should clearly define the coercion rules between the Kafka primitive types and CloudEvent types.

Does this make sense?

[pierDipi]

Sorry, if I'm not quite following.

IMHO, "Kafka protocol types" are the representations on the wire of Kafka communication between Client and Server.
Since the key for Kafka is just a byte array, in my mind there is no "Kafka protocol" involved in the key encoding/decoding logic because it's an application-level concern.

Sorry you didn't quite answer the question. Records are encoded using the Kafka Protocol, which only encoded values, not the types. Consequently it's up to the application to indicate (directly or indirectly) what's the type of the encoded record (the key in this particular case) in order to decode it.

Ack.

keyType can either be the Kafka protocol type or the target type (CloudEvent types). The latter seems to be the logical choice since the purpose of keyType is to help setting partitionKey, and it fits well into the Knative/CloudEvent world. Note that the opposite argument can be made when consuming events produced by a non-Knative/CloudEvent component (for instance a Java-based producer).

Ack

Bottom line, I would argue that the keyType value space should include all CloudEvent types and that we should clearly define the coercion rules between the Kafka primitive types and CloudEvent types.

Ack, I was thinking the same and I updated the issue accordingly.

[github-actions]

This issue is stale because it has been open for 90 days with no
activity. It will automatically close after 30 more days of
inactivity. Reopen the issue with /reopen. Mark the issue as
fresh by adding the comment /remove-lifecycle stale.

[pierDipi]

/remove-lifecycle stale
/triage accepted