You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by "suneet-s (via GitHub)" <gi...@apache.org> on 2023/04/20 01:48:50 UTC
[GitHub] [druid] suneet-s opened a new pull request, #14121: Make LoggingEmitter more useful by using Markers
suneet-s opened a new pull request, #14121:
URL: https://github.com/apache/druid/pull/14121
### Description
This change makes the events emitted by the logging emitter more useful by logging them with the feed of the event as a
log4j Marker.
This PR updates Druid's `Logger` interface to include functions that allows developers to mark log messages with a Marker, and then makes use of them in the `LoggingEmitter`.
The output from the logging emitter with the default Druid configuration will now look like
```
2023-04-19T23:54:51,121 INFO [qtp52562984-77] org.apache.druid.java.util.emitter.core.LoggingEmitter - [request_logs] {"feed":"request_logs","service":"druid/router","query":null,"host":"localhost:8888","sql":"SELECT 1337","queryStats":{"query/time":537,"success":true},"remoteAddr":"[0:0:0:0:0:0:0:1]","sqlQueryContext":{"timeout":15000,"sqlQueryId":"241a044f-da54-446d-8c16-6b62cace3247","queryId":"241a044f-da54-446d-8c16-6b62cace3247"},"timestamp":"2023-04-19T23:54:51.118Z"}
2023-04-19T23:49:13,336 INFO [MonitorScheduler-0] org.apache.druid.java.util.emitter.core.LoggingEmitter - [metrics] {"feed":"metrics","metric":"jetty/threadPool/busy","service":"druid/router","host":"localhost:8888","version":"27.0.0-SNAPSHOT","value":4,"timestamp":"2023-04-19T23:49:13.336Z"}
```
Druid operators can now configure their log4j config to route the logs emitted to different locations based on their use. For example, an operator might want to configure different feeds to be written to different files. They can now do this with config that looks like
In Druid common.runtime.properties
```
druid.emitter=logging
druid.emitter.logging.logLevel=info
# configure request logging via the emitter
druid.request.logging.type=emitter
druid.request.logging.feed=request_logs
```
Then update the log4j.xml (by default in `_common/log4j2.xml`) with a new appender and route the logs based on their feeds
```
<Appenders>
... // Other appender configs
<File name="emitterAppender" fileName="/logs/druid/emitter/%markerSimpleName.log">
<PatternLayout pattern="%d{ISO8601} - %m%n"/>
</File>
</Appenders>
<Loggers>
... // Other Logger configs
<Logger name="org.apache.druid.java.util.emitter.core.LoggingEmitter" level="info" additivity="false">
<Appender-ref ref="emitterAppender"/>
</Logger>
</Loggers>
```
#### Release note
<!-- Give your best effort to summarize your changes in a couple of sentences aimed toward Druid users.
If your change doesn't have end user impact, you can skip this section.
For tips about how to write a good release note, see [Release notes](https://github.com/apache/druid/blob/master/CONTRIBUTING.md#release-notes).
-->
<hr>
##### Key changed/added classes in this PR
* `*/log4j2.xml` - This changes the default log4j config to add Markers to be included in the log if it is specified in the code.
* `processing/src/main/java/org/apache/druid/java/util/common/logger/Logger.java`
<hr>
This PR has:
- [ ] been self-reviewed.
- [ ] using the [concurrency checklist](https://github.com/apache/druid/blob/master/dev/code-review/concurrency.md) (Remove this item if the PR doesn't have any relation to concurrency.)
- [ ] added documentation for new or modified features or behaviors.
- [ ] a release note entry in the PR description.
- [ ] added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
- [ ] added or updated version, license, or notice information in [licenses.yaml](https://github.com/apache/druid/blob/master/dev/license.md)
- [ ] added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
- [ ] added unit tests or modified existing tests to cover new code paths, ensuring the threshold for [code coverage](https://github.com/apache/druid/blob/master/dev/code-review/code-coverage.md) is met.
- [ ] added integration tests.
- [x] been tested in a test Druid cluster.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] suneet-s commented on a diff in pull request #14121: Make LoggingEmitter more useful by using Markers
Posted by "suneet-s (via GitHub)" <gi...@apache.org>.
suneet-s commented on code in PR #14121:
URL: https://github.com/apache/druid/pull/14121#discussion_r1176631927
##########
docs/configuration/index.md:
##########
@@ -422,6 +422,12 @@ There are several emitters available:
##### Logging Emitter Module
+The use this emitter module, set `druid.emitter=logging`. The Logging emitter will use the configured
+`druid.emitter.logging.loggerClass` to emit events. If the `loggerClass` is set to `LoggingEmitter` (the default), the
Review Comment:
The thing that threw me off in the original docs was the fact that the choices for the logger class was only other emitter classes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] gianm commented on a diff in pull request #14121: Make LoggingEmitter more useful by using Markers
Posted by "gianm (via GitHub)" <gi...@apache.org>.
gianm commented on code in PR #14121:
URL: https://github.com/apache/druid/pull/14121#discussion_r1176022455
##########
extensions-core/azure-extensions/pom.xml:
##########
@@ -159,7 +159,7 @@
<plugin>
<groupId>org.jacoco</groupId>
<artifactId>jacoco-maven-plugin</artifactId>
- <version>0.8.4</version>
+ <version>${jacoco.version}</version>
Review Comment:
IIRC, if the `<version>` is in `<pluginManagement>` in the main pom, then the line doesn't need to be present at all here.
##########
docs/configuration/index.md:
##########
@@ -422,6 +422,12 @@ There are several emitters available:
##### Logging Emitter Module
+The use this emitter module, set `druid.emitter=logging`. The Logging emitter will use the configured
+`druid.emitter.logging.loggerClass` to emit events. If the `loggerClass` is set to `LoggingEmitter` (the default), the
Review Comment:
> The Logging emitter will use the configured `druid.emitter.logging.loggerClass` to emit events. If the `loggerClass` is set to `LoggingEmitter` (the default), the events are logged as a single `json` object with a Marker…
All the `druid.emitter.logging.loggerClass` does is control the package name of the logger (what gets printed for `%c` in the pattern). The new doc line here makes it sound like it does more than that. Suggest something like:
> The `logging` emitter uses a log4j2 logger named `druid.emitter.logging.loggerClass` to emit events. Each event is logged as a single `json` object with a Marker…
Also, from looking at the code, I believe the existing doc is wrong: the default is `org.apache.druid.java.util.emitter.core.LoggingEmitter` not `LoggingEmitter`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] suneet-s commented on a diff in pull request #14121: Make LoggingEmitter more useful by using Markers
Posted by "suneet-s (via GitHub)" <gi...@apache.org>.
suneet-s commented on code in PR #14121:
URL: https://github.com/apache/druid/pull/14121#discussion_r1176572210
##########
extensions-core/azure-extensions/pom.xml:
##########
@@ -159,7 +159,7 @@
<plugin>
<groupId>org.jacoco</groupId>
<artifactId>jacoco-maven-plugin</artifactId>
- <version>0.8.4</version>
+ <version>${jacoco.version}</version>
Review Comment:
Removed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] gianm merged pull request #14121: Make LoggingEmitter more useful by using Markers
Posted by "gianm (via GitHub)" <gi...@apache.org>.
gianm merged PR #14121:
URL: https://github.com/apache/druid/pull/14121
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org