-
Notifications
You must be signed in to change notification settings - Fork 467
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
Issue 23631 create global log4j #23964
base: main
Are you sure you want to change the base?
Conversation
Integration Tests [postgres] Report 408 files - 1 408 suites - 1 58m 59s ⏱️ - 10m 16s For more details on these failures, see this check. Results for commit aa5f866. ± Comparison against base commit adfff75. ♻️ This comment has been updated with latest results. |
2bb3864
to
5a9ecd5
Compare
ab55184
to
66e3d46
Compare
c13fd20
to
ef29177
Compare
SonarQube Quality Gate Reliability Rating on New Code (is worse than A) See analysis details on SonarQube Fix issues before they fail your Quality Gate with SonarLint in your IDE. |
This PR is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 7 days. |
This PR was closed because it has been stalled with no activity. |
.recordStats() | ||
.expireAfter(new Expiry<String, Object>() { | ||
public long expireAfterCreate(String key, Object value, long currentTime) { | ||
long ttlInSeconds; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can avoid an else by setting Long.MAX_VALUE here
size = Config.getIntProperty("cache." + DEFAULT_CACHE + ".size", 100); | ||
if(value instanceof Expirable | ||
&& ((Expirable) value).getTtl() > 0) { | ||
ttlInSeconds = ((Expirable) value).getTtl(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
my personal preference is to use Expirable.cast(value).getTtl()
@@ -88,6 +88,29 @@ public boolean areRecordsLeftToIndex() throws DotDataException { | |||
public long recordsInQueue() throws DotDataException { | |||
return recordsInQueue(DbConnectionFactory.getConnection()); | |||
} | |||
|
|||
@Override | |||
public boolean waitForEmptyQueue(int maxWaitSeconds) throws DotDataException { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
set param to final
// This should be replaced with a more efficient solution that uses a notification mechanism | ||
// but that would require a more complex implementation and this is required for some tests currently | ||
try { | ||
long recordsInQueue = recordsInQueue(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is ok, but wondering if we cannot eventually change this kind of infinite stuff to Stream or Iterable
@@ -269,6 +281,14 @@ public static boolean connectionExists() { | |||
return isCreated; | |||
} // connectionExists. | |||
|
|||
public static Optional<Throwable> getCreatorStack() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
doc it is a public method
@@ -269,6 +281,14 @@ public static boolean connectionExists() { | |||
return isCreated; | |||
} // connectionExists. | |||
|
|||
public static Optional<Throwable> getCreatorStack() { | |||
if (connectionExists()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
my preference is one line return return connectionExists()? Optional.ofNullable(creatorStack.get()): Optional.empty();
@@ -457,6 +492,10 @@ public static void closeConnection(String ds) { | |||
|
|||
} | |||
|
|||
public static void setConnectionLogger(boolean connectionLogger) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
doc me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
set arg to final
/** | ||
* Attempts to find a session associated with the Thread. If there isn't a | ||
* session, it will create one. | ||
*/ | ||
public static Session getSession() { | ||
public static Session getSession(boolean createIfClosed) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
set to final arg
props.setProperty(key, value); | ||
} | ||
} | ||
|
||
public static Map<String,String> getOverrides() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
docme
return Map.copyOf(testOverrideTracker); | ||
} | ||
|
||
public static Map<String,String> compareOverrides(Map<String,String> before) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
doc me and set to final the arg
} | ||
|
||
public static Map<String,String> compareOverrides(Map<String,String> before) { | ||
Map<String,String> after = getOverrides(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
interesting method, I think it would be a good candidate to CollectionUtils
Updated pull request from #23632 Issue #23631 Enable log configuration by setting environment variable USE_GLOBAL_LOG4J=true
Default logging config found in log4j2/conf/log4j2-tomcat.xml
bin/setclasspath.sh or bin/setclasspath.bat contains logic to setup
if [ "$USE_GLOBAL_LOG4J" = true ]; then
echo Using Global Log4j
CLASSPATH="$CATALINA_HOME/log4j2/lib/*:$CATALINA_HOME/log4j2/conf$CLASSPATH"
JAVA_OPTS=" -DLog4jContextSelector=org.apache.logging.log4j.core.async.BasicAsyncLoggerContextSelector $JAVA_OPTS"
JAVA_OPTS=" -Djava.util.logging.manager=org.apache.logging.log4j.jul.LogManager $JAVA_OPTS"
fi
default Context selector creates a new context per Classloader and requires separate configuration for each. BasicContextSelection keeps the same context and configuration for all Classloaders, for each there is an Async version that makes all the loggers asynchronous and improves performance.
The log4j jars have been added to $CATALINA_HOME/log4j2/lib to support slf4j, commons-logging, Jul logging log4j 1. A log4j2-appserver jar is set up to replace the standard tomcat logging. Although the log4j2-appserver replaces the jul logging in tomcat directly Some plugin code uses java.util.logging and the LogManger needs to be set to pass this logging back to log4j.
By default the logfile is picked up from the classpath and found in CATALINA_HOME/log4j2/conf see
https://logging.apache.org/log4j/2.x/log4j-appserver/index.html
The log file could be specified with a jvm option -Dlog4j.configurationFile=path/to/log4j2.xml or map the file/folder in docker.
Read configuration guide https://logging.apache.org/log4j/2.x/manual/configuration.html on how to modify the config,https://logging.apache.org/log4j/2.x/manual/configuration.html It is currently set to only log to stdout. for docker.