Skip to content

KAFKA-6733: Support of printing additional ConsumerRecord fields in DefaultMessageFormatter - #4807

Closed
Matzz wants to merge 2 commits into
apache:trunkfrom
Matzz:print_all_record_fields_in_console_consumer
Closed

KAFKA-6733: Support of printing additional ConsumerRecord fields in DefaultMessageFormatter#4807
Matzz wants to merge 2 commits into
apache:trunkfrom
Matzz:print_all_record_fields_in_console_consumer

Conversation

@Matzz

@Matzz Matzz commented Apr 1, 2018

Copy link
Copy Markdown

New supported fields: offset, partition and headers. Code is backward compatible, so output will remain the same if user won't specify new configuration properties.
Added unit tests to DefaultMessageFormatter which cover previous and new properties.

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

@Matzz

Matzz commented Apr 1, 2018

Copy link
Copy Markdown
Author

@huxihx @ijuma Could I ask for review?

@Matzz
Matzz force-pushed the print_all_record_fields_in_console_consumer branch from 03b5a24 to b3df84a Compare April 2, 2018 12:31
@rajinisivaram

Copy link
Copy Markdown
Contributor

@ijuma Do we need a KIP for this since it is adding new properties to the CLI of ConsoleConsumer?

@Matzz

Matzz commented Apr 3, 2018

Copy link
Copy Markdown
Author

@rajinisivaram @ijuma I didn't found any documentation for current properties of DefaultMessageFormatter (provided by console consumer --property parameters). I'm happy to add some docs but I'm not sure where.

@Matzz
Matzz force-pushed the print_all_record_fields_in_console_consumer branch from b3df84a to b13fc04 Compare April 15, 2018 20:05
@Matzz

Matzz commented Apr 15, 2018

Copy link
Copy Markdown
Author

@rajinisivaram @ijumai resolved conflicts with trunk. Could I ask for some guide what's next? I should just wait for review?

@rajinisivaram

Copy link
Copy Markdown
Contributor

@Matzz Usually, when we add or change command line arguments to a tool, we need to write up a KIP (https://cwiki.apache.org/confluence/display/KAFKA/Kafka+Improvement+Proposals) and go throught the KIP review process. This is not exactly adding new CLI arguments, but it is adding externally visible properties, so not sure if we still need a KIP. Will see what @ijuma says.

BTW, looks like there are still conflicts on ConsoleConsumer.scala in the PR.

@Matzz
Matzz force-pushed the print_all_record_fields_in_console_consumer branch 2 times, most recently from c3774f6 to 5a47cd4 Compare April 20, 2018 09:20
@onzo-mateuszzakarczemny

Copy link
Copy Markdown

@rajinisivaram @ijuma A Kind nudge.
It takes a bit long for that PR. It doesn't brake any contract or any other backward comparability. It just adds couple more parameters for default message formatter (and therefore console consumer). I saw in other PRs that there was no KIP for such changes (eg. #4797)
If you look at the code you will see that the most of code is just bit of refactoring plus tests for the exiting functionality. Functionally there are just couple of if(shoidIPrintX) { print(x) }

@Matzz
Matzz force-pushed the print_all_record_fields_in_console_consumer branch from 5a47cd4 to 215eabe Compare May 15, 2018 21:44
@ijuma

ijuma commented May 21, 2018

Copy link
Copy Markdown
Member

@onzo-mateuszzakarczemny

Copy link
Copy Markdown

@ijuma Ok then. I'll create a KIP

@gwenshap

Copy link
Copy Markdown
Contributor

Was a KIP ever created? Can you follow up with #?

@gAmUssA

gAmUssA commented Jan 29, 2019

Copy link
Copy Markdown

@onzo-mateuszzakarczemny Hi Mateusz. Any progress on KIP? Thank you

@Matzz

Matzz commented Feb 11, 2019

Copy link
Copy Markdown
Author

Sorry I didn't have time to create a KIP. I'll try to work on that this week.

@gAmUssA

gAmUssA commented Feb 11, 2019

Copy link
Copy Markdown

@Matzz Thank you!

@Matzz
Matzz force-pushed the print_all_record_fields_in_console_consumer branch from 215eabe to b92f4f6 Compare February 16, 2019 21:47
@Matzz

Matzz commented Feb 18, 2019

Copy link
Copy Markdown
Author

KIP created - https://cwiki.apache.org/confluence/display/KAFKA/KIP-431%3A+Support+of+printing+additional+ConsumerRecord+fields+in+DefaultMessageFormatter
I need to create a email thread to now.

@Matzz

Matzz commented Feb 22, 2019

Copy link
Copy Markdown
Author

@Matzz

Matzz commented Mar 28, 2019

Copy link
Copy Markdown
Author

retest this please

@badaiaqrandista

Copy link
Copy Markdown
Contributor

Any update on this PR? Can this be merge? @Matzz @rajinisivaram

@stanislavkozlovski

Copy link
Copy Markdown
Contributor

@badaiaqrandista we cannot merge this as the KIP was not voted in. The vote thread was started on April 12th but has not received any votes.

@Matzz would you be interested in ressurrecting the thread?

@Matzz

Matzz commented Oct 24, 2019

Copy link
Copy Markdown
Author

@stanislavkozlovski
Yes sure. Just create new voting thread?

@stanislavkozlovski

Copy link
Copy Markdown
Contributor

@Matzz yeah!

@badaiaqrandista

badaiaqrandista commented Jul 29, 2020

Copy link
Copy Markdown
Contributor

This PR should be closed as it has been superseded by PR 9099 (#9099).

@dajac

dajac commented Oct 27, 2020

Copy link
Copy Markdown
Member

Closing as this was addressed by #9099.

@dajac dajac closed this Oct 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants