You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2022/08/07 10:31:29 UTC
[GitHub] [pinot] xiangfu0 opened a new pull request, #9180: Adding ability to update logger level
xiangfu0 opened a new pull request, #9180:
URL: https://github.com/apache/pinot/pull/9180
- Allow change logger level at runtime
--
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@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] walterddr commented on a diff in pull request #9180: Adding logger utils and allow change logger level at runtime
Posted by GitBox <gi...@apache.org>.
walterddr commented on code in PR #9180:
URL: https://github.com/apache/pinot/pull/9180#discussion_r940697513
##########
pinot-broker/src/main/java/org/apache/pinot/broker/api/resources/PinotBrokerLogger.java:
##########
@@ -0,0 +1,83 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.broker.api.resources;
+
+import io.swagger.annotations.Api;
+import io.swagger.annotations.ApiKeyAuthDefinition;
+import io.swagger.annotations.ApiOperation;
+import io.swagger.annotations.ApiParam;
+import io.swagger.annotations.Authorization;
+import io.swagger.annotations.SecurityDefinition;
+import io.swagger.annotations.SwaggerDefinition;
+import java.util.List;
+import java.util.Map;
+import javax.ws.rs.GET;
+import javax.ws.rs.PUT;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import javax.ws.rs.WebApplicationException;
+import javax.ws.rs.core.HttpHeaders;
+import javax.ws.rs.core.MediaType;
+import javax.ws.rs.core.Response;
+import org.apache.pinot.common.utils.LoggerUtils;
+
+import static org.apache.pinot.spi.utils.CommonConstants.SWAGGER_AUTHORIZATION_KEY;
+
+
+/**
+ * Logger resource.
+ */
+@Api(tags = "Logger", authorizations = {@Authorization(value = SWAGGER_AUTHORIZATION_KEY)})
+@SwaggerDefinition(securityDefinition = @SecurityDefinition(apiKeyAuthDefinitions = @ApiKeyAuthDefinition(name =
+ HttpHeaders.AUTHORIZATION, in = ApiKeyAuthDefinition.ApiKeyLocation.HEADER, key = SWAGGER_AUTHORIZATION_KEY)))
+@Path("/loggers/")
+public class PinotBrokerLogger {
+
+ @GET
+ @Path("")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "Get all the loggers", notes = "Return all the logger names")
+ public List<String> getLoggers(
+ @ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableNameWithType) {
+ return LoggerUtils.getAllLoggers();
+ }
+
+ @GET
+ @Path("{loggerName}")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "Get logger configs", notes = "Return logger info")
+ public Map<String, Object> getLogger(
+ @ApiParam(value = "Logger name", required = true) @PathParam("loggerName") String loggerName) {
Review Comment:
Oh. I completely misunderstood the LoggerName...
--
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@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] xiangfu0 merged pull request #9180: Adding logger utils and allow change logger level at runtime
Posted by GitBox <gi...@apache.org>.
xiangfu0 merged PR #9180:
URL: https://github.com/apache/pinot/pull/9180
--
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@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] kkrugler commented on a diff in pull request #9180: Adding logger utils and allow change logger level at runtime
Posted by GitBox <gi...@apache.org>.
kkrugler commented on code in PR #9180:
URL: https://github.com/apache/pinot/pull/9180#discussion_r940648745
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/LoggerUtils.java:
##########
@@ -0,0 +1,80 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.common.utils;
+
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+import org.apache.logging.log4j.Level;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.core.LoggerContext;
+import org.apache.logging.log4j.core.config.Configuration;
+import org.apache.logging.log4j.core.config.LoggerConfig;
+
+
+/**
+ * Logger utils for process level logger management.
+ */
+public class LoggerUtils {
+ private static final String ROOT = "root";
+ private static final String NAME = "name";
+ private static final String LEVEL = "level";
+ private static final String FILTER = "filter";
+
+ private LoggerUtils() {
+ }
+
+ public static Map<String, Object> setLoggerLevel(String loggerName, String logLevel) {
+ LoggerContext context = (LoggerContext) LogManager.getContext(false);
+ Configuration config = context.getConfiguration();
+ LoggerConfig loggerConfig =
Review Comment:
If the passed `loggerName` isn't valid, will you get back null (and this triggers an NPE) or the root logger configuration? Wondering if you could check for the name matching something returned by `getAllLoggers()`, and fail gracefully if not.
--
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@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] Jackie-Jiang commented on a diff in pull request #9180: Adding logger utils and allow change logger level at runtime
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on code in PR #9180:
URL: https://github.com/apache/pinot/pull/9180#discussion_r940519735
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/LoggerUtils.java:
##########
@@ -0,0 +1,80 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.common.utils;
+
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+import org.apache.logging.log4j.Level;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.core.LoggerContext;
+import org.apache.logging.log4j.core.config.Configuration;
+import org.apache.logging.log4j.core.config.LoggerConfig;
+
+
+/**
+ * Logger utils for process level logger management.
+ */
+public class LoggerUtils {
+ private static final String ROOT = "root";
+ private static final String NAME = "name";
+ private static final String LEVEL = "level";
+ private static final String FILTER = "filter";
+
+ private LoggerUtils() {
+ }
+
+ public static Map<String, Object> setLoggerLevel(String loggerName, String logLevel) {
+ LoggerContext context = (LoggerContext) LogManager.getContext(false);
+ Configuration config = context.getConfiguration();
+ LoggerConfig loggerConfig =
+ loggerName.equalsIgnoreCase(ROOT) ? config.getRootLogger() : config.getLoggerConfig(loggerName);
+ loggerConfig.setLevel(Level.valueOf(logLevel));
+ // This causes all Loggers to re-fetch information from their LoggerConfig.
+ context.updateLoggers();
+ return getLoggerResponse(loggerConfig);
+ }
+
+ public static Map<String, Object> getLoggerInfo(String loggerName) {
Review Comment:
```suggestion
@Nullable
public static Map<String, Object> getLoggerInfo(String loggerName) {
```
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/LoggerUtils.java:
##########
@@ -0,0 +1,80 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.common.utils;
+
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+import org.apache.logging.log4j.Level;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.core.LoggerContext;
+import org.apache.logging.log4j.core.config.Configuration;
+import org.apache.logging.log4j.core.config.LoggerConfig;
+
+
+/**
+ * Logger utils for process level logger management.
+ */
+public class LoggerUtils {
+ private static final String ROOT = "root";
+ private static final String NAME = "name";
+ private static final String LEVEL = "level";
+ private static final String FILTER = "filter";
+
+ private LoggerUtils() {
+ }
+
+ public static Map<String, Object> setLoggerLevel(String loggerName, String logLevel) {
Review Comment:
Let's add some javadoc for these methods
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/LoggerUtils.java:
##########
@@ -0,0 +1,80 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.common.utils;
+
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+import org.apache.logging.log4j.Level;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.core.LoggerContext;
+import org.apache.logging.log4j.core.config.Configuration;
+import org.apache.logging.log4j.core.config.LoggerConfig;
+
+
+/**
+ * Logger utils for process level logger management.
+ */
+public class LoggerUtils {
+ private static final String ROOT = "root";
+ private static final String NAME = "name";
+ private static final String LEVEL = "level";
+ private static final String FILTER = "filter";
+
+ private LoggerUtils() {
+ }
+
+ public static Map<String, Object> setLoggerLevel(String loggerName, String logLevel) {
+ LoggerContext context = (LoggerContext) LogManager.getContext(false);
+ Configuration config = context.getConfiguration();
+ LoggerConfig loggerConfig =
+ loggerName.equalsIgnoreCase(ROOT) ? config.getRootLogger() : config.getLoggerConfig(loggerName);
+ loggerConfig.setLevel(Level.valueOf(logLevel));
+ // This causes all Loggers to re-fetch information from their LoggerConfig.
+ context.updateLoggers();
+ return getLoggerResponse(loggerConfig);
+ }
+
+ public static Map<String, Object> getLoggerInfo(String loggerName) {
+ LoggerContext context = (LoggerContext) LogManager.getContext(false);
+ Configuration config = context.getConfiguration();
+ LoggerConfig loggerConfig =
+ loggerName.equalsIgnoreCase(ROOT) ? config.getRootLogger() : config.getLoggerConfig(loggerName);
+ if (loggerConfig == null) {
+ return null;
+ }
+ return getLoggerResponse(loggerConfig);
+ }
+
+ public static List<String> getAllLoggers() {
+ LoggerContext context = (LoggerContext) LogManager.getContext(false);
+ Configuration config = context.getConfiguration();
+ return config.getLoggers().values().stream().map(loggerConfig -> loggerConfig.toString())
+ .collect(Collectors.toList());
Review Comment:
(nit)
```suggestion
return config.getLoggers().values().stream().map(LoggerConfig::toString).collect(Collectors.toList());
```
--
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@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] kkrugler commented on a diff in pull request #9180: Adding logger utils and allow change logger level at runtime
Posted by GitBox <gi...@apache.org>.
kkrugler commented on code in PR #9180:
URL: https://github.com/apache/pinot/pull/9180#discussion_r940649081
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/LoggerUtils.java:
##########
@@ -0,0 +1,80 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.common.utils;
+
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+import org.apache.logging.log4j.Level;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.core.LoggerContext;
+import org.apache.logging.log4j.core.config.Configuration;
+import org.apache.logging.log4j.core.config.LoggerConfig;
+
+
+/**
+ * Logger utils for process level logger management.
+ */
+public class LoggerUtils {
+ private static final String ROOT = "root";
+ private static final String NAME = "name";
+ private static final String LEVEL = "level";
+ private static final String FILTER = "filter";
+
+ private LoggerUtils() {
+ }
+
+ public static Map<String, Object> setLoggerLevel(String loggerName, String logLevel) {
+ LoggerContext context = (LoggerContext) LogManager.getContext(false);
+ Configuration config = context.getConfiguration();
+ LoggerConfig loggerConfig =
+ loggerName.equalsIgnoreCase(ROOT) ? config.getRootLogger() : config.getLoggerConfig(loggerName);
+ loggerConfig.setLevel(Level.valueOf(logLevel));
Review Comment:
What happens if an invalid `logLevel` is passed?
--
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@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] xiangfu0 commented on a diff in pull request #9180: Adding logger utils and allow change logger level at runtime
Posted by GitBox <gi...@apache.org>.
xiangfu0 commented on code in PR #9180:
URL: https://github.com/apache/pinot/pull/9180#discussion_r940778301
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/LoggerUtils.java:
##########
@@ -0,0 +1,104 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.common.utils;
+
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+import javax.annotation.Nullable;
+import org.apache.logging.log4j.Level;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.core.LoggerContext;
+import org.apache.logging.log4j.core.config.Configuration;
+import org.apache.logging.log4j.core.config.LoggerConfig;
+
+
+/**
+ * Logger utils for process level logger management.
+ */
+public class LoggerUtils {
+ private static final String ROOT = "root";
+ private static final String NAME = "name";
+ private static final String LEVEL = "level";
+ private static final String FILTER = "filter";
+
+ private LoggerUtils() {
+ }
+
+ /**
+ * Set logger level at runtime.
+ * @param loggerName
+ * @param logLevel
+ * @return logger info
+ */
+ public static Map<String, Object> setLoggerLevel(String loggerName, String logLevel) {
+ LoggerContext context = (LoggerContext) LogManager.getContext(false);
+ Configuration config = context.getConfiguration();
+ if (!getAllLoggers().contains(loggerName)) {
+ throw new RuntimeException("Logger - " + loggerName + " not found");
+ }
+ LoggerConfig loggerConfig = getLoggerConfig(config, loggerName);
+ try {
+ loggerConfig.setLevel(Level.valueOf(logLevel));
+ } catch (Exception e) {
+ throw new RuntimeException("Unrecognized logger level - " + logLevel, e);
+ }
+ // This causes all Loggers to re-fetch information from their LoggerConfig.
+ context.updateLoggers();
+ return getLoggerResponse(loggerConfig);
+ }
+
+ /**
+ * Fetch logger info of name, level and filter.
+ * @param loggerName
+ * @return logger info
+ */
+ @Nullable
+ public static Map<String, Object> getLoggerInfo(String loggerName) {
+ LoggerContext context = (LoggerContext) LogManager.getContext(false);
+ Configuration config = context.getConfiguration();
+ if (!getAllLoggers().contains(loggerName)) {
+ return null;
+ }
+ LoggerConfig loggerConfig = getLoggerConfig(config, loggerName);
+ return getLoggerResponse(loggerConfig);
+ }
+
+ /**
+ * @return a list of all the logger names
+ */
+ public static List<String> getAllLoggers() {
+ LoggerContext context = (LoggerContext) LogManager.getContext(false);
+ Configuration config = context.getConfiguration();
+ return config.getLoggers().values().stream().map(LoggerConfig::toString).collect(Collectors.toList());
Review Comment:
no duplicate, logger actually give unmatched key to root logger, so we explicitly specify it here.
--
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@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] xiangfu0 commented on a diff in pull request #9180: Adding logger utils and allow change logger level at runtime
Posted by GitBox <gi...@apache.org>.
xiangfu0 commented on code in PR #9180:
URL: https://github.com/apache/pinot/pull/9180#discussion_r940676523
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/LoggerUtils.java:
##########
@@ -0,0 +1,80 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.common.utils;
+
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+import org.apache.logging.log4j.Level;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.core.LoggerContext;
+import org.apache.logging.log4j.core.config.Configuration;
+import org.apache.logging.log4j.core.config.LoggerConfig;
+
+
+/**
+ * Logger utils for process level logger management.
+ */
+public class LoggerUtils {
+ private static final String ROOT = "root";
+ private static final String NAME = "name";
+ private static final String LEVEL = "level";
+ private static final String FILTER = "filter";
+
+ private LoggerUtils() {
+ }
+
+ public static Map<String, Object> setLoggerLevel(String loggerName, String logLevel) {
+ LoggerContext context = (LoggerContext) LogManager.getContext(false);
+ Configuration config = context.getConfiguration();
+ LoggerConfig loggerConfig =
+ loggerName.equalsIgnoreCase(ROOT) ? config.getRootLogger() : config.getLoggerConfig(loggerName);
+ loggerConfig.setLevel(Level.valueOf(logLevel));
+ // This causes all Loggers to re-fetch information from their LoggerConfig.
+ context.updateLoggers();
+ return getLoggerResponse(loggerConfig);
+ }
+
+ public static Map<String, Object> getLoggerInfo(String loggerName) {
+ LoggerContext context = (LoggerContext) LogManager.getContext(false);
+ Configuration config = context.getConfiguration();
+ LoggerConfig loggerConfig =
+ loggerName.equalsIgnoreCase(ROOT) ? config.getRootLogger() : config.getLoggerConfig(loggerName);
Review Comment:
It won't. So we just add a logger name check before here.
--
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@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] xiangfu0 commented on a diff in pull request #9180: Adding logger utils and allow change logger level at runtime
Posted by GitBox <gi...@apache.org>.
xiangfu0 commented on code in PR #9180:
URL: https://github.com/apache/pinot/pull/9180#discussion_r941701802
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/LoggerUtils.java:
##########
@@ -0,0 +1,104 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.common.utils;
+
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+import javax.annotation.Nullable;
+import org.apache.logging.log4j.Level;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.core.LoggerContext;
+import org.apache.logging.log4j.core.config.Configuration;
+import org.apache.logging.log4j.core.config.LoggerConfig;
+
+
+/**
+ * Logger utils for process level logger management.
+ */
+public class LoggerUtils {
+ private static final String ROOT = "root";
+ private static final String NAME = "name";
+ private static final String LEVEL = "level";
+ private static final String FILTER = "filter";
+
+ private LoggerUtils() {
+ }
+
+ /**
+ * Set logger level at runtime.
+ * @param loggerName
+ * @param logLevel
+ * @return logger info
+ */
+ public static Map<String, Object> setLoggerLevel(String loggerName, String logLevel) {
+ LoggerContext context = (LoggerContext) LogManager.getContext(false);
+ Configuration config = context.getConfiguration();
+ if (!getAllLoggers().contains(loggerName)) {
+ throw new RuntimeException("Logger - " + loggerName + " not found");
+ }
+ LoggerConfig loggerConfig = getLoggerConfig(config, loggerName);
+ try {
+ loggerConfig.setLevel(Level.valueOf(logLevel));
+ } catch (Exception e) {
+ throw new RuntimeException("Unrecognized logger level - " + logLevel, e);
+ }
+ // This causes all Loggers to re-fetch information from their LoggerConfig.
+ context.updateLoggers();
+ return getLoggerResponse(loggerConfig);
+ }
+
+ /**
+ * Fetch logger info of name, level and filter.
+ * @param loggerName
+ * @return logger info
+ */
+ @Nullable
+ public static Map<String, Object> getLoggerInfo(String loggerName) {
+ LoggerContext context = (LoggerContext) LogManager.getContext(false);
+ Configuration config = context.getConfiguration();
+ if (!getAllLoggers().contains(loggerName)) {
+ return null;
+ }
+ LoggerConfig loggerConfig = getLoggerConfig(config, loggerName);
+ return getLoggerResponse(loggerConfig);
+ }
+
+ /**
+ * @return a list of all the logger names
+ */
+ public static List<String> getAllLoggers() {
+ LoggerContext context = (LoggerContext) LogManager.getContext(false);
+ Configuration config = context.getConfiguration();
+ return config.getLoggers().values().stream().map(LoggerConfig::toString).collect(Collectors.toList());
+ }
+
+ private static LoggerConfig getLoggerConfig(Configuration config, String loggerName) {
+ return loggerName.equalsIgnoreCase(ROOT) ? config.getRootLogger() : config.getLoggerConfig(loggerName);
+ }
+
+ private static Map<String, Object> getLoggerResponse(LoggerConfig loggerConfig) {
Review Comment:
changed to String, just extra null check needed.
--
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@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] walterddr commented on a diff in pull request #9180: Adding logger utils and allow change logger level at runtime
Posted by GitBox <gi...@apache.org>.
walterddr commented on code in PR #9180:
URL: https://github.com/apache/pinot/pull/9180#discussion_r940630486
##########
pinot-broker/src/main/java/org/apache/pinot/broker/api/resources/PinotBrokerLogger.java:
##########
@@ -0,0 +1,83 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.broker.api.resources;
+
+import io.swagger.annotations.Api;
+import io.swagger.annotations.ApiKeyAuthDefinition;
+import io.swagger.annotations.ApiOperation;
+import io.swagger.annotations.ApiParam;
+import io.swagger.annotations.Authorization;
+import io.swagger.annotations.SecurityDefinition;
+import io.swagger.annotations.SwaggerDefinition;
+import java.util.List;
+import java.util.Map;
+import javax.ws.rs.GET;
+import javax.ws.rs.PUT;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import javax.ws.rs.WebApplicationException;
+import javax.ws.rs.core.HttpHeaders;
+import javax.ws.rs.core.MediaType;
+import javax.ws.rs.core.Response;
+import org.apache.pinot.common.utils.LoggerUtils;
+
+import static org.apache.pinot.spi.utils.CommonConstants.SWAGGER_AUTHORIZATION_KEY;
+
+
+/**
+ * Logger resource.
+ */
+@Api(tags = "Logger", authorizations = {@Authorization(value = SWAGGER_AUTHORIZATION_KEY)})
+@SwaggerDefinition(securityDefinition = @SecurityDefinition(apiKeyAuthDefinitions = @ApiKeyAuthDefinition(name =
+ HttpHeaders.AUTHORIZATION, in = ApiKeyAuthDefinition.ApiKeyLocation.HEADER, key = SWAGGER_AUTHORIZATION_KEY)))
+@Path("/loggers/")
+public class PinotBrokerLogger {
+
+ @GET
+ @Path("")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "Get all the loggers", notes = "Return all the logger names")
+ public List<String> getLoggers(
+ @ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableNameWithType) {
+ return LoggerUtils.getAllLoggers();
+ }
+
+ @GET
+ @Path("{loggerName}")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "Get logger configs", notes = "Return logger info")
+ public Map<String, Object> getLogger(
+ @ApiParam(value = "Logger name", required = true) @PathParam("loggerName") String loggerName) {
Review Comment:
good to explain how the logger name will look like.
- is it fully-qualified name? or simple class name?
- will it find transitive dependency loggers
- what about shaded/relocated ones?
--
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@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] Jackie-Jiang commented on a diff in pull request #9180: Adding logger utils and allow change logger level at runtime
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on code in PR #9180:
URL: https://github.com/apache/pinot/pull/9180#discussion_r941598114
##########
pinot-common/src/test/java/org/apache/pinot/common/utils/LoggerUtilsTest.java:
##########
@@ -0,0 +1,117 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.common.utils;
+
+import com.google.common.collect.ImmutableList;
+import java.util.List;
+import java.util.Map;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.testng.Assert;
+import org.testng.annotations.Test;
+
+
+public class LoggerUtilsTest {
+ private static final Logger LOGGER = LoggerFactory.getLogger(LoggerUtilsTest.class);
+
+ @Test
+ public void testGetAllLoggers() {
+ List<String> allLoggers = LoggerUtils.getAllLoggers();
+ Assert.assertEquals(allLoggers.size(), 1);
Review Comment:
(nit) We may use static import for `Assert` for tests
##########
pinot-broker/src/main/java/org/apache/pinot/broker/api/resources/PinotBrokerLogger.java:
##########
@@ -0,0 +1,83 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.broker.api.resources;
+
+import io.swagger.annotations.Api;
+import io.swagger.annotations.ApiKeyAuthDefinition;
+import io.swagger.annotations.ApiOperation;
+import io.swagger.annotations.ApiParam;
+import io.swagger.annotations.Authorization;
+import io.swagger.annotations.SecurityDefinition;
+import io.swagger.annotations.SwaggerDefinition;
+import java.util.List;
+import java.util.Map;
+import javax.ws.rs.GET;
+import javax.ws.rs.PUT;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import javax.ws.rs.QueryParam;
+import javax.ws.rs.WebApplicationException;
+import javax.ws.rs.core.HttpHeaders;
+import javax.ws.rs.core.MediaType;
+import javax.ws.rs.core.Response;
+import org.apache.pinot.common.utils.LoggerUtils;
+
+import static org.apache.pinot.spi.utils.CommonConstants.SWAGGER_AUTHORIZATION_KEY;
+
+
+/**
+ * Logger resource.
+ */
+@Api(tags = "Logger", authorizations = {@Authorization(value = SWAGGER_AUTHORIZATION_KEY)})
+@SwaggerDefinition(securityDefinition = @SecurityDefinition(apiKeyAuthDefinitions = @ApiKeyAuthDefinition(name =
+ HttpHeaders.AUTHORIZATION, in = ApiKeyAuthDefinition.ApiKeyLocation.HEADER, key = SWAGGER_AUTHORIZATION_KEY)))
+@Path("/")
+public class PinotBrokerLogger {
+
+ @GET
+ @Path("/loggers")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "Get all the loggers", notes = "Return all the logger names")
+ public List<String> getLoggers() {
+ return LoggerUtils.getAllLoggers();
+ }
+
+ @GET
+ @Path("/loggers/{loggerName}")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "Get logger configs", notes = "Return logger info")
+ public Map<String, Object> getLogger(
+ @ApiParam(value = "Logger name", required = true) @PathParam("loggerName") String loggerName) {
+ Map<String, Object> loggerInfo = LoggerUtils.getLoggerInfo(loggerName);
+ if (loggerInfo == null) {
+ throw new WebApplicationException(String.format("Logger %s not found", loggerName), Response.Status.NOT_FOUND);
+ }
+ return loggerInfo;
+ }
+
+ @PUT
+ @Path("/loggers/{loggerName}/level")
Review Comment:
Suggest changing it to
```suggestion
@Path("/loggers/{loggerName}")
```
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/LoggerUtils.java:
##########
@@ -0,0 +1,104 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.common.utils;
+
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+import javax.annotation.Nullable;
+import org.apache.logging.log4j.Level;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.core.LoggerContext;
+import org.apache.logging.log4j.core.config.Configuration;
+import org.apache.logging.log4j.core.config.LoggerConfig;
+
+
+/**
+ * Logger utils for process level logger management.
+ */
+public class LoggerUtils {
+ private static final String ROOT = "root";
+ private static final String NAME = "name";
+ private static final String LEVEL = "level";
+ private static final String FILTER = "filter";
+
+ private LoggerUtils() {
+ }
+
+ /**
+ * Set logger level at runtime.
+ * @param loggerName
+ * @param logLevel
+ * @return logger info
+ */
+ public static Map<String, Object> setLoggerLevel(String loggerName, String logLevel) {
+ LoggerContext context = (LoggerContext) LogManager.getContext(false);
+ Configuration config = context.getConfiguration();
+ if (!getAllLoggers().contains(loggerName)) {
+ throw new RuntimeException("Logger - " + loggerName + " not found");
+ }
+ LoggerConfig loggerConfig = getLoggerConfig(config, loggerName);
+ try {
+ loggerConfig.setLevel(Level.valueOf(logLevel));
+ } catch (Exception e) {
+ throw new RuntimeException("Unrecognized logger level - " + logLevel, e);
+ }
+ // This causes all Loggers to re-fetch information from their LoggerConfig.
+ context.updateLoggers();
+ return getLoggerResponse(loggerConfig);
+ }
+
+ /**
+ * Fetch logger info of name, level and filter.
+ * @param loggerName
+ * @return logger info
+ */
+ @Nullable
+ public static Map<String, Object> getLoggerInfo(String loggerName) {
+ LoggerContext context = (LoggerContext) LogManager.getContext(false);
+ Configuration config = context.getConfiguration();
+ if (!getAllLoggers().contains(loggerName)) {
+ return null;
+ }
+ LoggerConfig loggerConfig = getLoggerConfig(config, loggerName);
+ return getLoggerResponse(loggerConfig);
+ }
+
+ /**
+ * @return a list of all the logger names
+ */
+ public static List<String> getAllLoggers() {
+ LoggerContext context = (LoggerContext) LogManager.getContext(false);
+ Configuration config = context.getConfiguration();
+ return config.getLoggers().values().stream().map(LoggerConfig::toString).collect(Collectors.toList());
+ }
+
+ private static LoggerConfig getLoggerConfig(Configuration config, String loggerName) {
+ return loggerName.equalsIgnoreCase(ROOT) ? config.getRootLogger() : config.getLoggerConfig(loggerName);
+ }
+
+ private static Map<String, Object> getLoggerResponse(LoggerConfig loggerConfig) {
Review Comment:
Should we always return a `Map<String, String>` and store `loggerConfig.getFilter().toString()`? Not sure if `Filter` object can be properly serialized in the rest response
--
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@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] kkrugler commented on a diff in pull request #9180: Adding logger utils and allow change logger level at runtime
Posted by GitBox <gi...@apache.org>.
kkrugler commented on code in PR #9180:
URL: https://github.com/apache/pinot/pull/9180#discussion_r940647451
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/LoggerUtils.java:
##########
@@ -0,0 +1,80 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.common.utils;
+
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+import org.apache.logging.log4j.Level;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.core.LoggerContext;
+import org.apache.logging.log4j.core.config.Configuration;
+import org.apache.logging.log4j.core.config.LoggerConfig;
+
+
+/**
+ * Logger utils for process level logger management.
+ */
+public class LoggerUtils {
+ private static final String ROOT = "root";
+ private static final String NAME = "name";
+ private static final String LEVEL = "level";
+ private static final String FILTER = "filter";
+
+ private LoggerUtils() {
+ }
+
+ public static Map<String, Object> setLoggerLevel(String loggerName, String logLevel) {
+ LoggerContext context = (LoggerContext) LogManager.getContext(false);
+ Configuration config = context.getConfiguration();
+ LoggerConfig loggerConfig =
+ loggerName.equalsIgnoreCase(ROOT) ? config.getRootLogger() : config.getLoggerConfig(loggerName);
+ loggerConfig.setLevel(Level.valueOf(logLevel));
+ // This causes all Loggers to re-fetch information from their LoggerConfig.
+ context.updateLoggers();
+ return getLoggerResponse(loggerConfig);
+ }
+
+ public static Map<String, Object> getLoggerInfo(String loggerName) {
+ LoggerContext context = (LoggerContext) LogManager.getContext(false);
+ Configuration config = context.getConfiguration();
+ LoggerConfig loggerConfig =
+ loggerName.equalsIgnoreCase(ROOT) ? config.getRootLogger() : config.getLoggerConfig(loggerName);
Review Comment:
Will `getLoggerConfig()` actually return null if it doesn't get a match? The documentation says "This will remove tokens from the package name as necessary or return the root LoggerConfig if no other matches were found".
--
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@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] Jackie-Jiang commented on a diff in pull request #9180: Adding logger utils and allow change logger level at runtime
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on code in PR #9180:
URL: https://github.com/apache/pinot/pull/9180#discussion_r940762275
##########
pinot-common/src/test/java/org/apache/pinot/common/utils/LoggerUtilsTest.java:
##########
@@ -0,0 +1,84 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.common.utils;
+
+import com.google.common.collect.ImmutableList;
+import java.util.List;
+import java.util.Map;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.testng.Assert;
+import org.testng.annotations.Test;
+
+
+public class LoggerUtilsTest {
+ private static final Logger LOGGER = LoggerFactory.getLogger(LoggerUtilsTest.class);
+
+ @Test
+ public void testGetAllLoggers() {
+ List<String> allLoggers = LoggerUtils.getAllLoggers();
+ Assert.assertEquals(allLoggers.size(), 1);
+ Assert.assertEquals(allLoggers.get(0), "root");
+ }
+
+ @Test
+ public void testGetLoggerInfo() {
+ Map<String, Object> rootLoggerInfo = LoggerUtils.getLoggerInfo("root");
+ Assert.assertEquals(rootLoggerInfo.get("name"), "root");
+ Assert.assertEquals(rootLoggerInfo.get("level"), "WARN");
+ Assert.assertNull(rootLoggerInfo.get("filter"));
+
+ Assert.assertNull(LoggerUtils.getLoggerInfo("notExistLogger"));
+ }
+
+ @Test
+ public void testChangeLoggerLevel() {
+ Assert.assertEquals(LoggerUtils.getLoggerInfo("root").get("level"), "WARN");
+ for (String level : ImmutableList.of("WARN", "INFO", "DEBUG", "ERROR", "WARN")) {
+ LoggerUtils.setLoggerLevel("root", level);
+ System.out.println("Current logger level is " + level);
Review Comment:
(minor) Remove the console output
##########
pinot-broker/src/main/java/org/apache/pinot/broker/api/resources/PinotBrokerLogger.java:
##########
@@ -0,0 +1,83 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.broker.api.resources;
+
+import io.swagger.annotations.Api;
+import io.swagger.annotations.ApiKeyAuthDefinition;
+import io.swagger.annotations.ApiOperation;
+import io.swagger.annotations.ApiParam;
+import io.swagger.annotations.Authorization;
+import io.swagger.annotations.SecurityDefinition;
+import io.swagger.annotations.SwaggerDefinition;
+import java.util.List;
+import java.util.Map;
+import javax.ws.rs.GET;
+import javax.ws.rs.PUT;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import javax.ws.rs.WebApplicationException;
+import javax.ws.rs.core.HttpHeaders;
+import javax.ws.rs.core.MediaType;
+import javax.ws.rs.core.Response;
+import org.apache.pinot.common.utils.LoggerUtils;
+
+import static org.apache.pinot.spi.utils.CommonConstants.SWAGGER_AUTHORIZATION_KEY;
+
+
+/**
+ * Logger resource.
+ */
+@Api(tags = "Logger", authorizations = {@Authorization(value = SWAGGER_AUTHORIZATION_KEY)})
+@SwaggerDefinition(securityDefinition = @SecurityDefinition(apiKeyAuthDefinitions = @ApiKeyAuthDefinition(name =
+ HttpHeaders.AUTHORIZATION, in = ApiKeyAuthDefinition.ApiKeyLocation.HEADER, key = SWAGGER_AUTHORIZATION_KEY)))
+@Path("/loggers/")
+public class PinotBrokerLogger {
+
+ @GET
+ @Path("")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "Get all the loggers", notes = "Return all the logger names")
+ public List<String> getLoggers(
+ @ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableNameWithType) {
+ return LoggerUtils.getAllLoggers();
+ }
+
+ @GET
+ @Path("{loggerName}")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "Get logger configs", notes = "Return logger info")
+ public Map<String, Object> getLogger(
+ @ApiParam(value = "Logger name", required = true) @PathParam("loggerName") String loggerName) {
+ Map<String, Object> loggerInfo = LoggerUtils.getLoggerInfo(loggerName);
+ if (loggerInfo == null) {
+ throw new WebApplicationException(String.format("Logger %s not found", loggerName), Response.Status.NOT_FOUND);
+ }
+ return loggerInfo;
+ }
+
+ @PUT
+ @Path("{loggerName}/level/{logLevel}")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "Set logger level", notes = "Set logger level for a given logger")
+ public Map<String, Object> setLoggerLevel(@ApiParam(value = "Logger name") @PathParam("loggerName") String loggerName,
+ @ApiParam(value = "Logger level") @PathParam("logLevel") String logLevel) {
Review Comment:
Consider making level a query param
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/LoggerUtils.java:
##########
@@ -0,0 +1,104 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.common.utils;
+
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+import javax.annotation.Nullable;
+import org.apache.logging.log4j.Level;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.core.LoggerContext;
+import org.apache.logging.log4j.core.config.Configuration;
+import org.apache.logging.log4j.core.config.LoggerConfig;
+
+
+/**
+ * Logger utils for process level logger management.
+ */
+public class LoggerUtils {
+ private static final String ROOT = "root";
+ private static final String NAME = "name";
+ private static final String LEVEL = "level";
+ private static final String FILTER = "filter";
+
+ private LoggerUtils() {
+ }
+
+ /**
+ * Set logger level at runtime.
+ * @param loggerName
+ * @param logLevel
+ * @return logger info
+ */
+ public static Map<String, Object> setLoggerLevel(String loggerName, String logLevel) {
+ LoggerContext context = (LoggerContext) LogManager.getContext(false);
+ Configuration config = context.getConfiguration();
+ if (!getAllLoggers().contains(loggerName)) {
+ throw new RuntimeException("Logger - " + loggerName + " not found");
+ }
+ LoggerConfig loggerConfig = getLoggerConfig(config, loggerName);
+ try {
+ loggerConfig.setLevel(Level.valueOf(logLevel));
+ } catch (Exception e) {
+ throw new RuntimeException("Unrecognized logger level - " + logLevel, e);
+ }
+ // This causes all Loggers to re-fetch information from their LoggerConfig.
+ context.updateLoggers();
+ return getLoggerResponse(loggerConfig);
+ }
+
+ /**
+ * Fetch logger info of name, level and filter.
+ * @param loggerName
+ * @return logger info
+ */
+ @Nullable
+ public static Map<String, Object> getLoggerInfo(String loggerName) {
+ LoggerContext context = (LoggerContext) LogManager.getContext(false);
+ Configuration config = context.getConfiguration();
+ if (!getAllLoggers().contains(loggerName)) {
+ return null;
+ }
+ LoggerConfig loggerConfig = getLoggerConfig(config, loggerName);
+ return getLoggerResponse(loggerConfig);
+ }
+
+ /**
+ * @return a list of all the logger names
+ */
+ public static List<String> getAllLoggers() {
+ LoggerContext context = (LoggerContext) LogManager.getContext(false);
+ Configuration config = context.getConfiguration();
+ return config.getLoggers().values().stream().map(LoggerConfig::toString).collect(Collectors.toList());
Review Comment:
What is the key of the `config.getLoggers()`? For the root config, seems the key is empty string. What about other logger configs?
Can it ever have duplicate values? If so, should we return a set instead?
##########
pinot-common/src/test/java/org/apache/pinot/common/utils/LoggerUtilsTest.java:
##########
@@ -0,0 +1,84 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.common.utils;
+
+import com.google.common.collect.ImmutableList;
+import java.util.List;
+import java.util.Map;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.testng.Assert;
+import org.testng.annotations.Test;
+
+
+public class LoggerUtilsTest {
+ private static final Logger LOGGER = LoggerFactory.getLogger(LoggerUtilsTest.class);
+
+ @Test
+ public void testGetAllLoggers() {
+ List<String> allLoggers = LoggerUtils.getAllLoggers();
+ Assert.assertEquals(allLoggers.size(), 1);
+ Assert.assertEquals(allLoggers.get(0), "root");
+ }
+
+ @Test
+ public void testGetLoggerInfo() {
+ Map<String, Object> rootLoggerInfo = LoggerUtils.getLoggerInfo("root");
+ Assert.assertEquals(rootLoggerInfo.get("name"), "root");
+ Assert.assertEquals(rootLoggerInfo.get("level"), "WARN");
+ Assert.assertNull(rootLoggerInfo.get("filter"));
+
+ Assert.assertNull(LoggerUtils.getLoggerInfo("notExistLogger"));
+ }
+
+ @Test
+ public void testChangeLoggerLevel() {
+ Assert.assertEquals(LoggerUtils.getLoggerInfo("root").get("level"), "WARN");
+ for (String level : ImmutableList.of("WARN", "INFO", "DEBUG", "ERROR", "WARN")) {
+ LoggerUtils.setLoggerLevel("root", level);
+ System.out.println("Current logger level is " + level);
+ loggerAllLevels();
Review Comment:
We should verify whether the log has the certain level enabled, e.g. `assertTrue(LOGGER.isWarnEnabled())`
##########
pinot-broker/src/main/java/org/apache/pinot/broker/api/resources/PinotBrokerLogger.java:
##########
@@ -0,0 +1,83 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.broker.api.resources;
+
+import io.swagger.annotations.Api;
+import io.swagger.annotations.ApiKeyAuthDefinition;
+import io.swagger.annotations.ApiOperation;
+import io.swagger.annotations.ApiParam;
+import io.swagger.annotations.Authorization;
+import io.swagger.annotations.SecurityDefinition;
+import io.swagger.annotations.SwaggerDefinition;
+import java.util.List;
+import java.util.Map;
+import javax.ws.rs.GET;
+import javax.ws.rs.PUT;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import javax.ws.rs.WebApplicationException;
+import javax.ws.rs.core.HttpHeaders;
+import javax.ws.rs.core.MediaType;
+import javax.ws.rs.core.Response;
+import org.apache.pinot.common.utils.LoggerUtils;
+
+import static org.apache.pinot.spi.utils.CommonConstants.SWAGGER_AUTHORIZATION_KEY;
+
+
+/**
+ * Logger resource.
+ */
+@Api(tags = "Logger", authorizations = {@Authorization(value = SWAGGER_AUTHORIZATION_KEY)})
+@SwaggerDefinition(securityDefinition = @SecurityDefinition(apiKeyAuthDefinitions = @ApiKeyAuthDefinition(name =
+ HttpHeaders.AUTHORIZATION, in = ApiKeyAuthDefinition.ApiKeyLocation.HEADER, key = SWAGGER_AUTHORIZATION_KEY)))
+@Path("/loggers/")
+public class PinotBrokerLogger {
+
+ @GET
+ @Path("")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "Get all the loggers", notes = "Return all the logger names")
+ public List<String> getLoggers(
+ @ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableNameWithType) {
Review Comment:
Why taking a table name?
##########
pinot-broker/src/main/java/org/apache/pinot/broker/api/resources/PinotBrokerLogger.java:
##########
@@ -0,0 +1,83 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.broker.api.resources;
+
+import io.swagger.annotations.Api;
+import io.swagger.annotations.ApiKeyAuthDefinition;
+import io.swagger.annotations.ApiOperation;
+import io.swagger.annotations.ApiParam;
+import io.swagger.annotations.Authorization;
+import io.swagger.annotations.SecurityDefinition;
+import io.swagger.annotations.SwaggerDefinition;
+import java.util.List;
+import java.util.Map;
+import javax.ws.rs.GET;
+import javax.ws.rs.PUT;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import javax.ws.rs.WebApplicationException;
+import javax.ws.rs.core.HttpHeaders;
+import javax.ws.rs.core.MediaType;
+import javax.ws.rs.core.Response;
+import org.apache.pinot.common.utils.LoggerUtils;
+
+import static org.apache.pinot.spi.utils.CommonConstants.SWAGGER_AUTHORIZATION_KEY;
+
+
+/**
+ * Logger resource.
+ */
+@Api(tags = "Logger", authorizations = {@Authorization(value = SWAGGER_AUTHORIZATION_KEY)})
+@SwaggerDefinition(securityDefinition = @SecurityDefinition(apiKeyAuthDefinitions = @ApiKeyAuthDefinition(name =
+ HttpHeaders.AUTHORIZATION, in = ApiKeyAuthDefinition.ApiKeyLocation.HEADER, key = SWAGGER_AUTHORIZATION_KEY)))
+@Path("/loggers/")
Review Comment:
(minor) Not sure if this works as expected. I remember running into some issues before, and we changed all of the root path as `"/"`
--
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@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] xiangfu0 commented on a diff in pull request #9180: Adding logger utils and allow change logger level at runtime
Posted by GitBox <gi...@apache.org>.
xiangfu0 commented on code in PR #9180:
URL: https://github.com/apache/pinot/pull/9180#discussion_r940675823
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/LoggerUtils.java:
##########
@@ -0,0 +1,80 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.common.utils;
+
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+import org.apache.logging.log4j.Level;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.core.LoggerContext;
+import org.apache.logging.log4j.core.config.Configuration;
+import org.apache.logging.log4j.core.config.LoggerConfig;
+
+
+/**
+ * Logger utils for process level logger management.
+ */
+public class LoggerUtils {
+ private static final String ROOT = "root";
+ private static final String NAME = "name";
+ private static final String LEVEL = "level";
+ private static final String FILTER = "filter";
+
+ private LoggerUtils() {
+ }
+
+ public static Map<String, Object> setLoggerLevel(String loggerName, String logLevel) {
+ LoggerContext context = (LoggerContext) LogManager.getContext(false);
+ Configuration config = context.getConfiguration();
+ LoggerConfig loggerConfig =
Review Comment:
it will give null, not root.
Added test to catch this.
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/LoggerUtils.java:
##########
@@ -0,0 +1,80 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.common.utils;
+
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+import org.apache.logging.log4j.Level;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.core.LoggerContext;
+import org.apache.logging.log4j.core.config.Configuration;
+import org.apache.logging.log4j.core.config.LoggerConfig;
+
+
+/**
+ * Logger utils for process level logger management.
+ */
+public class LoggerUtils {
+ private static final String ROOT = "root";
+ private static final String NAME = "name";
+ private static final String LEVEL = "level";
+ private static final String FILTER = "filter";
+
+ private LoggerUtils() {
+ }
+
+ public static Map<String, Object> setLoggerLevel(String loggerName, String logLevel) {
+ LoggerContext context = (LoggerContext) LogManager.getContext(false);
+ Configuration config = context.getConfiguration();
+ LoggerConfig loggerConfig =
+ loggerName.equalsIgnoreCase(ROOT) ? config.getRootLogger() : config.getLoggerConfig(loggerName);
+ loggerConfig.setLevel(Level.valueOf(logLevel));
Review Comment:
add the test
--
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@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] xiangfu0 commented on a diff in pull request #9180: Adding logger utils and allow change logger level at runtime
Posted by GitBox <gi...@apache.org>.
xiangfu0 commented on code in PR #9180:
URL: https://github.com/apache/pinot/pull/9180#discussion_r940684024
##########
pinot-broker/src/main/java/org/apache/pinot/broker/api/resources/PinotBrokerLogger.java:
##########
@@ -0,0 +1,83 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.broker.api.resources;
+
+import io.swagger.annotations.Api;
+import io.swagger.annotations.ApiKeyAuthDefinition;
+import io.swagger.annotations.ApiOperation;
+import io.swagger.annotations.ApiParam;
+import io.swagger.annotations.Authorization;
+import io.swagger.annotations.SecurityDefinition;
+import io.swagger.annotations.SwaggerDefinition;
+import java.util.List;
+import java.util.Map;
+import javax.ws.rs.GET;
+import javax.ws.rs.PUT;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import javax.ws.rs.WebApplicationException;
+import javax.ws.rs.core.HttpHeaders;
+import javax.ws.rs.core.MediaType;
+import javax.ws.rs.core.Response;
+import org.apache.pinot.common.utils.LoggerUtils;
+
+import static org.apache.pinot.spi.utils.CommonConstants.SWAGGER_AUTHORIZATION_KEY;
+
+
+/**
+ * Logger resource.
+ */
+@Api(tags = "Logger", authorizations = {@Authorization(value = SWAGGER_AUTHORIZATION_KEY)})
+@SwaggerDefinition(securityDefinition = @SecurityDefinition(apiKeyAuthDefinitions = @ApiKeyAuthDefinition(name =
+ HttpHeaders.AUTHORIZATION, in = ApiKeyAuthDefinition.ApiKeyLocation.HEADER, key = SWAGGER_AUTHORIZATION_KEY)))
+@Path("/loggers/")
+public class PinotBrokerLogger {
+
+ @GET
+ @Path("")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "Get all the loggers", notes = "Return all the logger names")
+ public List<String> getLoggers(
+ @ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableNameWithType) {
+ return LoggerUtils.getAllLoggers();
+ }
+
+ @GET
+ @Path("{loggerName}")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "Get logger configs", notes = "Return logger info")
+ public Map<String, Object> getLogger(
+ @ApiParam(value = "Logger name", required = true) @PathParam("loggerName") String loggerName) {
Review Comment:
The logger name is defined by the user: e.g. this log4j2.xml: https://github.com/apache/pinot/blob/master/pinot-tools/src/main/resources/log4j2.xml#L48
It will have 3 loggers: ["root","org.reflections","org.apache.pinot.tools.admin"]
Note that appenders level are not changeable, which means the appender `console`'s level is not changeable from this API.
--
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@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] xiangfu0 commented on a diff in pull request #9180: Adding logger utils and allow change logger level at runtime
Posted by GitBox <gi...@apache.org>.
xiangfu0 commented on code in PR #9180:
URL: https://github.com/apache/pinot/pull/9180#discussion_r940778301
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/LoggerUtils.java:
##########
@@ -0,0 +1,104 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.common.utils;
+
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+import javax.annotation.Nullable;
+import org.apache.logging.log4j.Level;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.core.LoggerContext;
+import org.apache.logging.log4j.core.config.Configuration;
+import org.apache.logging.log4j.core.config.LoggerConfig;
+
+
+/**
+ * Logger utils for process level logger management.
+ */
+public class LoggerUtils {
+ private static final String ROOT = "root";
+ private static final String NAME = "name";
+ private static final String LEVEL = "level";
+ private static final String FILTER = "filter";
+
+ private LoggerUtils() {
+ }
+
+ /**
+ * Set logger level at runtime.
+ * @param loggerName
+ * @param logLevel
+ * @return logger info
+ */
+ public static Map<String, Object> setLoggerLevel(String loggerName, String logLevel) {
+ LoggerContext context = (LoggerContext) LogManager.getContext(false);
+ Configuration config = context.getConfiguration();
+ if (!getAllLoggers().contains(loggerName)) {
+ throw new RuntimeException("Logger - " + loggerName + " not found");
+ }
+ LoggerConfig loggerConfig = getLoggerConfig(config, loggerName);
+ try {
+ loggerConfig.setLevel(Level.valueOf(logLevel));
+ } catch (Exception e) {
+ throw new RuntimeException("Unrecognized logger level - " + logLevel, e);
+ }
+ // This causes all Loggers to re-fetch information from their LoggerConfig.
+ context.updateLoggers();
+ return getLoggerResponse(loggerConfig);
+ }
+
+ /**
+ * Fetch logger info of name, level and filter.
+ * @param loggerName
+ * @return logger info
+ */
+ @Nullable
+ public static Map<String, Object> getLoggerInfo(String loggerName) {
+ LoggerContext context = (LoggerContext) LogManager.getContext(false);
+ Configuration config = context.getConfiguration();
+ if (!getAllLoggers().contains(loggerName)) {
+ return null;
+ }
+ LoggerConfig loggerConfig = getLoggerConfig(config, loggerName);
+ return getLoggerResponse(loggerConfig);
+ }
+
+ /**
+ * @return a list of all the logger names
+ */
+ public static List<String> getAllLoggers() {
+ LoggerContext context = (LoggerContext) LogManager.getContext(false);
+ Configuration config = context.getConfiguration();
+ return config.getLoggers().values().stream().map(LoggerConfig::toString).collect(Collectors.toList());
Review Comment:
no duplicate, logger actually give unmatched key to root logger, so we explicitly specify root here.
--
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@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter commented on pull request #9180: Adding ability to update logger level
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #9180:
URL: https://github.com/apache/pinot/pull/9180#issuecomment-1207383018
# [Codecov](https://codecov.io/gh/apache/pinot/pull/9180?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#9180](https://codecov.io/gh/apache/pinot/pull/9180?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ad54a11) into [master](https://codecov.io/gh/apache/pinot/commit/449f89a3d7b809daa5504147d283e763d7f44a53?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (449f89a) will **increase** coverage by `38.56%`.
> The diff coverage is `n/a`.
```diff
@@ Coverage Diff @@
## master #9180 +/- ##
=============================================
+ Coverage 28.49% 67.05% +38.56%
- Complexity 53 4812 +4759
=============================================
Files 1836 1377 -459
Lines 98103 71699 -26404
Branches 14897 11475 -3422
=============================================
+ Hits 27956 48081 +20125
+ Misses 67460 20147 -47313
- Partials 2687 3471 +784
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| unittests1 | `67.05% <ø> (?)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/9180?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...va/org/apache/pinot/core/routing/RoutingTable.java](https://codecov.io/gh/apache/pinot/pull/9180/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yb3V0aW5nL1JvdXRpbmdUYWJsZS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...va/org/apache/pinot/common/config/NettyConfig.java](https://codecov.io/gh/apache/pinot/pull/9180/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vY29uZmlnL05ldHR5Q29uZmlnLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...a/org/apache/pinot/common/metrics/MinionMeter.java](https://codecov.io/gh/apache/pinot/pull/9180/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9NaW5pb25NZXRlci5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...g/apache/pinot/common/metrics/ControllerMeter.java](https://codecov.io/gh/apache/pinot/pull/9180/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Db250cm9sbGVyTWV0ZXIuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../apache/pinot/common/metrics/BrokerQueryPhase.java](https://codecov.io/gh/apache/pinot/pull/9180/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Ccm9rZXJRdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../apache/pinot/common/metrics/MinionQueryPhase.java](https://codecov.io/gh/apache/pinot/pull/9180/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9NaW5pb25RdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ache/pinot/server/access/AccessControlFactory.java](https://codecov.io/gh/apache/pinot/pull/9180/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VydmVyL2FjY2Vzcy9BY2Nlc3NDb250cm9sRmFjdG9yeS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...he/pinot/common/messages/SegmentReloadMessage.java](https://codecov.io/gh/apache/pinot/pull/9180/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWVzc2FnZXMvU2VnbWVudFJlbG9hZE1lc3NhZ2UuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...he/pinot/common/messages/TableDeletionMessage.java](https://codecov.io/gh/apache/pinot/pull/9180/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWVzc2FnZXMvVGFibGVEZWxldGlvbk1lc3NhZ2UuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...pinot/core/data/manager/realtime/TimerService.java](https://codecov.io/gh/apache/pinot/pull/9180/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvVGltZXJTZXJ2aWNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [1613 more](https://codecov.io/gh/apache/pinot/pull/9180/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
:mega: Codecov can now indicate which changes are the most critical in Pull Requests. [Learn more](https://about.codecov.io/product/feature/runtime-insights/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
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@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org