You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2021/09/08 07:38:19 UTC
[GitHub] [pulsar] ivankelly opened a new pull request #11963: [Issue #11799][logging] PIP-89: Enum based logging
ivankelly opened a new pull request #11963:
URL: https://github.com/apache/pulsar/pull/11963
Log an enum constant rather than a raw string. This allows component
annotation and javadoc to be attached to the log event.
--
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@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] lhotari commented on a change in pull request #11963: [Issue #11799][logging] PIP-89: Enum based logging
Posted by GitBox <gi...@apache.org>.
lhotari commented on a change in pull request #11963:
URL: https://github.com/apache/pulsar/pull/11963#discussion_r704146074
##########
File path: structured-event-log/src/main/java/org/apache/pulsar/structuredeventlog/slf4j/Slf4jEvent.java
##########
@@ -160,7 +166,15 @@ public void log(String event) {
MDC.put("startTimestamp", startTime.toString());
MDC.put("durationMs", String.valueOf(Duration.between(startTime, clock.instant()).toMillis()));
}
- Logger logger = LoggerFactory.getLogger("stevlog");
+ Logger logger;
+ if (component != null) {
+ MDC.put("component", component);
+ logger = LoggerFactory.getLogger(
+ new StringBuilder("stevlog.").append(component)
+ .append(".").append(event).toString());
Review comment:
If this part of the code is a hotspot, it would be better if the logging code is mostly "garbage free". Would it be possible to refactor this so that the logger name isn't created for each call and looked up?
--
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@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] ivankelly merged pull request #11963: [Issue #11799][logging] PIP-89: Enum based logging
Posted by GitBox <gi...@apache.org>.
ivankelly merged pull request #11963:
URL: https://github.com/apache/pulsar/pull/11963
--
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@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] lhotari commented on a change in pull request #11963: [Issue #11799][logging] PIP-89: Enum based logging
Posted by GitBox <gi...@apache.org>.
lhotari commented on a change in pull request #11963:
URL: https://github.com/apache/pulsar/pull/11963#discussion_r704244610
##########
File path: structured-event-log/src/main/java/org/apache/pulsar/structuredeventlog/slf4j/Slf4jEvent.java
##########
@@ -160,7 +166,15 @@ public void log(String event) {
MDC.put("startTimestamp", startTime.toString());
MDC.put("durationMs", String.valueOf(Duration.between(startTime, clock.instant()).toMillis()));
}
- Logger logger = LoggerFactory.getLogger("stevlog");
+ Logger logger;
+ if (component != null) {
+ MDC.put("component", component);
+ logger = LoggerFactory.getLogger(
+ new StringBuilder("stevlog.").append(component)
+ .append(".").append(event).toString());
Review comment:
Elegant solution!
--
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@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] ivankelly commented on a change in pull request #11963: [Issue #11799][logging] PIP-89: Enum based logging
Posted by GitBox <gi...@apache.org>.
ivankelly commented on a change in pull request #11963:
URL: https://github.com/apache/pulsar/pull/11963#discussion_r704194954
##########
File path: structured-event-log/src/main/java/org/apache/pulsar/structuredeventlog/slf4j/Slf4jEvent.java
##########
@@ -160,7 +166,15 @@ public void log(String event) {
MDC.put("startTimestamp", startTime.toString());
MDC.put("durationMs", String.valueOf(Duration.between(startTime, clock.instant()).toMillis()));
}
- Logger logger = LoggerFactory.getLogger("stevlog");
+ Logger logger;
+ if (component != null) {
+ MDC.put("component", component);
+ logger = LoggerFactory.getLogger(
+ new StringBuilder("stevlog.").append(component)
+ .append(".").append(event).toString());
Review comment:
will change it to use a hashmap in TLS
--
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@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] ivankelly commented on a change in pull request #11963: [Issue #11799][logging] PIP-89: Enum based logging
Posted by GitBox <gi...@apache.org>.
ivankelly commented on a change in pull request #11963:
URL: https://github.com/apache/pulsar/pull/11963#discussion_r704237307
##########
File path: structured-event-log/src/main/java/org/apache/pulsar/structuredeventlog/slf4j/Slf4jEvent.java
##########
@@ -160,7 +166,15 @@ public void log(String event) {
MDC.put("startTimestamp", startTime.toString());
MDC.put("durationMs", String.valueOf(Duration.between(startTime, clock.instant()).toMillis()));
}
- Logger logger = LoggerFactory.getLogger("stevlog");
+ Logger logger;
+ if (component != null) {
+ MDC.put("component", component);
+ logger = LoggerFactory.getLogger(
+ new StringBuilder("stevlog.").append(component)
+ .append(".").append(event).toString());
Review comment:
@lhotari new change up
--
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@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] eolivelli commented on a change in pull request #11963: [Issue #11799][logging] PIP-89: Enum based logging
Posted by GitBox <gi...@apache.org>.
eolivelli commented on a change in pull request #11963:
URL: https://github.com/apache/pulsar/pull/11963#discussion_r704297313
##########
File path: structured-event-log/src/main/java/org/apache/pulsar/structuredeventlog/slf4j/Slf4jEvent.java
##########
@@ -36,6 +39,9 @@
import org.slf4j.LoggerFactory;
class Slf4jEvent implements Event {
+ private static final ThreadLocal<Map<Object, Logger>> loggersTLS = ThreadLocal.withInitial(() -> new HashMap<>());
+ private static final Logger stringLogger = LoggerFactory.getLogger("stevlog");
Review comment:
what about `org.apache.pulsar.structured.event.log` ?
it is better that the logger name starts with "org.apache.pulsar", in order to let the users shutdown Pulsar stuff buy using only "org.apache.pulsar" prefix
--
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@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org