You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@logging.apache.org by flowcont <gi...@git.apache.org> on 2017/09/30 17:26:54 UTC
[GitHub] logging-log4j2 pull request #112: LOG4J2-2062 Added property to KafkaAppende...
GitHub user flowcont opened a pull request:
https://github.com/apache/logging-log4j2/pull/112
LOG4J2-2062 Added property to KafkaAppender to send a Key to Kafka
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/flowcont/logging-log4j2 feature/LOG4J2-2062
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/logging-log4j2/pull/112.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #112
----
commit 9d3795005355efa238233603995f55000fdcab39
Author: Flowcont <jo...@gmail.com>
Date: 2017-09-30T17:18:47Z
LOG4J2-2062 Added property to KafkaAppender to send a Key to Kafka
----
---
[GitHub] logging-log4j2 pull request #112: LOG4J2-2062 Added property to KafkaAppende...
Posted by mikaelstaldal <gi...@git.apache.org>.
Github user mikaelstaldal commented on a diff in the pull request:
https://github.com/apache/logging-log4j2/pull/112#discussion_r142995363
--- Diff: log4j-core/src/test/java/org/apache/logging/log4j/core/appender/mom/kafka/KafkaAppenderTest.java ---
@@ -135,6 +135,22 @@ public void testAsyncAppend() throws Exception {
assertEquals(LOG_MESSAGE, new String(item.value(), StandardCharsets.UTF_8));
}
+ @Test
+ public void testAppendWithKey() throws Exception {
+ final Appender appender = ctx.getRequiredAppender("KafkaAppenderWithKey");
+ final LogEvent logEvent = createLogEvent();
+ appender.append(logEvent);
+ final List<ProducerRecord<byte[], byte[]>> history = kafka.history();
+ assertEquals(1, history.size());
+ final ProducerRecord<byte[], byte[]> item = history.get(0);
+ assertNotNull(item);
+ assertEquals(TOPIC_NAME, item.topic());
+ String msgKey = item.key().toString();
+ byte[] keyValue = "key".getBytes();
--- End diff --
Specify same charset here also.
---
[GitHub] logging-log4j2 issue #112: LOG4J2-2062 Added property to KafkaAppender to se...
Posted by mikaelstaldal <gi...@git.apache.org>.
Github user mikaelstaldal commented on the issue:
https://github.com/apache/logging-log4j2/pull/112
And don't forget to update documentation in `appenders.xml`.
---
[GitHub] logging-log4j2 pull request #112: LOG4J2-2062 Added property to KafkaAppende...
Posted by mikaelstaldal <gi...@git.apache.org>.
Github user mikaelstaldal commented on a diff in the pull request:
https://github.com/apache/logging-log4j2/pull/112#discussion_r142995940
--- Diff: src/site/xdoc/manual/appenders.xml ---
@@ -1735,6 +1735,11 @@ public class JpaLogEntity extends AbstractLogEventWrapperEntity {
<td>The Kafka topic to use. Required.</td>
</tr>
<tr>
+ <td>key</td>
+ <td>String</td>
+ <td>The key that will be sent to Kafka with every message.</td>
--- End diff --
Should probably mention that it's optional, and what default is.
---
[GitHub] logging-log4j2 pull request #112: LOG4J2-2062 Added property to KafkaAppende...
Posted by Abrasha <gi...@git.apache.org>.
Github user Abrasha commented on a diff in the pull request:
https://github.com/apache/logging-log4j2/pull/112#discussion_r143281153
--- Diff: log4j-core/src/main/java/org/apache/logging/log4j/core/appender/mom/kafka/KafkaManager.java ---
@@ -47,18 +47,24 @@
private final int timeoutMillis;
private final String topic;
+ private final byte[] key;
private final boolean syncSend;
- public KafkaManager(final LoggerContext loggerContext, final String name, final String topic, final boolean syncSend, final Property[] properties) {
+ public KafkaManager(final LoggerContext loggerContext, final String name, final String topic, final boolean syncSend,
+ final Property[] properties, final String key) {
super(loggerContext, name);
this.topic = Objects.requireNonNull(topic, "topic");
this.syncSend = syncSend;
config.setProperty("key.serializer", "org.apache.kafka.common.serialization.ByteArraySerializer");
config.setProperty("value.serializer", "org.apache.kafka.common.serialization.ByteArraySerializer");
config.setProperty("batch.size", "0");
for (final Property property : properties) {
+
config.setProperty(property.getName(), property.getValue());
}
+
+ this.key = (key != null ) ? key.getBytes() : null ;
--- End diff --
For example: `java.nio.charset.StandardCharsets.UTF_8`
---
[GitHub] logging-log4j2 pull request #112: LOG4J2-2062 Added property to KafkaAppende...
Posted by mikaelstaldal <gi...@git.apache.org>.
Github user mikaelstaldal commented on a diff in the pull request:
https://github.com/apache/logging-log4j2/pull/112#discussion_r142996203
--- Diff: src/site/xdoc/manual/appenders.xml ---
@@ -1735,6 +1735,11 @@ public class JpaLogEntity extends AbstractLogEventWrapperEntity {
<td>The Kafka topic to use. Required.</td>
</tr>
<tr>
+ <td>key</td>
+ <td>String</td>
+ <td>The key that will be sent to Kafka with every message.</td>
+ </tr>
+ <tr>
--- End diff --
We should add a comment on the `properties` section to not specify `key.serializer` (as well as not specify `value.serializer`).
---
[GitHub] logging-log4j2 issue #112: LOG4J2-2062 Added property to KafkaAppender to se...
Posted by mikaelstaldal <gi...@git.apache.org>.
Github user mikaelstaldal commented on the issue:
https://github.com/apache/logging-log4j2/pull/112
:+1:
---
[GitHub] logging-log4j2 pull request #112: LOG4J2-2062 Added property to KafkaAppende...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/logging-log4j2/pull/112
---
[GitHub] logging-log4j2 pull request #112: LOG4J2-2062 Added property to KafkaAppende...
Posted by mikaelstaldal <gi...@git.apache.org>.
Github user mikaelstaldal commented on a diff in the pull request:
https://github.com/apache/logging-log4j2/pull/112#discussion_r142994778
--- Diff: log4j-core/src/main/java/org/apache/logging/log4j/core/appender/mom/kafka/KafkaManager.java ---
@@ -47,18 +47,24 @@
private final int timeoutMillis;
private final String topic;
+ private final byte[] key;
private final boolean syncSend;
- public KafkaManager(final LoggerContext loggerContext, final String name, final String topic, final boolean syncSend, final Property[] properties) {
+ public KafkaManager(final LoggerContext loggerContext, final String name, final String topic, final boolean syncSend,
+ final Property[] properties, final String key) {
super(loggerContext, name);
this.topic = Objects.requireNonNull(topic, "topic");
this.syncSend = syncSend;
config.setProperty("key.serializer", "org.apache.kafka.common.serialization.ByteArraySerializer");
config.setProperty("value.serializer", "org.apache.kafka.common.serialization.ByteArraySerializer");
config.setProperty("batch.size", "0");
for (final Property property : properties) {
+
config.setProperty(property.getName(), property.getValue());
}
+
+ this.key = (key != null ) ? key.getBytes() : null ;
--- End diff --
`key.getBytes()` will use some platform default charset. It's not good to rely on that, better to specify charset explicitly.
---
[GitHub] logging-log4j2 issue #112: LOG4J2-2062 Added property to KafkaAppender to se...
Posted by mikaelstaldal <gi...@git.apache.org>.
Github user mikaelstaldal commented on the issue:
https://github.com/apache/logging-log4j2/pull/112
It would also be useful to enable interpolation with [lookups](https://logging.apache.org/log4j/2.x/manual/lookups.html) for the key, so that you could e.g. use some value from the thread context:
```
<Kafka name="Kafka" topic="log-test" key="$${ctx:key}">
```
See e.g. [here](https://github.com/apache/logging-log4j2/blob/master/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/HttpURLConnectionManager.java#L95) about how to do that.
---
[GitHub] logging-log4j2 issue #112: LOG4J2-2062 Added property to KafkaAppender to se...
Posted by mikaelstaldal <gi...@git.apache.org>.
Github user mikaelstaldal commented on the issue:
https://github.com/apache/logging-log4j2/pull/112
@flowcont I think it would make more sense to have the value for the key as an attribute on the Kafka appender element, and not as a nested property, since the nested properties are passed directly to the [Kafka client library](http://kafka.apache.org/documentation.html#producerconfigs).
So you can specify it like this:
```
<Appenders>
<Kafka name="Kafka" topic="log-test" key="myKey">
<PatternLayout pattern="%date %message"/>
<Property name="bootstrap.servers">localhost:9092</Property>
</Kafka>
</Appenders>
```
---