BartMiki commented on PR #12915: URL: https://github.com/apache/druid/pull/12915#issuecomment-1219105113
@FrankChen021 Thank you for the comment! Yes, the serialization of the event is very simple, all implementations are Jackson aware, so serialization of Event to JSON is as simple as [in this test](https://github.com/apache/druid/blob/7fb1153bba73bbb1ae245a8acc7bf605463ed725/server/src/test/java/org/apache/druid/server/log/DefaultRequestLogEventTest.java#L72) or in the [HttpPostEmitter](https://github.com/apache/druid/blob/7fb1153bba73bbb1ae245a8acc7bf605463ed725/core/src/main/java/org/apache/druid/java/util/emitter/core/HttpPostEmitter.java#L272): ```java objectMapper.writeValueAsString(event) ``` There are [tests](https://github.com/apache/druid/blob/7fb1153bba73bbb1ae245a8acc7bf605463ed725/server/src/test/java/org/apache/druid/server/log/DefaultRequestLogEventTest.java#L83) for checking the `.toMap`, but they only cover `.toMap` and compare it against other map (they do not serialize it further). However, the problem is that the original KafkaEmitter requires to pass additional property to the output event, which is `clusterName` (if present). That is why we cannot serialize using simple POJO: ```java objectMapper.writeValueAsString(event) ``` The original author [created new map](https://github.com/apache/druid/blob/7fb1153bba73bbb1ae245a8acc7bf605463ed725/extensions-contrib/kafka-emitter/src/main/java/org/apache/druid/emitter/kafka/KafkaEmitter.java#L169-L176) based on the `.toMap` and this optional `clusterName` property. This is the problem, as there are no polymorphism-aware maps and lists (see [Jackson ticket](https://github.com/FasterXML/jackson-databind/issues/699#issuecomment-73382972)). The only other solution that comes to my mind is to do: 1. If additional properties map is empty, then just return `objectMapper.writeValueAsString(event)` 2. If not empty, then: a) serialize event to `ObjectNode` using `objectMapper.valueToTree(event)` (serialize whole object in one go) b) add all KV pairs from additional parameters to the ObjectNode c) return serialized `ObjectNode` as a string We won't use `.toMap` at any stage as we will be using the same simple implementation of Event -> JSON as other Emitters. What do you think @FrankChen021 ? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
