Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade log4j 1.2 to 2.16.0 #1364

Merged
merged 11 commits into from
Dec 14, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion frontend/archive/build.gradle
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
dependencies {
api "commons-io:commons-io:2.6"
api "org.slf4j:slf4j-api:${slf4j_api_version}"
api "org.slf4j:slf4j-log4j12:${slf4j_log4j12_version}"
api "org.apache.logging.log4j:log4j-slf4j-impl:${slf4j_log4j_version}"
api "com.google.code.gson:gson:${gson_version}"

testImplementation "commons-cli:commons-cli:${commons_cli_version}"
Expand Down
2 changes: 1 addition & 1 deletion frontend/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ configure(javaProjects()) {
test.finalizedBy(project.tasks.jacocoTestReport)

compileJava {
options.compilerArgs << "-Xlint:all,-options,-static" << "-Werror"
options.compilerArgs << "-proc:none" << "-Xlint:all,-options,-static" << "-Werror"
}

jacocoTestCoverageVerification {
Expand Down
6 changes: 3 additions & 3 deletions frontend/gradle.properties
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@ commons_cli_version=1.3.1
gson_version=2.8.5
prometheus_version=0.9.0
netty_version=4.1.53.Final
slf4j_api_version=1.7.25
slf4j_log4j12_version=1.7.25
slf4j_api_version=1.7.32
slf4j_log4j_version=2.16.0
testng_version=7.1.0
torchserve_sdk_version=0.0.4
snakeyaml_version=1.8
grpc_version=1.31.1
protoc_version=3.13.0
protoc_version=3.13.0
13 changes: 8 additions & 5 deletions frontend/server/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ dependencies {
implementation project(":archive")
implementation "commons-cli:commons-cli:${commons_cli_version}"
implementation "org.pytorch:torchserve-plugins-sdk:${torchserve_sdk_version}"
implementation "org.apache.logging.log4j:log4j-core:${slf4j_log4j_version}"
testImplementation "org.testng:testng:${testng_version}"
}

Expand All @@ -26,8 +27,10 @@ jar {
exclude "META-INF//NOTICE*"
}

test.doFirst {
systemProperty "tsConfigFile", 'src/test/resources/config.properties'
systemProperty "METRICS_LOCATION","build/logs"
systemProperty "LOG_LOCATION","build/logs"
}
test {
doFirst {
systemProperty "tsConfigFile", 'src/test/resources/config.properties'
}
environment "METRICS_LOCATION","build/logs"
environment "LOG_LOCATION","build/logs"
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@

public class GRPCJob extends Job {
private static final Logger logger = LoggerFactory.getLogger(Job.class);
private static final org.apache.log4j.Logger loggerTsMetrics =
org.apache.log4j.Logger.getLogger(ConfigManager.MODEL_SERVER_METRICS_LOGGER);
private static final Logger loggerTsMetrics =
LoggerFactory.getLogger(ConfigManager.MODEL_SERVER_METRICS_LOGGER);
private static final Dimension DIMENSION = new Dimension("Level", "Host");

private StreamObserver<PredictionResponse> predictionResponseObserver;
Expand Down Expand Up @@ -55,6 +55,7 @@ public void response(
TimeUnit.MILLISECONDS.convert(
getScheduled() - getBegin(), TimeUnit.NANOSECONDS));
loggerTsMetrics.info(
"{}",
new Metric(
"QueueTime",
queueTime,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@
public class RestJob extends Job {

private static final Logger logger = LoggerFactory.getLogger(Job.class);
private static final org.apache.log4j.Logger loggerTsMetrics =
org.apache.log4j.Logger.getLogger(ConfigManager.MODEL_SERVER_METRICS_LOGGER);
private static final Logger loggerTsMetrics =
LoggerFactory.getLogger(ConfigManager.MODEL_SERVER_METRICS_LOGGER);
private static final Dimension DIMENSION = new Dimension("Level", "Host");

private ChannelHandlerContext ctx;
Expand Down Expand Up @@ -87,6 +87,7 @@ public void response(
TimeUnit.MILLISECONDS.convert(
getScheduled() - getBegin(), TimeUnit.NANOSECONDS));
loggerTsMetrics.info(
"{}",
new Metric(
"QueueTime",
queueTime,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@
public class MetricCollector implements Runnable {

private static final Logger logger = LoggerFactory.getLogger(MetricCollector.class);
private static final org.apache.log4j.Logger loggerMetrics =
org.apache.log4j.Logger.getLogger(ConfigManager.MODEL_SERVER_METRICS_LOGGER);
private static final Logger loggerMetrics =
LoggerFactory.getLogger(ConfigManager.MODEL_SERVER_METRICS_LOGGER);
private ConfigManager configManager;

public MetricCollector(ConfigManager configManager) {
Expand Down Expand Up @@ -77,7 +77,7 @@ public void run() {
if (metric == null) {
logger.warn("Parse metrics failed: " + line);
} else {
loggerMetrics.info(metric);
loggerMetrics.info("{}", metric);
metricsSystem.add(metric);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,9 @@
import org.apache.commons.cli.Option;
import org.apache.commons.cli.Options;
import org.apache.commons.io.IOUtils;
import org.apache.log4j.Appender;
import org.apache.log4j.AsyncAppender;
import org.apache.log4j.Logger;
import org.pytorch.serve.servingsdk.snapshot.SnapshotSerializer;
import org.pytorch.serve.snapshot.SnapshotSerializerFactory;
import org.slf4j.LoggerFactory;

public final class ConfigManager {
// Variables that can be configured through config.properties and Environment Variables
Expand Down Expand Up @@ -692,30 +690,9 @@ private File findTsHome() {
}

private void enableAsyncLogging() {
enableAsyncLogging(Logger.getRootLogger());
enableAsyncLogging(Logger.getLogger(MODEL_METRICS_LOGGER));
enableAsyncLogging(Logger.getLogger(MODEL_LOGGER));
enableAsyncLogging(Logger.getLogger(MODEL_SERVER_METRICS_LOGGER));
enableAsyncLogging(Logger.getLogger("ACCESS_LOG"));
enableAsyncLogging(Logger.getLogger("org.pytorch.serve"));
}

private void enableAsyncLogging(Logger logger) {
AsyncAppender asyncAppender = new AsyncAppender();

@SuppressWarnings("unchecked")
Enumeration<Appender> en = logger.getAllAppenders();
while (en.hasMoreElements()) {
Appender appender = en.nextElement();
if (appender instanceof AsyncAppender) {
// already async
return;
}

logger.removeAppender(appender);
asyncAppender.addAppender(appender);
}
logger.addAppender(asyncAppender);
System.setProperty(
"log4j2.contextSelector",
"org.apache.logging.log4j.core.async.AsyncLoggerContextSelector");
}

public HashMap<String, String> getBackendConfiguration() {
Expand Down Expand Up @@ -836,14 +813,12 @@ public int getJsonIntValue(String modelName, String version, String element, int
value = defaultVal;
}
} catch (ClassCastException | IllegalStateException e) {
Logger.getRootLogger()
LoggerFactory.getLogger(ConfigManager.class)
.error(
"Invalid value for model: "
+ modelName
+ ":"
+ version
+ ", parameter: "
+ element);
"Invalid value for model: {}:{}, parameter: {}",
modelName,
version,
element);
return defaultVal;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,8 @@ public final class NettyUtils {
ConfigManager.getInstance().getHostName(),
DIMENSION);

private static final org.apache.log4j.Logger loggerTsMetrics =
org.apache.log4j.Logger.getLogger(ConfigManager.MODEL_SERVER_METRICS_LOGGER);
private static final Logger loggerTsMetrics =
LoggerFactory.getLogger(ConfigManager.MODEL_SERVER_METRICS_LOGGER);

private NettyUtils() {}

Expand Down Expand Up @@ -157,11 +157,11 @@ public static void sendHttpResponse(
}
int code = resp.status().code();
if (code >= 200 && code < 300) {
loggerTsMetrics.info(REQUESTS_2_XX);
loggerTsMetrics.info("{}", REQUESTS_2_XX);
} else if (code >= 400 && code < 500) {
loggerTsMetrics.info(REQUESTS_4_XX);
loggerTsMetrics.info("{}", REQUESTS_4_XX);
} else {
loggerTsMetrics.info(REQUESTS_5_XX);
loggerTsMetrics.info("{}", REQUESTS_5_XX);
}

String allowedOrigin = configManager.getCorsAllowedOrigin();
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,11 +1,23 @@
package org.pytorch.serve.util.logging;

import org.apache.log4j.PatternLayout;
import org.apache.log4j.spi.LoggingEvent;
import org.apache.logging.log4j.core.Layout;
import org.apache.logging.log4j.core.LogEvent;
import org.apache.logging.log4j.core.config.Node;
import org.apache.logging.log4j.core.config.plugins.Plugin;
import org.apache.logging.log4j.core.layout.AbstractStringLayout;
import org.apache.logging.log4j.message.Message;
import org.pytorch.serve.metrics.Dimension;
import org.pytorch.serve.metrics.Metric;

public class QLogLayout extends PatternLayout {
@Plugin(
name = "QLogLayout",
category = Node.CATEGORY,
elementType = Layout.ELEMENT_TYPE,
printObject = true)
public class QLogLayout extends AbstractStringLayout {
public QLogLayout() {
super(null, null, null);
}

/**
* Model server also supports query log formatting.
Expand Down Expand Up @@ -61,67 +73,68 @@ public class QLogLayout extends PatternLayout {
* @return
*/
@Override
public String format(LoggingEvent event) {
Object eventMessage = event.getMessage();
public String toSerializable(LogEvent event) {
Message eventMessage = event.getMessage();
if (eventMessage == null || eventMessage.getParameters() == null) {
return null;
}
String programName =
getStringOrDefault(System.getenv("MXNETMODELSERVER_PROGRAM"), "MXNetModelServer");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just noticed this in the code - separately, this could be fixed to use 'TS' in another PR/issue.

String domain = getStringOrDefault(System.getenv("DOMAIN"), "Unknown");

long currentTimeInSec = System.currentTimeMillis() / 1000;
Object[] parameters = eventMessage.getParameters();

if (eventMessage == null) {
return null;
}
if (eventMessage instanceof Metric) {
String marketPlace = System.getenv("REALM");
StringBuilder stringBuilder = new StringBuilder();
for (Object obj : parameters) {
if (obj instanceof Metric) {
Metric metric = (Metric) obj;
String marketPlace = System.getenv("REALM");

StringBuilder stringBuilder = new StringBuilder();
Metric metric = (Metric) eventMessage;
stringBuilder.append("HostName=").append(metric.getHostName());
stringBuilder.append("HostName=").append(metric.getHostName());

if (metric.getRequestId() != null && !metric.getRequestId().isEmpty()) {
stringBuilder.append("\nRequestId=").append(metric.getRequestId());
}
if (metric.getRequestId() != null && !metric.getRequestId().isEmpty()) {
stringBuilder.append("\nRequestId=").append(metric.getRequestId());
}

// Marketplace format should be : <programName>:<domain>:<realm>
if (marketPlace != null && !marketPlace.isEmpty()) {
stringBuilder
.append("\nMarketplace=")
.append(programName)
.append(':')
.append(domain)
.append(':')
.append(marketPlace);
}
// Marketplace format should be : <programName>:<domain>:<realm>
if (marketPlace != null && !marketPlace.isEmpty()) {
stringBuilder
.append("\nMarketplace=")
.append(programName)
.append(':')
.append(domain)
.append(':')
.append(marketPlace);
}

stringBuilder
.append("\nStartTime=")
.append(
getStringOrDefault(
metric.getTimestamp(), Long.toString(currentTimeInSec)));
stringBuilder
.append("\nStartTime=")
.append(
getStringOrDefault(
metric.getTimestamp(), Long.toString(currentTimeInSec)));

stringBuilder
.append("\nProgram=")
.append(programName)
.append("\nMetrics=")
.append(metric.getMetricName())
.append('=')
.append(metric.getValue())
.append(' ')
.append(metric.getUnit());
for (Dimension dimension : metric.getDimensions()) {
stringBuilder
.append("\nProgram=")
.append(programName)
.append("\nMetrics=")
.append(metric.getMetricName())
.append('=')
.append(metric.getValue())
.append(' ')
.append(dimension.getName())
.append('|')
.append(dimension.getValue())
.append(' ');
.append(metric.getUnit());
for (Dimension dimension : metric.getDimensions()) {
stringBuilder
.append(' ')
.append(dimension.getName())
.append('|')
.append(dimension.getValue())
.append(' ');
}
stringBuilder.append("\nEOE\n");
}
stringBuilder.append("\nEOE\n");

return stringBuilder.toString();
}
return eventMessage.toString();
return stringBuilder.toString();
}

private static String getStringOrDefault(String val, String defVal) {
Expand Down
Loading