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 2022/02/10 03:41:20 UTC
[GitHub] [pulsar] ethqunzhong opened a new pull request #14200: Broker: support dynamic update log level at runtime
ethqunzhong opened a new pull request #14200:
URL: https://github.com/apache/pulsar/pull/14200
### Motivation
As default, Broker log level set by `log4j2.yaml` with `pulsar.log.level=info`, if developers want to update log level to debug, need to Modify `log42j.yaml#pulsar.log.level=debug` and then restart Broker service.
This PR support developers dynamic update Broker log level at runtime,It is very useful for online trouble shooting without having to stop Broker service and start it again.
### Modifications
- add admin cli (`bin/pulsar-admin brokers update-logger-level --classname ${CLASSNAME} --level ${LEVEL}`)
- add rest endpoints (`/admin/v2/brokers/log4j/{classname}/{level}`)
### Verifying this change
- Make sure that the change passes the CI checks.
* This change added tests and can be verified as follows:`org.apache.pulsar.admin.cli.PulsarAdminToolTest#brokers`
* works as expected on product environment.
### Does this pull request potentially affect one of the following parts:
*If `yes` was chosen, please highlight the changes*
- Dependencies (does it add or upgrade a dependency): (no)
- The public API: (no)
- The schema: (no)
- The default values of configurations: (no)
- The wire protocol: (no)
- The rest endpoints: (yes)
- The admin cli options: (yes)
- Anything that affects deployment: (no)
### Documentation
- [x] `no-need-doc`
--
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] ethqunzhong commented on pull request #14200: Broker: support dynamic update log level at runtime
Posted by GitBox <gi...@apache.org>.
ethqunzhong commented on pull request #14200:
URL: https://github.com/apache/pulsar/pull/14200#issuecomment-1047651738
> IIRC log4j supports automatic reloading of configuration files. Isn't it enough for you?
yes, Log4j has the ability to automatically configure itself during initialization and reload config while config files changed.
but it is more convenient for developers update logger level by programmatically way.
--
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] Jason918 commented on a change in pull request #14200: Broker: support dynamic update log level at runtime
Posted by GitBox <gi...@apache.org>.
Jason918 commented on a change in pull request #14200:
URL: https://github.com/apache/pulsar/pull/14200#discussion_r806573736
##########
File path: conf/log4j2.yaml
##########
@@ -134,7 +134,6 @@ Configuration:
additivity: true
AppenderRef:
- ref: "${sys:pulsar.log.appender}"
- level: "${sys:pulsar.log.level}"
Review comment:
User may already have set `pulsar.log.level=warn` in their deployment. Will this PR change the settings?
--
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] ethqunzhong commented on a change in pull request #14200: Broker: support dynamic update log level at runtime
Posted by GitBox <gi...@apache.org>.
ethqunzhong commented on a change in pull request #14200:
URL: https://github.com/apache/pulsar/pull/14200#discussion_r806730706
##########
File path: conf/log4j2.yaml
##########
@@ -134,7 +134,6 @@ Configuration:
additivity: true
AppenderRef:
- ref: "${sys:pulsar.log.appender}"
- level: "${sys:pulsar.log.level}"
Review comment:
i have set `pulsar.log.level=warn` for test. but not what i expected. described as this issue #14298
--
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] codelipenghui commented on a change in pull request #14200: Broker: support dynamic update log level at runtime
Posted by GitBox <gi...@apache.org>.
codelipenghui commented on a change in pull request #14200:
URL: https://github.com/apache/pulsar/pull/14200#discussion_r830831556
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/BrokersBase.java
##########
@@ -437,5 +440,62 @@ private void doShutDownBrokerGracefully(int maxConcurrentUnloadPerSec,
pulsar().getBrokerService().unloadNamespaceBundlesGracefully(maxConcurrentUnloadPerSec, forcedTerminateTopic);
pulsar().closeAsync();
}
+
+ /**
+ * dynamically update logger level at runtime.
+ *
+ * @param targetClassName : class name to update
+ * @param targetLevel : target log level
+ */
+ private CompletableFuture<Void> internalUpdateLoggerLevelAsync(String targetClassName, String targetLevel) {
+ CompletableFuture<Void> loggerLevelFuture = new CompletableFuture<>();
+ try {
+ String className;
+ // if set "ROOT" will take effect to rootLogger
+ if (targetClassName.trim().equalsIgnoreCase("ROOT")) {
+ className = LogManager.ROOT_LOGGER_NAME;
+ } else {
+ className = targetClassName;
+ }
+ Level newLevel;
+ try {
+ newLevel = Level.valueOf(targetLevel);
+ Configurator.setAllLevels(className, newLevel);
+ } catch (IllegalArgumentException | NullPointerException e) {
+ // Unknown Log Level or NULL
+ throw new RestException(Status.PRECONDITION_FAILED, "Invalid logger level.");
+ }
+ } catch (RestException re) {
+ throw re;
+ } catch (Exception ie) {
+ throw new RestException(Status.PRECONDITION_FAILED, "Internal error.");
+ }
+ return CompletableFuture.completedFuture(null);
+ }
+
+ @POST
+ @Path("/logging/{classname}/{level}")
+ @ApiOperation(value =
+ "dynamic update logger level at runtime. This operation requires Pulsar super-user privileges.")
+ @ApiResponses(value = {
+ @ApiResponse(code = 204, message = "class logger level updated successfully"),
+ @ApiResponse(code = 403, message = "You don't have admin permission to update logger level."),
+ @ApiResponse(code = 500, message = "Internal server error")})
+ public void updateLoggerLevelDynamically(@Suspended final AsyncResponse asyncResponse,
Review comment:
Is it able to provide a get method to get the current applied log levels?
##########
File path: pulsar-client-admin-api/src/main/java/org/apache/pulsar/client/admin/Brokers.java
##########
@@ -304,4 +304,18 @@
* @return version of broker.
*/
String getVersion() throws PulsarAdminException;
+
+ /**
+ * dynamically update logger level in runtime.
+ *
+ * @throws PulsarAdminException if update logger level failed.
+ */
+ void updateLoggerLevel(String classname, String level) throws PulsarAdminException;
+
+ /**
+ * Reset logger level dynamically in runtime asynchronously.
+ * @return CompletableFuture
+ */
+ CompletableFuture<Void> updateLoggerLevelAsync(String classname, String level);
Review comment:
```suggestion
CompletableFuture<Void> updateLoggerLevelAsync(String loggerName, String level);
```
##########
File path: pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdBrokers.java
##########
@@ -176,6 +176,20 @@ void run() throws Exception {
}
}
+ @Parameters(commandDescription = "Dynamic update logger level in runtime by classname.")
+ private class UpdateLoggerLevelCmd extends CliCommand {
+ @Parameter(names = {"-c", "--classname"}, description =
+ "The except class name,if set \"ROOT\" will take effect to rootLogger", required = true)
+ private String classname;
Review comment:
```suggestion
private String loggerName;
```
##########
File path: conf/log4j2.yaml
##########
@@ -134,7 +134,6 @@ Configuration:
additivity: true
AppenderRef:
- ref: "${sys:pulsar.log.appender}"
- level: "${sys:pulsar.log.level}"
Review comment:
> Can you explain from the code, why pulsar.log.level this system env could still take effect after we delete from log4j2.yaml? Did we pass this variable to log4j somewhere else?
+1, from the source code here https://github.com/apache/pulsar/blob/master/bin/pulsar#L299, I think we can't remove this line here.
##########
File path: pulsar-client-admin-api/src/main/java/org/apache/pulsar/client/admin/Brokers.java
##########
@@ -304,4 +304,18 @@
* @return version of broker.
*/
String getVersion() throws PulsarAdminException;
+
+ /**
+ * dynamically update logger level in runtime.
+ *
+ * @throws PulsarAdminException if update logger level failed.
+ */
+ void updateLoggerLevel(String classname, String level) throws PulsarAdminException;
Review comment:
```suggestion
void updateLoggerLevel(String loggerName, String level) throws PulsarAdminException;
```
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/BrokersBase.java
##########
@@ -437,5 +440,62 @@ private void doShutDownBrokerGracefully(int maxConcurrentUnloadPerSec,
pulsar().getBrokerService().unloadNamespaceBundlesGracefully(maxConcurrentUnloadPerSec, forcedTerminateTopic);
pulsar().closeAsync();
}
+
+ /**
+ * dynamically update logger level at runtime.
+ *
+ * @param targetClassName : class name to update
+ * @param targetLevel : target log level
+ */
+ private CompletableFuture<Void> internalUpdateLoggerLevelAsync(String targetClassName, String targetLevel) {
+ CompletableFuture<Void> loggerLevelFuture = new CompletableFuture<>();
+ try {
+ String className;
+ // if set "ROOT" will take effect to rootLogger
+ if (targetClassName.trim().equalsIgnoreCase("ROOT")) {
+ className = LogManager.ROOT_LOGGER_NAME;
+ } else {
+ className = targetClassName;
+ }
+ Level newLevel;
+ try {
+ newLevel = Level.valueOf(targetLevel);
+ Configurator.setAllLevels(className, newLevel);
+ } catch (IllegalArgumentException | NullPointerException e) {
+ // Unknown Log Level or NULL
+ throw new RestException(Status.PRECONDITION_FAILED, "Invalid logger level.");
+ }
+ } catch (RestException re) {
+ throw re;
+ } catch (Exception ie) {
+ throw new RestException(Status.PRECONDITION_FAILED, "Internal error.");
+ }
+ return CompletableFuture.completedFuture(null);
+ }
+
+ @POST
+ @Path("/logging/{classname}/{level}")
+ @ApiOperation(value =
+ "dynamic update logger level at runtime. This operation requires Pulsar super-user privileges.")
+ @ApiResponses(value = {
+ @ApiResponse(code = 204, message = "class logger level updated successfully"),
+ @ApiResponse(code = 403, message = "You don't have admin permission to update logger level."),
+ @ApiResponse(code = 500, message = "Internal server error")})
+ public void updateLoggerLevelDynamically(@Suspended final AsyncResponse asyncResponse,
+ @PathParam("classname") String classname,
Review comment:
It should be the logger name here, not the class name. Usually, we will use the class name as the logger name, but not 100%
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/BrokersBase.java
##########
@@ -437,5 +441,67 @@ private void doShutDownBrokerGracefully(int maxConcurrentUnloadPerSec,
pulsar().getBrokerService().unloadNamespaceBundlesGracefully(maxConcurrentUnloadPerSec, forcedTerminateTopic);
pulsar().closeAsync();
}
+
+ /**
+ * dynamically update log4j2 logger level in runtime.
+ *
+ * @param targetClassName
+ * @param targetLevel
+ */
+ private synchronized void updateLoggerLevel(String targetClassName, String targetLevel) {
+ try {
+ String className;
+ if (targetClassName.trim().equalsIgnoreCase("ROOT")) {
Review comment:
> Please add this usage in command description.
+1
##########
File path: pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdBrokers.java
##########
@@ -176,6 +176,20 @@ void run() throws Exception {
}
}
+ @Parameters(commandDescription = "Dynamic update logger level in runtime by classname.")
+ private class UpdateLoggerLevelCmd extends CliCommand {
+ @Parameter(names = {"-c", "--classname"}, description =
Review comment:
```suggestion
@Parameter(names = {"-l", "--logger-name"}, description =
```
--
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] ethqunzhong commented on a change in pull request #14200: Broker: support dynamic update log level at runtime
Posted by GitBox <gi...@apache.org>.
ethqunzhong commented on a change in pull request #14200:
URL: https://github.com/apache/pulsar/pull/14200#discussion_r807897213
##########
File path: conf/log4j2.yaml
##########
@@ -134,7 +134,6 @@ Configuration:
additivity: true
AppenderRef:
- ref: "${sys:pulsar.log.appender}"
- level: "${sys:pulsar.log.level}"
Review comment:
> User may already have set `pulsar.log.level=warn` in their deployment. Will this PR change the settings?
i have a test
if User set `pulsar.log.level=warn` in their deployment while starting service.
this PR will not change the settings , all loggers's level is `warn`, meanwhile , the PR featrue also works.
> ~~level: "${sys:pulsar.log.level}"~~
this line config is redundant and will disturb the programmatically way.
--
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] ethqunzhong commented on a change in pull request #14200: Broker: support dynamic update log level at runtime
Posted by GitBox <gi...@apache.org>.
ethqunzhong commented on a change in pull request #14200:
URL: https://github.com/apache/pulsar/pull/14200#discussion_r807648147
##########
File path: pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdBrokers.java
##########
@@ -176,6 +176,20 @@ void run() throws Exception {
}
}
+ @Parameters(commandDescription = "Update dynamic log4j2 logger level in runtime by classname.")
+ private class UpdateLoggerLevelCmd extends CliCommand {
Review comment:
🤔 Maybe not.
in product environment, `authenticationEnabled = true` as usual ,so only developers or admin can do this.
in addition, user call this request will mark the broker ip in log as follow:
<img width="1075" alt="image" src="https://user-images.githubusercontent.com/16517186/154221713-efb4d2d2-5a79-4d47-adf6-514fdf70c957.png">
--
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] Jason918 commented on a change in pull request #14200: Broker: support dynamic update log level at runtime
Posted by GitBox <gi...@apache.org>.
Jason918 commented on a change in pull request #14200:
URL: https://github.com/apache/pulsar/pull/14200#discussion_r812525236
##########
File path: conf/log4j2.yaml
##########
@@ -134,7 +134,6 @@ Configuration:
additivity: true
AppenderRef:
- ref: "${sys:pulsar.log.appender}"
- level: "${sys:pulsar.log.level}"
Review comment:
Can you explain from the code, why `pulsar.log.level` this system env could still take effect after we delete from log4j2.yaml? Did we pass this variable to log4j somewhere else?
--
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] Jason918 commented on a change in pull request #14200: Broker: support dynamic update log level at runtime
Posted by GitBox <gi...@apache.org>.
Jason918 commented on a change in pull request #14200:
URL: https://github.com/apache/pulsar/pull/14200#discussion_r803283040
##########
File path: pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdBrokers.java
##########
@@ -176,6 +176,19 @@ void run() throws Exception {
}
}
+ @Parameters(commandDescription = "Update dynamic log4j2 logger level in runtime by classname.")
+ private class UpdateLoggerLevelCmd extends CliCommand {
+ @Parameter(names = "--classname", description = "The except class name", required = true)
Review comment:
Add "-c" for short?
##########
File path: pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdBrokers.java
##########
@@ -176,6 +176,19 @@ void run() throws Exception {
}
}
+ @Parameters(commandDescription = "Update dynamic log4j2 logger level in runtime by classname.")
+ private class UpdateLoggerLevelCmd extends CliCommand {
+ @Parameter(names = "--classname", description = "The except class name", required = true)
+ private String classname;
+ @Parameter(names = "--level", description = "The target logger level", required = true)
Review comment:
Add "-l" for short?
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/BrokersBase.java
##########
@@ -437,5 +441,67 @@ private void doShutDownBrokerGracefully(int maxConcurrentUnloadPerSec,
pulsar().getBrokerService().unloadNamespaceBundlesGracefully(maxConcurrentUnloadPerSec, forcedTerminateTopic);
pulsar().closeAsync();
}
+
+ /**
+ * dynamically update log4j2 logger level in runtime.
+ *
+ * @param targetClassName
+ * @param targetLevel
+ */
+ private synchronized void updateLoggerLevel(String targetClassName, String targetLevel) {
+ try {
+ String className;
+ if (targetClassName.trim().equalsIgnoreCase("ROOT")) {
Review comment:
Please add this usage in command description.
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/BrokersBase.java
##########
@@ -437,5 +441,67 @@ private void doShutDownBrokerGracefully(int maxConcurrentUnloadPerSec,
pulsar().getBrokerService().unloadNamespaceBundlesGracefully(maxConcurrentUnloadPerSec, forcedTerminateTopic);
pulsar().closeAsync();
}
+
+ /**
+ * dynamically update log4j2 logger level in runtime.
+ *
+ * @param targetClassName
+ * @param targetLevel
+ */
+ private synchronized void updateLoggerLevel(String targetClassName, String targetLevel) {
+ try {
+ String className;
+ if (targetClassName.trim().equalsIgnoreCase("ROOT")) {
+ className = LogManager.ROOT_LOGGER_NAME;
+ } else {
+ try {
+ className = targetClassName.trim();
+ // Check if class name valid
+ Class.forName(className);
+ } catch (ClassNotFoundException e) {
+ // Logger not found
+ throw new RestException(Status.NOT_FOUND, "Logger not found.");
+ }
+ }
+
+ Level level;
+ try {
+ level = Level.valueOf(targetLevel);
+ } catch (IllegalArgumentException e) {
+ // Level not found
+ throw new RestException(Status.PRECONDITION_FAILED, "Invalid logger level.");
+ }
+
+ if (level != null) {
+ Level originLevel = LogManager.getLogger(className).getLevel();
+ Configurator.setAllLevels(className, level);
+ LOG.info("[{}] Successfully update log level for className: {} ({} -> {}.)", clientAppId(), className, originLevel, level);
+ } else {
+ LOG.error("[{}] Failed to update log level for {}", clientAppId(), className);
+ }
+ } catch (RestException re) {
+ LOG.error("[{}] Failed to update log level for className: {}, targetLevel: {} due to rest exception.", clientAppId(), targetClassName, targetLevel);
+ throw re;
+ } catch (Exception ie) {
+ LOG.error("[{}] Failed to update log level for {} to {} due to internal error.",
+ clientAppId(), targetClassName, targetLevel);
+ throw new RestException(ie);
+ }
+ }
+
+ @POST
+ @Path("/log4j/{classname}/{level}")
+ @ApiOperation(value =
+ "update dynamic log4j2 logger level in runtime by classname. This operation requires Pulsar super-user privileges.")
+ @ApiResponses(value = {
+ @ApiResponse(code = 204, message = "class logger level updated successfully"),
+ @ApiResponse(code = 403, message = "You don't have admin permission to update log4j2 logger level."),
+ @ApiResponse(code = 500, message = "Internal server error")})
+ public void updateLoggerLevelDynamically(@PathParam("classname") String classname,
Review comment:
Can you implement this in async?
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/BrokersBase.java
##########
@@ -437,5 +441,67 @@ private void doShutDownBrokerGracefully(int maxConcurrentUnloadPerSec,
pulsar().getBrokerService().unloadNamespaceBundlesGracefully(maxConcurrentUnloadPerSec, forcedTerminateTopic);
pulsar().closeAsync();
}
+
+ /**
+ * dynamically update log4j2 logger level in runtime.
+ *
+ * @param targetClassName
+ * @param targetLevel
+ */
+ private synchronized void updateLoggerLevel(String targetClassName, String targetLevel) {
+ try {
+ String className;
+ if (targetClassName.trim().equalsIgnoreCase("ROOT")) {
+ className = LogManager.ROOT_LOGGER_NAME;
+ } else {
+ try {
+ className = targetClassName.trim();
+ // Check if class name valid
+ Class.forName(className);
+ } catch (ClassNotFoundException e) {
+ // Logger not found
+ throw new RestException(Status.NOT_FOUND, "Logger not found.");
+ }
+ }
+
+ Level level;
+ try {
+ level = Level.valueOf(targetLevel);
+ } catch (IllegalArgumentException e) {
+ // Level not found
+ throw new RestException(Status.PRECONDITION_FAILED, "Invalid logger level.");
+ }
+
+ if (level != null) {
+ Level originLevel = LogManager.getLogger(className).getLevel();
+ Configurator.setAllLevels(className, level);
+ LOG.info("[{}] Successfully update log level for className: {} ({} -> {}.)", clientAppId(), className, originLevel, level);
+ } else {
+ LOG.error("[{}] Failed to update log level for {}", clientAppId(), className);
Review comment:
throw exception for failure?
--
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 #14200: Broker: support dynamic update log level at runtime
Posted by GitBox <gi...@apache.org>.
eolivelli commented on a change in pull request #14200:
URL: https://github.com/apache/pulsar/pull/14200#discussion_r806735706
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/BrokersBase.java
##########
@@ -437,5 +440,79 @@ private void doShutDownBrokerGracefully(int maxConcurrentUnloadPerSec,
pulsar().getBrokerService().unloadNamespaceBundlesGracefully(maxConcurrentUnloadPerSec, forcedTerminateTopic);
pulsar().closeAsync();
}
+
+ /**
+ * dynamically update log4j2 logger level at runtime.
+ *
+ * @param targetClassName : class name to update
+ * @param targetLevel : target log level
+ */
+ private CompletableFuture<Void> internalUpdateLoggerLevelAsync(String targetClassName, String targetLevel) {
+ CompletableFuture<Void> loggerLevelFuture = new CompletableFuture<>();
+ CompletableFuture.runAsync(() -> {
+ try {
+ String className;
+ // if set "ROOT" will take effect to rootLogger
+ if (targetClassName.trim().equalsIgnoreCase("ROOT")) {
+ className = LogManager.ROOT_LOGGER_NAME;
+ } else {
+ try {
+ className = targetClassName.trim();
+ // Check if class name valid
+ Class.forName(className);
Review comment:
Here you aree triggering the load of a class from a APi call.
This may be seen as a security issue.
There is no need to do this.
Please remove
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/BrokersBase.java
##########
@@ -437,5 +440,79 @@ private void doShutDownBrokerGracefully(int maxConcurrentUnloadPerSec,
pulsar().getBrokerService().unloadNamespaceBundlesGracefully(maxConcurrentUnloadPerSec, forcedTerminateTopic);
pulsar().closeAsync();
}
+
+ /**
+ * dynamically update log4j2 logger level at runtime.
+ *
+ * @param targetClassName : class name to update
+ * @param targetLevel : target log level
+ */
+ private CompletableFuture<Void> internalUpdateLoggerLevelAsync(String targetClassName, String targetLevel) {
+ CompletableFuture<Void> loggerLevelFuture = new CompletableFuture<>();
+ CompletableFuture.runAsync(() -> {
Review comment:
Why are we executing this a a unknown thread?
We can execute this code in the main thread of the http handler
##########
File path: pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdBrokers.java
##########
@@ -176,6 +176,20 @@ void run() throws Exception {
}
}
+ @Parameters(commandDescription = "Update dynamic log4j2 logger level in runtime by classname.")
+ private class UpdateLoggerLevelCmd extends CliCommand {
Review comment:
Please note that there is no control about the broker who is serving the request.
Can we report this to the user?
- report the id of the broker that process the request
- write a generic warning
--
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] Jason918 commented on a change in pull request #14200: Broker: support dynamic update log level at runtime
Posted by GitBox <gi...@apache.org>.
Jason918 commented on a change in pull request #14200:
URL: https://github.com/apache/pulsar/pull/14200#discussion_r806573736
##########
File path: conf/log4j2.yaml
##########
@@ -134,7 +134,6 @@ Configuration:
additivity: true
AppenderRef:
- ref: "${sys:pulsar.log.appender}"
- level: "${sys:pulsar.log.level}"
Review comment:
User may already have set `pulsar.log.level=warn` in their deployment. Will this PR change the settings?
--
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] ethqunzhong commented on a change in pull request #14200: Broker: support dynamic update log level at runtime
Posted by GitBox <gi...@apache.org>.
ethqunzhong commented on a change in pull request #14200:
URL: https://github.com/apache/pulsar/pull/14200#discussion_r806730706
##########
File path: conf/log4j2.yaml
##########
@@ -134,7 +134,6 @@ Configuration:
additivity: true
AppenderRef:
- ref: "${sys:pulsar.log.appender}"
- level: "${sys:pulsar.log.level}"
Review comment:
i have set `pulsar.log.level=warn` for test. but not what i expected. described as this issue #14298
**update:** 🤲
i have a test, result as follow:
as log4j2.yaml using way,
`pulsar.log.level` go into effect only set by ENV property `PULSAR_LOG_LEVEL` which set in `pulsar_env.sh` while service starting.
while service running, change `pulsar.log.level` in log4j2.yaml is not work, `pulsar.log.level` is unmodifiable now.
--
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] Jason918 commented on a change in pull request #14200: Broker: support dynamic update log level at runtime
Posted by GitBox <gi...@apache.org>.
Jason918 commented on a change in pull request #14200:
URL: https://github.com/apache/pulsar/pull/14200#discussion_r804406630
##########
File path: conf/log4j2.yaml
##########
@@ -134,7 +134,6 @@ Configuration:
additivity: true
AppenderRef:
- ref: "${sys:pulsar.log.appender}"
- level: "${sys:pulsar.log.level}"
Review comment:
This should not be changed as we should keep default behavior.
--
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] ethqunzhong commented on a change in pull request #14200: Broker: support dynamic update log level at runtime
Posted by GitBox <gi...@apache.org>.
ethqunzhong commented on a change in pull request #14200:
URL: https://github.com/apache/pulsar/pull/14200#discussion_r812518287
##########
File path: conf/log4j2.yaml
##########
@@ -134,7 +134,6 @@ Configuration:
additivity: true
AppenderRef:
- ref: "${sys:pulsar.log.appender}"
- level: "${sys:pulsar.log.level}"
Review comment:
i have a test in standalone mode after removed this line.
1. `pulsar.log.level` set by ENV property PULSAR_LOG_LEVEL in pulsar_env.sh to `warn`
<img width="224" alt="image" src="https://user-images.githubusercontent.com/16517186/155252415-39f73dfd-4d33-466b-872c-3fe71fb5e395.png">
2. then start pulsar service in standalone mode. the service log level is warn, worked as desired.
<img width="1066" alt="image" src="https://user-images.githubusercontent.com/16517186/155252639-87e46c42-0106-4ca1-9de4-4c6168b23c9c.png">
3. then run cmd `bin/pulsar-admin brokers update-logger-level --classname org.apache.zookeeper.server.FinalRequestProcessor --level debug` to validation feature in this PR. workd as desired.
<img width="1076" alt="image" src="https://user-images.githubusercontent.com/16517186/155252852-e9b7ccb1-740c-419f-9f67-4fd8f2072ae9.png">
so remove this line would not invalidate user's previous setting (set by ENV property `PULSAR_LOG_LEVEL` in pulsar_env.sh
--
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] ethqunzhong commented on a change in pull request #14200: Broker: support dynamic update log level at runtime
Posted by GitBox <gi...@apache.org>.
ethqunzhong commented on a change in pull request #14200:
URL: https://github.com/apache/pulsar/pull/14200#discussion_r809805671
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/BrokersBase.java
##########
@@ -437,5 +440,79 @@ private void doShutDownBrokerGracefully(int maxConcurrentUnloadPerSec,
pulsar().getBrokerService().unloadNamespaceBundlesGracefully(maxConcurrentUnloadPerSec, forcedTerminateTopic);
pulsar().closeAsync();
}
+
+ /**
+ * dynamically update log4j2 logger level at runtime.
+ *
+ * @param targetClassName : class name to update
+ * @param targetLevel : target log level
+ */
+ private CompletableFuture<Void> internalUpdateLoggerLevelAsync(String targetClassName, String targetLevel) {
+ CompletableFuture<Void> loggerLevelFuture = new CompletableFuture<>();
+ CompletableFuture.runAsync(() -> {
Review comment:
You're right. updated.
--
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] ethqunzhong commented on a change in pull request #14200: Broker: support dynamic update log level at runtime
Posted by GitBox <gi...@apache.org>.
ethqunzhong commented on a change in pull request #14200:
URL: https://github.com/apache/pulsar/pull/14200#discussion_r806540190
##########
File path: conf/log4j2.yaml
##########
@@ -134,7 +134,6 @@ Configuration:
additivity: true
AppenderRef:
- ref: "${sys:pulsar.log.appender}"
- level: "${sys:pulsar.log.level}"
Review comment:
as default `pulsar.log.level=info`
i have do a test in product env as follow:
<img width="1022" alt="image" src="https://user-images.githubusercontent.com/16517186/154014948-2d2547f2-7f1b-4943-a0e2-a8096d6abd1a.png">
remove this item seems do not change default log4j behavior.
as answer in [[stackoverflow:programmatically-change-log-level-in-log4j2](https://stackoverflow.com/questions/23434252/programmatically-change-log-level-in-log4j2) :
i have try all mentioned way to change logger level by programmatically way, it not work unless remove `level: "${sys:pulsar.log.level}"`
Have you ever had similar problems?
--
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 #14200: Broker: support dynamic update log level at runtime
Posted by GitBox <gi...@apache.org>.
eolivelli commented on a change in pull request #14200:
URL: https://github.com/apache/pulsar/pull/14200#discussion_r811722928
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/BrokersBase.java
##########
@@ -437,5 +440,62 @@ private void doShutDownBrokerGracefully(int maxConcurrentUnloadPerSec,
pulsar().getBrokerService().unloadNamespaceBundlesGracefully(maxConcurrentUnloadPerSec, forcedTerminateTopic);
pulsar().closeAsync();
}
+
+ /**
+ * dynamically update log4j2 logger level at runtime.
+ *
+ * @param targetClassName : class name to update
+ * @param targetLevel : target log level
+ */
+ private CompletableFuture<Void> internalUpdateLoggerLevelAsync(String targetClassName, String targetLevel) {
+ CompletableFuture<Void> loggerLevelFuture = new CompletableFuture<>();
+ try {
+ String className;
+ // if set "ROOT" will take effect to rootLogger
+ if (targetClassName.trim().equalsIgnoreCase("ROOT")) {
+ className = LogManager.ROOT_LOGGER_NAME;
+ } else {
+ className = targetClassName;
+ }
+ Level newLevel;
+ try {
+ newLevel = Level.valueOf(targetLevel);
+ Configurator.setAllLevels(className, newLevel);
+ } catch (IllegalArgumentException | NullPointerException e) {
+ // Unknown Log Level or NULL
+ throw new RestException(Status.PRECONDITION_FAILED, "Invalid logger level.");
+ }
+ } catch (RestException re) {
+ throw re;
+ } catch (Exception ie) {
+ throw new RestException(Status.PRECONDITION_FAILED, "Internal error.");
+ }
+ return CompletableFuture.completedFuture(null);
+ }
+
+ @POST
+ @Path("/log4j/{classname}/{level}")
Review comment:
I wouldn't use 'log4j' in this path.
'logging' is enough.
We are not sure that we will keep log4j forever.
This API is broad enough to be compatible with major logging frameworks in the future
--
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] ethqunzhong commented on a change in pull request #14200: Broker: support dynamic update log level at runtime
Posted by GitBox <gi...@apache.org>.
ethqunzhong commented on a change in pull request #14200:
URL: https://github.com/apache/pulsar/pull/14200#discussion_r804384196
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/BrokersBase.java
##########
@@ -437,5 +441,67 @@ private void doShutDownBrokerGracefully(int maxConcurrentUnloadPerSec,
pulsar().getBrokerService().unloadNamespaceBundlesGracefully(maxConcurrentUnloadPerSec, forcedTerminateTopic);
pulsar().closeAsync();
}
+
+ /**
+ * dynamically update log4j2 logger level in runtime.
+ *
+ * @param targetClassName
+ * @param targetLevel
+ */
+ private synchronized void updateLoggerLevel(String targetClassName, String targetLevel) {
Review comment:
it is redundant,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@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] ethqunzhong commented on pull request #14200: Broker: support dynamic update log level at runtime
Posted by GitBox <gi...@apache.org>.
ethqunzhong commented on pull request #14200:
URL: https://github.com/apache/pulsar/pull/14200#issuecomment-1047493780
/pulsarbot run-failure-checks
--
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] Jason918 commented on a change in pull request #14200: Broker: support dynamic update log level at runtime
Posted by GitBox <gi...@apache.org>.
Jason918 commented on a change in pull request #14200:
URL: https://github.com/apache/pulsar/pull/14200#discussion_r811841559
##########
File path: conf/log4j2.yaml
##########
@@ -134,7 +134,6 @@ Configuration:
additivity: true
AppenderRef:
- ref: "${sys:pulsar.log.appender}"
- level: "${sys:pulsar.log.level}"
Review comment:
> this PR will not change the settings
I am a little confused that if you removed this, how would user's previous setting (set by ENV property `PULSAR_LOG_LEVEL` in pulsar_env.sh) take effect?
--
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] mattisonchao commented on a change in pull request #14200: Broker: support dynamic update log level at runtime
Posted by GitBox <gi...@apache.org>.
mattisonchao commented on a change in pull request #14200:
URL: https://github.com/apache/pulsar/pull/14200#discussion_r804274556
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/BrokersBase.java
##########
@@ -43,6 +43,10 @@
import javax.ws.rs.container.Suspended;
import javax.ws.rs.core.Response;
import javax.ws.rs.core.Response.Status;
+
Review comment:
```suggestion
```
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/BrokersBase.java
##########
@@ -437,5 +441,67 @@ private void doShutDownBrokerGracefully(int maxConcurrentUnloadPerSec,
pulsar().getBrokerService().unloadNamespaceBundlesGracefully(maxConcurrentUnloadPerSec, forcedTerminateTopic);
pulsar().closeAsync();
}
+
+ /**
+ * dynamically update log4j2 logger level in runtime.
+ *
+ * @param targetClassName
+ * @param targetLevel
+ */
+ private synchronized void updateLoggerLevel(String targetClassName, String targetLevel) {
+ try {
+ String className;
+ if (targetClassName.trim().equalsIgnoreCase("ROOT")) {
+ className = LogManager.ROOT_LOGGER_NAME;
+ } else {
+ try {
+ className = targetClassName.trim();
+ // Check if class name valid
+ Class.forName(className);
+ } catch (ClassNotFoundException e) {
+ // Logger not found
+ throw new RestException(Status.NOT_FOUND, "Logger not found.");
+ }
+ }
+
+ Level level;
+ try {
+ level = Level.valueOf(targetLevel);
+ } catch (IllegalArgumentException e) {
+ // Level not found
+ throw new RestException(Status.PRECONDITION_FAILED, "Invalid logger level.");
+ }
+
+ if (level != null) {
+ Level originLevel = LogManager.getLogger(className).getLevel();
+ Configurator.setAllLevels(className, level);
+ LOG.info("[{}] Successfully update log level for className: {} ({} -> {}.)", clientAppId(), className, originLevel, level);
+ } else {
+ LOG.error("[{}] Failed to update log level for {}", clientAppId(), className);
+ }
+ } catch (RestException re) {
+ LOG.error("[{}] Failed to update log level for className: {}, targetLevel: {} due to rest exception.", clientAppId(), targetClassName, targetLevel);
+ throw re;
+ } catch (Exception ie) {
+ LOG.error("[{}] Failed to update log level for {} to {} due to internal error.",
+ clientAppId(), targetClassName, targetLevel);
+ throw new RestException(ie);
+ }
+ }
+
+ @POST
+ @Path("/log4j/{classname}/{level}")
+ @ApiOperation(value =
+ "update dynamic log4j2 logger level in runtime by classname. This operation requires Pulsar super-user privileges.")
+ @ApiResponses(value = {
+ @ApiResponse(code = 204, message = "class logger level updated successfully"),
+ @ApiResponse(code = 403, message = "You don't have admin permission to update log4j2 logger level."),
+ @ApiResponse(code = 500, message = "Internal server error")})
+ public void updateLoggerLevelDynamically(@PathParam("classname") String classname,
+ @PathParam("level") String level) throws Exception {
Review comment:
```suggestion
@PathParam("level") String level) {
```
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/BrokersBase.java
##########
@@ -437,5 +441,67 @@ private void doShutDownBrokerGracefully(int maxConcurrentUnloadPerSec,
pulsar().getBrokerService().unloadNamespaceBundlesGracefully(maxConcurrentUnloadPerSec, forcedTerminateTopic);
pulsar().closeAsync();
}
+
+ /**
+ * dynamically update log4j2 logger level in runtime.
+ *
+ * @param targetClassName
+ * @param targetLevel
+ */
+ private synchronized void updateLoggerLevel(String targetClassName, String targetLevel) {
Review comment:
Why synchronized?
--
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 #14200: Broker: support dynamic update log level at runtime
Posted by GitBox <gi...@apache.org>.
eolivelli commented on a change in pull request #14200:
URL: https://github.com/apache/pulsar/pull/14200#discussion_r806735706
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/BrokersBase.java
##########
@@ -437,5 +440,79 @@ private void doShutDownBrokerGracefully(int maxConcurrentUnloadPerSec,
pulsar().getBrokerService().unloadNamespaceBundlesGracefully(maxConcurrentUnloadPerSec, forcedTerminateTopic);
pulsar().closeAsync();
}
+
+ /**
+ * dynamically update log4j2 logger level at runtime.
+ *
+ * @param targetClassName : class name to update
+ * @param targetLevel : target log level
+ */
+ private CompletableFuture<Void> internalUpdateLoggerLevelAsync(String targetClassName, String targetLevel) {
+ CompletableFuture<Void> loggerLevelFuture = new CompletableFuture<>();
+ CompletableFuture.runAsync(() -> {
+ try {
+ String className;
+ // if set "ROOT" will take effect to rootLogger
+ if (targetClassName.trim().equalsIgnoreCase("ROOT")) {
+ className = LogManager.ROOT_LOGGER_NAME;
+ } else {
+ try {
+ className = targetClassName.trim();
+ // Check if class name valid
+ Class.forName(className);
Review comment:
Here you aree triggering the load of a class from a APi call.
This may be seen as a security issue.
There is no need to do this.
Please remove
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/BrokersBase.java
##########
@@ -437,5 +440,79 @@ private void doShutDownBrokerGracefully(int maxConcurrentUnloadPerSec,
pulsar().getBrokerService().unloadNamespaceBundlesGracefully(maxConcurrentUnloadPerSec, forcedTerminateTopic);
pulsar().closeAsync();
}
+
+ /**
+ * dynamically update log4j2 logger level at runtime.
+ *
+ * @param targetClassName : class name to update
+ * @param targetLevel : target log level
+ */
+ private CompletableFuture<Void> internalUpdateLoggerLevelAsync(String targetClassName, String targetLevel) {
+ CompletableFuture<Void> loggerLevelFuture = new CompletableFuture<>();
+ CompletableFuture.runAsync(() -> {
Review comment:
Why are we executing this a a unknown thread?
We can execute this code in the main thread of the http handler
##########
File path: pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdBrokers.java
##########
@@ -176,6 +176,20 @@ void run() throws Exception {
}
}
+ @Parameters(commandDescription = "Update dynamic log4j2 logger level in runtime by classname.")
+ private class UpdateLoggerLevelCmd extends CliCommand {
Review comment:
Please note that there is no control about the broker who is serving the request.
Can we report this to the user?
- report the id of the broker that process the request
- write a generic warning
--
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] ethqunzhong commented on a change in pull request #14200: Broker: support dynamic update log level at runtime
Posted by GitBox <gi...@apache.org>.
ethqunzhong commented on a change in pull request #14200:
URL: https://github.com/apache/pulsar/pull/14200#discussion_r807635627
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/BrokersBase.java
##########
@@ -437,5 +440,79 @@ private void doShutDownBrokerGracefully(int maxConcurrentUnloadPerSec,
pulsar().getBrokerService().unloadNamespaceBundlesGracefully(maxConcurrentUnloadPerSec, forcedTerminateTopic);
pulsar().closeAsync();
}
+
+ /**
+ * dynamically update log4j2 logger level at runtime.
+ *
+ * @param targetClassName : class name to update
+ * @param targetLevel : target log level
+ */
+ private CompletableFuture<Void> internalUpdateLoggerLevelAsync(String targetClassName, String targetLevel) {
+ CompletableFuture<Void> loggerLevelFuture = new CompletableFuture<>();
+ CompletableFuture.runAsync(() -> {
+ try {
+ String className;
+ // if set "ROOT" will take effect to rootLogger
+ if (targetClassName.trim().equalsIgnoreCase("ROOT")) {
+ className = LogManager.ROOT_LOGGER_NAME;
+ } else {
+ try {
+ className = targetClassName.trim();
+ // Check if class name valid
+ Class.forName(className);
Review comment:
👍🏻 good suggestions. it is redundant. 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@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] ethqunzhong commented on a change in pull request #14200: Broker: support dynamic update log level at runtime
Posted by GitBox <gi...@apache.org>.
ethqunzhong commented on a change in pull request #14200:
URL: https://github.com/apache/pulsar/pull/14200#discussion_r806540190
##########
File path: conf/log4j2.yaml
##########
@@ -134,7 +134,6 @@ Configuration:
additivity: true
AppenderRef:
- ref: "${sys:pulsar.log.appender}"
- level: "${sys:pulsar.log.level}"
Review comment:
as default `pulsar.log.level=info`
i have do a test in product env as follow:
<img width="1022" alt="image" src="https://user-images.githubusercontent.com/16517186/154014948-2d2547f2-7f1b-4943-a0e2-a8096d6abd1a.png">
remove this item seems do not change default log4j behavior.
as answer in [[stackoverflow:programmatically-change-log-level-in-log4j2](https://stackoverflow.com/questions/23434252/programmatically-change-log-level-in-log4j2) :
i have try all mentioned way to change logger level by programmatically way, it not work unless remove `level: "${sys:pulsar.log.level}"`
Have you ever had similar problems?
##########
File path: conf/log4j2.yaml
##########
@@ -134,7 +134,6 @@ Configuration:
additivity: true
AppenderRef:
- ref: "${sys:pulsar.log.appender}"
- level: "${sys:pulsar.log.level}"
Review comment:
i have set `pulsar.log.level=warn` for test. but not what i expected. described as this issue #14298
--
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] ethqunzhong commented on a change in pull request #14200: Broker: support dynamic update log level at runtime
Posted by GitBox <gi...@apache.org>.
ethqunzhong commented on a change in pull request #14200:
URL: https://github.com/apache/pulsar/pull/14200#discussion_r804384292
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/BrokersBase.java
##########
@@ -437,5 +441,67 @@ private void doShutDownBrokerGracefully(int maxConcurrentUnloadPerSec,
pulsar().getBrokerService().unloadNamespaceBundlesGracefully(maxConcurrentUnloadPerSec, forcedTerminateTopic);
pulsar().closeAsync();
}
+
+ /**
+ * dynamically update log4j2 logger level in runtime.
+ *
+ * @param targetClassName
+ * @param targetLevel
+ */
+ private synchronized void updateLoggerLevel(String targetClassName, String targetLevel) {
+ try {
+ String className;
+ if (targetClassName.trim().equalsIgnoreCase("ROOT")) {
+ className = LogManager.ROOT_LOGGER_NAME;
+ } else {
+ try {
+ className = targetClassName.trim();
+ // Check if class name valid
+ Class.forName(className);
+ } catch (ClassNotFoundException e) {
+ // Logger not found
+ throw new RestException(Status.NOT_FOUND, "Logger not found.");
+ }
+ }
+
+ Level level;
+ try {
+ level = Level.valueOf(targetLevel);
+ } catch (IllegalArgumentException e) {
+ // Level not found
+ throw new RestException(Status.PRECONDITION_FAILED, "Invalid logger level.");
+ }
+
+ if (level != null) {
+ Level originLevel = LogManager.getLogger(className).getLevel();
+ Configurator.setAllLevels(className, level);
+ LOG.info("[{}] Successfully update log level for className: {} ({} -> {}.)", clientAppId(), className, originLevel, level);
+ } else {
+ LOG.error("[{}] Failed to update log level for {}", clientAppId(), className);
+ }
+ } catch (RestException re) {
+ LOG.error("[{}] Failed to update log level for className: {}, targetLevel: {} due to rest exception.", clientAppId(), targetClassName, targetLevel);
+ throw re;
+ } catch (Exception ie) {
+ LOG.error("[{}] Failed to update log level for {} to {} due to internal error.",
+ clientAppId(), targetClassName, targetLevel);
+ throw new RestException(ie);
+ }
+ }
+
+ @POST
+ @Path("/log4j/{classname}/{level}")
+ @ApiOperation(value =
+ "update dynamic log4j2 logger level in runtime by classname. This operation requires Pulsar super-user privileges.")
+ @ApiResponses(value = {
+ @ApiResponse(code = 204, message = "class logger level updated successfully"),
+ @ApiResponse(code = 403, message = "You don't have admin permission to update log4j2 logger level."),
+ @ApiResponse(code = 500, message = "Internal server error")})
+ public void updateLoggerLevelDynamically(@PathParam("classname") String classname,
Review comment:
added async method
##########
File path: pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdBrokers.java
##########
@@ -176,6 +176,19 @@ void run() throws Exception {
}
}
+ @Parameters(commandDescription = "Update dynamic log4j2 logger level in runtime by classname.")
+ private class UpdateLoggerLevelCmd extends CliCommand {
+ @Parameter(names = "--classname", description = "The except class name", required = true)
+ private String classname;
+ @Parameter(names = "--level", description = "The target logger level", required = true)
Review comment:
Thank Your Suggestion. added.
##########
File path: pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdBrokers.java
##########
@@ -176,6 +176,19 @@ void run() throws Exception {
}
}
+ @Parameters(commandDescription = "Update dynamic log4j2 logger level in runtime by classname.")
+ private class UpdateLoggerLevelCmd extends CliCommand {
+ @Parameter(names = "--classname", description = "The except class name", required = true)
Review comment:
Thank Your Suggestion. added.
--
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] wolfstudy commented on pull request #14200: Broker: support dynamic update log level at runtime
Posted by GitBox <gi...@apache.org>.
wolfstudy commented on pull request #14200:
URL: https://github.com/apache/pulsar/pull/14200#issuecomment-1046655355
cc @eolivelli @Jason918 PTAL again thanks
--
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