[microservices] KafkaRequestSerializer not serializing a object with attribute name "value"

[sprijk]

Is there an existing issue that is already proposing this?

• [X] I have searched the existing issues

Is your feature request related to a problem? Please describe it

When using the Kafka client the default serializer (KafkaRequestSerializer) checks if a message is not a Kafka(JS) message using:

const isNotKafkaMessage =
    isNil(value) ||
    !isObject(value) ||
    (!('key' in value) && !('value' in value));

when using the Kafka Client and sending/emitting an object with a measurement like: { time: 1, value: 12 } the boolean isNotKafkaMessage becomes false, thinking the message is a typical KafkaJS message with key and value attributes. Of course, the intention is to treat this message as a JSON object that in the end needs to be serialized using JSON.stringify()

Describe the solution you'd like

in my opinion the check should be changed to:

!(('key' in value) && ('value' in value))

to check both key and value attribute names existence in the object...

Teachability, documentation, adoption, migration strategy

I think the docs on the Kafka Client should be updated with the notion you should not use the attributes names value or key in the messages you want to send/emit. Possibly an explainer on how to use your own Serializer and attach that to the KafkaClient

What is the motivation / use case for changing the behavior?

To not have to particularly exclude used keyword value or key but only treat the message as a Kafka message if they both exist.

[sprijk]

for now it seems to be best to change the default serializer with an adjusted version in the options object

[kamilmysliwiec]

Would you like to create a PR for this issue?

[sprijk]

well, how would we handle backward compatibility for people who are currently calling the send()/emit() functions like

kafkaClient.emit(TOPIC_NAME, {
  value: {
    time: 1,
    value: 12
  }
})

trusting on the current serializer? using this implementation with the proposed new check would regress their current working code.

[jochongs]

I've created a fix PR #13375. Could you take a look? :)