Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs(logging): log4j2 support updates #11254

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

PaulRMellor
Copy link
Contributor

@PaulRMellor PaulRMellor commented Mar 11, 2025

Documentation

Updates the logging content to reflect new support and Kafka 4.0 adoption of log4j2

  • Removes/updates references to log4j1 and log4j1 logger names
  • Updates logging content in the deploying guide
    • Refreshes logging config overview
  • streamlines logging information in API reference guide to remove repetition

Checklist

Please go through this checklist and make sure all applicable tasks have been done

  • Write tests
  • Make sure all tests pass
  • Update documentation
  • Check RBAC rights for Kubernetes / OpenShift roles
  • Try your changes from Pod inside your Kubernetes and OpenShift cluster, not just locally
  • Reference relevant issue(s) and close them after merging
  • Update CHANGELOG.md
  • Supply screenshots for visual changes, such as Grafana dashboards

Signed-off-by: prmellor <pmellor@redhat.com>
@PaulRMellor PaulRMellor added this to the 0.46.0 milestone Mar 11, 2025
Signed-off-by: prmellor <pmellor@redhat.com>
@PaulRMellor
Copy link
Contributor Author

Thanks for the comments @fvaleri
I've updated the examples as directed.
I've also updated the section on adding logging filters (appenders) based on your feedback.

Signed-off-by: prmellor <pmellor@redhat.com>
Copy link
Contributor

@fvaleri fvaleri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks.

Comment on lines +40 to +45
----
logger.healthy.name = http.openapi.operation.healthy
logger.healthy.level = WARN
logger.ready.name = http.openapi.operation.ready
logger.ready.level = WARN
----
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is still a bit confusing. If these loggers are defined, you should not need to create them.

@ppatierno Can you please clarify it? Do these loggers really exist? Or how is it? They do not seem to be in the Log4j2 configuration file.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I see them in strimzi-kafka-bridge/config
/log4j2.properties and strimzi-kafka-operator/cluster-operator/.../default-logging
/KafkaBridgeCluster.properties
Let's see what Paolo says

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two yes, that would mean you can leave out the .name lines. But not the other ones - they do not exist there.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what you mean here.
The bridge defines loggers for the single operations here https://github.com/strimzi/strimzi-kafka-bridge/blob/main/src/main/java/io/strimzi/kafka/bridge/http/HttpOpenApiOperation.java#L30 and through the above lines you can set the verbosity level after creating the name by following the rules above.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is getting the logger. Does it also define it? If so, does it define it only when the method is actually called? I'm not sure this realyl answers the question if the logger should be defined in the configuration or not (and just the log level can be configured).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am pretty sure I am going to say something you already well know but I still get confused on what you are trying to say here.
The code I linked above is just for getting the logger as you said and using it. It's different for each endpoint (i.e. http.openapi.operation.healthy in the example but could be http.openapi.operation.send for the endpoint related to the send operation and so on).
The two lines we have in the properties file are for assigning a name to the logger related to the specific component in the code. With that name we can configure the logger, in this case just the log level.
I don't think we can get rid of any of them, if this is what you are suggesting and I am getting difficulties to understand :-)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I guess we should maybe replace the lines 14-57 with something like this?

Kafka Bridge has its own preconfigured loggers:

* `rootLogger` (default logger for all classes)
* `bridge` (all classes in the `io.strimzi.kafka.bridge` package)
* `healthy` (The healthy endpoint `http.openapi.operation.healthy` - defaults to default: `WARN`)
* `ready` (The healthy endpoint `http.openapi.operation.ready` - defaults to default: `WARN`)

For the pre-configured loggers, you can directly configure the log level.
For example:

[source,yaml,subs="+quotes,attributes"]
----
apiVersion: {KafkaApiVersion}
kind: KafkaBridge
spec:
  # ...
  logging:
    type: inline
    loggers:
      rootLogger.level: INFO
      logger.ready.level: DEBUG
  # ...
----

You can also create custom loggers for the other operations based on HTTP API endpoints:

* `createConsumer`
* `deleteConsumer`
* `subscribe`
* `unsubscribe`
* `poll`
* `assign`
* `commit`
* `send`
* `sendToPartition`
* `seekToBeginning`
* `seekToEnd`
* `seek`
* `openapi`

Each operation has a corresponding API endpoint, defined according to the OpenAPI specification, allowing fine-grained logging of HTTP requests.
Configure each logger by assigning it a name as `http.openapi.operation.<operation_id>`. 
For example, to set the logging level for the send operation logger:

[source,yaml,subs="+quotes,attributes"]
----
apiVersion: {KafkaApiVersion}
kind: KafkaBridge
spec:
  # ...
  logging:
    type: inline
    loggers:
      logger.send.name: "http.openapi.operation.send"
      logger.send.level: DEBUG
  # ...
----

Signed-off-by: prmellor <pmellor@redhat.com>
Signed-off-by: prmellor <pmellor@redhat.com>
Comment on lines +125 to +134
* `rootLogger`
* `kafka`
* `orgapachekafka`
* `requestlogger`
* `requestchannel`
* `requestchannel`
* `controller`
* `logcleaner`
* `statechange`
* `authorizer`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly to #11254 (comment), we should clarify which packages / classes do these loggers cover as it is not completely obvious from their names.

Comment on lines +153 to +161
rootLogger.level: INFO
logger.transaction.name: logger.kafka.coordinator.transaction
logger.transaction.level: TRACE
logger.cleaner.name: logger.kafka.log.LogCleanerManager
logger.cleaner.level: DEBUG
logger.request.name: logger.kafka.request.logger
logger.request.level: DEBUG
logger.oauth.name: logger.io.strimzi.kafka.oauth
logger.oauth.level: DEBUG
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The names should not the loggers. We should also use the predefined loggers here where possible.

Suggested change
rootLogger.level: INFO
logger.transaction.name: logger.kafka.coordinator.transaction
logger.transaction.level: TRACE
logger.cleaner.name: logger.kafka.log.LogCleanerManager
logger.cleaner.level: DEBUG
logger.request.name: logger.kafka.request.logger
logger.request.level: DEBUG
logger.oauth.name: logger.io.strimzi.kafka.oauth
logger.oauth.level: DEBUG
rootLogger.level: INFO
logger.logcleaner.level: DEBUG
logger.requestlogger.level: DEBUG
logger.transaction.name: kafka.coordinator.transaction
logger.transaction.level: TRACE
logger.oauth.name: io.strimzi.kafka.oauth
logger.oauth.level: DEBUG

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants