Lower Kafka ConsumerConfig log level#14134
Conversation
nevillelyh
force-pushed
the
neville/kafka-logging
branch
from
8a5faf0 to
be8db30
Compare
nevillelyh
changed the title
| { | ||
| default KafkaConsumer<byte[], byte[]> create(ConnectorSession session) | ||
| { | ||
| Logger.getLogger(ConsumerConfig.class.getName()).setLevel(Level.WARNING); |
There was a problem hiding this comment.
We cannot do it like that.
First it would have to be done once per connector initialization so somewhere in the io.trino.plugin.kafka.KafkaConnectorFactory#create.
Also if set logging statically then you would override anything that is set via https://trino.io/docs/current/admin/properties-logging.html.
So I think we need to disable logging in tests. Somewhere here io.trino.plugin.kafka.KafkaQueryRunnerBuilder#build and update docs to mention that it is a good practice to set this in log.properties so your log file is less polluted.
I would do it as two PRs as they require different people for review.
There was a problem hiding this comment.
Also if set logging statically then you would override anything that is set via https://trino.io/docs/current/admin/properties-logging.html.
Yeah I was looking for alternatives like a connector level conf file but can't find any. Didn't want to change the defaultlog.properties.
So instead of hard coding it here, we just have to document how to suppress this & hard code setLevel in tests, right?
There was a problem hiding this comment.
Also looks like Kafka logging is already suppressed in KafkaQueryRunnerBuilder. So this should be a doc change only?
There was a problem hiding this comment.
So this should be a doc change only?
Yes, it looks like that. There also product tests where we can implement this suggestion. So we can avoid poluting logs in product tests.
See io.trino.tests.product.launcher.env.common.Kafka where we could have log.properties file that is used.
Please reach to @andrewdibiasio6 or @lukasz-walkiewicz to help you run product tests for kafka here.
There was a problem hiding this comment.
Replacing with these 2 PRs:
2 participants
Description
KafkaConsumerlogsConsumerConfigviaAbstractConfigon object creation and pollutes Trino logging.This change lowers
ConsumerConfiglog level.AbstractConfighas a constructor withboolean doLogbut it's not exposed throughConsumerConfig. And there doesn't seem to be an easy way to configurej.u.Loggerwithin a module. Airlift log-manager's has aLogging.setLevelthat might do the job but it's used tests only. Any idea why?Non-technical explanation
Make Kafka connector logging less verbose.
Release notes
(x) This is not user-visible and no release notes are required.
( ) Release notes are required, please propose a release note for me.
( ) Release notes are required, with the following suggested text: