> On Feb. 25, 2014, 7:14 p.m., Neha Narkhede wrote: > > clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java, > > line 122 > > <https://reviews.apache.org/r/18343/diff/1/?file=499802#file499802line122> > > > > could we also have config.logOveridden() ?
We have two things: the full config values and the user-suplied strings (later are a subset of the former). I think the full config values are more useful as they include any parsing logic as well as showing the default so I'll do those. > On Feb. 25, 2014, 7:14 p.m., Neha Narkhede wrote: > > clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java, > > line 227 > > <https://reviews.apache.org/r/18343/diff/1/?file=499802#file499802line227> > > > > My understanding is that any per message log message be at trace and > > rest be at debug(). Very few remaining user facing messages should be at > > info(). > > > > Given that, should this also be at trace()? My approach was I wrote the logging and used it to debug some issues. My criteria was the following: 1. Normal non-failure should have no logging by default since we are just a humble client 2. Debug should produce stuff that is of general noteworthiness when you turn on logging. My criteria was roughly that running in debug you should hear about all noteworthy things that occur but have gobs of logging. You should be okay running with debug logging on with little or no per impact. 3. Trace is more detailed step-by-step tracing of all execution including fairly boring stuff like messages passing through different layers. So I guess the questions are (1) do those criteria make sense, and (2) is this something you would probably want to know about in debug level. > On Feb. 25, 2014, 7:14 p.m., Neha Narkhede wrote: > > clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java, > > line 265 > > <https://reviews.apache.org/r/18343/diff/1/?file=499802#file499802line265> > > > > Since this is not a per message but instead is a per producer message, > > can this be at debug()? > > > > Can we also have corresponding log messages on producer startup? Done. > On Feb. 25, 2014, 7:14 p.m., Neha Narkhede wrote: > > clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordAccumulator.java, > > line 133 > > <https://reviews.apache.org/r/18343/diff/1/?file=499805#file499805line133> > > > > Since this is also not expected per message, should this be at debug()? This is definitely in my definition of trace above (i.e. almost certainly not interesting and totally normal steady-state behavior. > On Feb. 25, 2014, 7:14 p.m., Neha Narkhede wrote: > > clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordBatch.java, > > line 77 > > <https://reviews.apache.org/r/18343/diff/1/?file=499806#file499806line77> > > > > Let's also specify the topic partition here. Done. - Jay ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/18343/#review35431 ----------------------------------------------------------- On Feb. 21, 2014, 4:26 a.m., Jay Kreps wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/18343/ > ----------------------------------------------------------- > > (Updated Feb. 21, 2014, 4:26 a.m.) > > > Review request for kafka. > > > Bugs: KAFKA-1250 > https://issues.apache.org/jira/browse/KAFKA-1250 > > > Repository: kafka > > > Description > ------- > > KAFKA-1250 Add logging to new producer. > > > Diffs > ----- > > build.gradle 58a6396f33413cc42231f445075f36552568563a > clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java > e4bc97279585818860487a39a93b6481742b91db > clients/src/main/java/org/apache/kafka/clients/producer/ProducerRecord.java > 034bf33385fe3b4222f482ccd16de6b530e1c1a7 > > clients/src/main/java/org/apache/kafka/clients/producer/internals/Metadata.java > 62613a3e29a7e5ffb1cc56d267793fef72857fc6 > > clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordAccumulator.java > ce5cf27efa08b79e501439cf79bc8666054a5429 > > clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordBatch.java > eb16f6d236e07b16654623606294a051531b5f58 > > clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java > e373265f19f6ec9d40b1813a1ab7e6b5b10e0acd > clients/src/main/java/org/apache/kafka/common/config/AbstractConfig.java > c3148e5a9061d9748b617e3772c132fd89d35f05 > clients/src/main/java/org/apache/kafka/common/metrics/JmxReporter.java > e08c349a6cd7617eb025709c3fb7890639ef610e > clients/src/main/java/org/apache/kafka/common/network/Selector.java > f1e474cd53011970c6bd3db6dfacd0f12ed9ce6b > clients/src/main/java/org/apache/kafka/common/requests/RequestHeader.java > 457abb1ad44ed67ef6d015dc7859d1c4048d02fe > clients/src/main/java/org/apache/kafka/common/requests/RequestSend.java > c5e9020b2e7692c37c65bbe6bcbce5b37574baac > clients/src/main/java/org/apache/kafka/common/utils/KafkaThread.java > 9ff793f38d70d52580255d9c759c16e3c4116e10 > > Diff: https://reviews.apache.org/r/18343/diff/ > > > Testing > ------- > > > Thanks, > > Jay Kreps > >
