You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Badai Aqrandista <ba...@confluent.io> on 2020/08/04 14:49:27 UTC
Re: [DISCUSS] KIP-431: Support of printing additional ConsumerRecord
fields in DefaultMessageFormatter
Hi all
I have made additional changes to the PR as per review from David Jacot.
Please review.
Regards
Badai
On Wed, Jul 29, 2020 at 11:17 PM Badai Aqrandista <ba...@confluent.io>
wrote:
> Hi all
>
> I have created a PR for KIP-431 against the latest trunk:
> https://github.com/apache/kafka/pull/9099
>
> Please review.
>
> Regards
> Badai
>
> On Tue, Jul 21, 2020 at 2:13 AM Matthias J. Sax <mj...@apache.org> wrote:
> >
> > Thanks Badai. LGTM.
> >
> > On 7/19/20 4:26 PM, Badai Aqrandista wrote:
> > > Hi all
> > >
> > > I have made a small change to KIP-431 to make it clearer which one is
> > > "Partition" and "Offset". Also I have moved key field to the back,
> > > before the value:
> > >
> > > $ kafka-console-consumer.sh --bootstrap-server localhost:9092 --topic
> > > test --from-beginning --property print.partition=true --property
> > > print.key=true --property print.timestamp=true --property
> > > print.offset=true --property print.headers=true --property
> > > key.separator='|'
> > >
> > > CreateTime:1592475472398|Partition:0|Offset:3|h1:v1,h2:v2|key1|value1
> > > CreateTime:1592475472456|Partition:0|Offset:4|NO_HEADERS|key2|value2
> > >
> > > Regards
> > > Badai
> > >
> > > On Sun, Jun 21, 2020 at 11:39 PM Badai Aqrandista <ba...@confluent.io>
> wrote:
> > >>
> > >> Excellent.
> > >>
> > >> Would like to hear more feedback from others.
> > >>
> > >> On Sat, Jun 20, 2020 at 1:27 AM David Jacot <dj...@confluent.io>
> wrote:
> > >>>
> > >>> Hi Badai,
> > >>>
> > >>> Thanks for your reply.
> > >>>
> > >>> 2. Yes, that makes sense.
> > >>>
> > >>> Best,
> > >>> David
> > >>>
> > >>> On Thu, Jun 18, 2020 at 2:08 PM Badai Aqrandista <ba...@confluent.io>
> wrote:
> > >>>
> > >>>> David
> > >>>>
> > >>>> Thank you for replying
> > >>>>
> > >>>> 1. It seems that `print.partition` is already implemented. Do you
> confirm?
> > >>>> BADAI: Yes, you are correct. I have removed it from the KIP.
> > >>>>
> > >>>> 2. Will `null.literal` be only used when the value of the message
> > >>>> is NULL or for any fields? Also, it seems that we print out "null"
> > >>>> today when the key or the value is empty. Shall we use "null" as
> > >>>> a default instead of ""?
> > >>>> BADAI: For any fields. Do you think this is useful?
> > >>>>
> > >>>> 3. Could we add a small example of the output in the KIP?
> > >>>> BADAI: Yes, I have updated the KIP to add a couple of example.
> > >>>>
> > >>>> 4. When there are no headers, are we going to print something
> > >>>> to indicate it to the user? For instance, we print out NO_TIMESTAMP
> > >>>> where there is no timestamp.
> > >>>> BADAI: Yes, good idea. I have updated the KIP to print NO_HEADERS.
> > >>>>
> > >>>> Thanks
> > >>>> Badai
> > >>>>
> > >>>>
> > >>>> On Thu, Jun 18, 2020 at 7:25 PM David Jacot <dj...@confluent.io>
> wrote:
> > >>>>>
> > >>>>> Hi Badai,
> > >>>>>
> > >>>>> Thanks for resuming this. I have few small comments:
> > >>>>>
> > >>>>> 1. It seems that `print.partition` is already implemented. Do you
> > >>>> confirm?
> > >>>>>
> > >>>>> 2. Will `null.literal` be only used when the value of the message
> > >>>>> is NULL or for any fields? Also, it seems that we print out "null"
> > >>>>> today when the key or the value is empty. Shall we use "null" as
> > >>>>> a default instead of ""?
> > >>>>>
> > >>>>> 3. Could we add a small example of the output in the KIP?
> > >>>>>
> > >>>>> 4. When there are no headers, are we going to print something
> > >>>>> to indicate it to the user? For instance, we print out NO_TIMESTAMP
> > >>>>> where there is no timestamp.
> > >>>>>
> > >>>>> Best,
> > >>>>> David
> > >>>>>
> > >>>>> On Wed, Jun 17, 2020 at 4:53 PM Badai Aqrandista <
> badai@confluent.io>
> > >>>> wrote:
> > >>>>>
> > >>>>>> Hi all,
> > >>>>>>
> > >>>>>> I have contacted Mateusz separately and he is ok for me to take
> over
> > >>>>>> KIP-431:
> > >>>>>>
> > >>>>>>
> > >>>>>>
> > >>>>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-431%3A+Support+of+printing+additional+ConsumerRecord+fields+in+DefaultMessageFormatter
> > >>>>>>
> > >>>>>> I have updated it a bit. Can anyone give a quick look at it again
> and
> > >>>>>> give me some feedback?
> > >>>>>>
> > >>>>>> This feature will be very helpful for people supporting Kafka in
> > >>>>>> operations.
> > >>>>>>
> > >>>>>> If it is ready for a vote, please let me know.
> > >>>>>>
> > >>>>>> Thanks
> > >>>>>> Badai
> > >>>>>>
> > >>>>>> On Sat, Jun 13, 2020 at 10:59 PM Badai Aqrandista <
> badai@confluent.io>
> > >>>>>> wrote:
> > >>>>>>>
> > >>>>>>> Mateusz
> > >>>>>>>
> > >>>>>>> This KIP would be very useful for debugging. But the last
> discussion
> > >>>>>>> is in Feb 2019.
> > >>>>>>>
> > >>>>>>> Are you ok if I take over this KIP?
> > >>>>>>>
> > >>>>>>> --
> > >>>>>>> Thanks,
> > >>>>>>> Badai
> > >>>>>>
> > >>>>>>
> > >>>>>>
> > >>>>>> --
> > >>>>>> Thanks,
> > >>>>>> Badai
> > >>>>>>
> > >>>>
> > >>>>
> > >>>>
> > >>>> --
> > >>>> Thanks,
> > >>>> Badai
> > >>>>
> > >>
> > >>
> > >>
> > >> --
> > >> Thanks,
> > >> Badai
> > >
> > >
> > >
> >
>
>
> --
> Thanks,
> Badai
>
--
Thanks,
Badai
Re: [DISCUSS] KIP-431: Support of printing additional ConsumerRecord
fields in DefaultMessageFormatter
Posted by Badai Aqrandista <ba...@confluent.io>.
Hi all,
I just realised that KIP-431 introduces an incompatible change when an
undocumented property "print.partition" is set to "true". I found this
because of the system test failure. However, because the
incompatibility affects an undocumented property, I do not have
migration plan in this KIP.
And I have added the following to KIP-431 (
https://cwiki.apache.org/confluence/display/KAFKA/KIP-431%3A+Support+of+printing+additional+ConsumerRecord+fields+in+DefaultMessageFormatter
).
-----
Compatibility, Deprecation, and Migration Plan
KIP-431 introduces incompatibility when "print.partition=true" (this
property exists in the code before KIP-431 but not documented). Before
KIP-431, "kafka-console-consumer" prints the partition as a number
after the value for example: "key1|value1|0". After this KIP,
"kafka-console-consumer" prints the partition number prefixed with
"Partition:" before the key (if printed) and value, for example:
"Partition:0|key1|value1" . Because this property was not documented,
no migration plan is implemented in KIP-431.
The other changes are backward compatible because they do not exist
before. Apart from "print.partition=true", if a user does not use any
new parameters, then the output of console consumer will look the same
as before.
-----
What do you think?
Regards
Badai
On Wed, Aug 5, 2020 at 12:49 AM Badai Aqrandista <ba...@confluent.io> wrote:
>
> Hi all
>
> I have made additional changes to the PR as per review from David Jacot. Please review.
>
> Regards
> Badai
>
> On Wed, Jul 29, 2020 at 11:17 PM Badai Aqrandista <ba...@confluent.io> wrote:
>>
>> Hi all
>>
>> I have created a PR for KIP-431 against the latest trunk:
>> https://github.com/apache/kafka/pull/9099
>>
>> Please review.
>>
>> Regards
>> Badai
>>
>> On Tue, Jul 21, 2020 at 2:13 AM Matthias J. Sax <mj...@apache.org> wrote:
>> >
>> > Thanks Badai. LGTM.
>> >
>> > On 7/19/20 4:26 PM, Badai Aqrandista wrote:
>> > > Hi all
>> > >
>> > > I have made a small change to KIP-431 to make it clearer which one is
>> > > "Partition" and "Offset". Also I have moved key field to the back,
>> > > before the value:
>> > >
>> > > $ kafka-console-consumer.sh --bootstrap-server localhost:9092 --topic
>> > > test --from-beginning --property print.partition=true --property
>> > > print.key=true --property print.timestamp=true --property
>> > > print.offset=true --property print.headers=true --property
>> > > key.separator='|'
>> > >
>> > > CreateTime:1592475472398|Partition:0|Offset:3|h1:v1,h2:v2|key1|value1
>> > > CreateTime:1592475472456|Partition:0|Offset:4|NO_HEADERS|key2|value2
>> > >
>> > > Regards
>> > > Badai
>> > >
>> > > On Sun, Jun 21, 2020 at 11:39 PM Badai Aqrandista <ba...@confluent.io> wrote:
>> > >>
>> > >> Excellent.
>> > >>
>> > >> Would like to hear more feedback from others.
>> > >>
>> > >> On Sat, Jun 20, 2020 at 1:27 AM David Jacot <dj...@confluent.io> wrote:
>> > >>>
>> > >>> Hi Badai,
>> > >>>
>> > >>> Thanks for your reply.
>> > >>>
>> > >>> 2. Yes, that makes sense.
>> > >>>
>> > >>> Best,
>> > >>> David
>> > >>>
>> > >>> On Thu, Jun 18, 2020 at 2:08 PM Badai Aqrandista <ba...@confluent.io> wrote:
>> > >>>
>> > >>>> David
>> > >>>>
>> > >>>> Thank you for replying
>> > >>>>
>> > >>>> 1. It seems that `print.partition` is already implemented. Do you confirm?
>> > >>>> BADAI: Yes, you are correct. I have removed it from the KIP.
>> > >>>>
>> > >>>> 2. Will `null.literal` be only used when the value of the message
>> > >>>> is NULL or for any fields? Also, it seems that we print out "null"
>> > >>>> today when the key or the value is empty. Shall we use "null" as
>> > >>>> a default instead of ""?
>> > >>>> BADAI: For any fields. Do you think this is useful?
>> > >>>>
>> > >>>> 3. Could we add a small example of the output in the KIP?
>> > >>>> BADAI: Yes, I have updated the KIP to add a couple of example.
>> > >>>>
>> > >>>> 4. When there are no headers, are we going to print something
>> > >>>> to indicate it to the user? For instance, we print out NO_TIMESTAMP
>> > >>>> where there is no timestamp.
>> > >>>> BADAI: Yes, good idea. I have updated the KIP to print NO_HEADERS.
>> > >>>>
>> > >>>> Thanks
>> > >>>> Badai
>> > >>>>
>> > >>>>
>> > >>>> On Thu, Jun 18, 2020 at 7:25 PM David Jacot <dj...@confluent.io> wrote:
>> > >>>>>
>> > >>>>> Hi Badai,
>> > >>>>>
>> > >>>>> Thanks for resuming this. I have few small comments:
>> > >>>>>
>> > >>>>> 1. It seems that `print.partition` is already implemented. Do you
>> > >>>> confirm?
>> > >>>>>
>> > >>>>> 2. Will `null.literal` be only used when the value of the message
>> > >>>>> is NULL or for any fields? Also, it seems that we print out "null"
>> > >>>>> today when the key or the value is empty. Shall we use "null" as
>> > >>>>> a default instead of ""?
>> > >>>>>
>> > >>>>> 3. Could we add a small example of the output in the KIP?
>> > >>>>>
>> > >>>>> 4. When there are no headers, are we going to print something
>> > >>>>> to indicate it to the user? For instance, we print out NO_TIMESTAMP
>> > >>>>> where there is no timestamp.
>> > >>>>>
>> > >>>>> Best,
>> > >>>>> David
>> > >>>>>
>> > >>>>> On Wed, Jun 17, 2020 at 4:53 PM Badai Aqrandista <ba...@confluent.io>
>> > >>>> wrote:
>> > >>>>>
>> > >>>>>> Hi all,
>> > >>>>>>
>> > >>>>>> I have contacted Mateusz separately and he is ok for me to take over
>> > >>>>>> KIP-431:
>> > >>>>>>
>> > >>>>>>
>> > >>>>>>
>> > >>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-431%3A+Support+of+printing+additional+ConsumerRecord+fields+in+DefaultMessageFormatter
>> > >>>>>>
>> > >>>>>> I have updated it a bit. Can anyone give a quick look at it again and
>> > >>>>>> give me some feedback?
>> > >>>>>>
>> > >>>>>> This feature will be very helpful for people supporting Kafka in
>> > >>>>>> operations.
>> > >>>>>>
>> > >>>>>> If it is ready for a vote, please let me know.
>> > >>>>>>
>> > >>>>>> Thanks
>> > >>>>>> Badai
>> > >>>>>>
>> > >>>>>> On Sat, Jun 13, 2020 at 10:59 PM Badai Aqrandista <ba...@confluent.io>
>> > >>>>>> wrote:
>> > >>>>>>>
>> > >>>>>>> Mateusz
>> > >>>>>>>
>> > >>>>>>> This KIP would be very useful for debugging. But the last discussion
>> > >>>>>>> is in Feb 2019.
>> > >>>>>>>
>> > >>>>>>> Are you ok if I take over this KIP?
>> > >>>>>>>
>> > >>>>>>> --
>> > >>>>>>> Thanks,
>> > >>>>>>> Badai
>> > >>>>>>
>> > >>>>>>
>> > >>>>>>
>> > >>>>>> --
>> > >>>>>> Thanks,
>> > >>>>>> Badai
>> > >>>>>>
>> > >>>>
>> > >>>>
>> > >>>>
>> > >>>> --
>> > >>>> Thanks,
>> > >>>> Badai
>> > >>>>
>> > >>
>> > >>
>> > >>
>> > >> --
>> > >> Thanks,
>> > >> Badai
>> > >
>> > >
>> > >
>> >
>>
>>
>> --
>> Thanks,
>> Badai
>
>
>
> --
> Thanks,
> Badai
>
--
Thanks,
Badai