Skip to content

Commit b6047b2

Browse files
authored
Set SlowLog logging to TRACE in tests (#114344) (#114349)
The tests depend on the SlowLog loggers running at TRACE level but were not setting the level themselves. Instead they relied on the SlowLog setting the level to trace internally when it was created. If something else globally adjusted log levels between the time the SlowLog loggers were created and the tests ran, the tests could fail. And in fact, `ScopedSettingsTest.testFallbackToLoggerLevel` was updating the root log level, which had the side effect of updating the SlowLog level. In #112183 SlowLog's log initialization was made static, which opened up its test to failure when ScopedSettingsTest ran before a SlowLog test in the same JVM. I do not know if the intention of the SlowLog is that it overrides the global log level and should always be set at TRACE, in which case this fix is incorrect. It seems surprising, but I don't know why else SlowLog would explicitly initialize itself to TRACE. However, if that was the intention, the code was already at risk due to having no guard against being changed by Loggers.setLevel on an ancestor log. The change in this PR is at least not a regression in that behaviour. It does no longer start out at TRACE however, which is a change in behaviour.
1 parent 33b55df commit b6047b2

File tree

4 files changed

+15
-14
lines changed

4 files changed

+15
-14
lines changed

server/src/main/java/org/elasticsearch/index/IndexingSlowLog.java

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,11 @@
99

1010
package org.elasticsearch.index;
1111

12-
import org.apache.logging.log4j.Level;
1312
import org.apache.logging.log4j.LogManager;
1413
import org.apache.logging.log4j.Logger;
1514
import org.apache.logging.log4j.util.StringBuilders;
1615
import org.elasticsearch.common.Strings;
1716
import org.elasticsearch.common.logging.ESLogMessage;
18-
import org.elasticsearch.common.logging.Loggers;
1917
import org.elasticsearch.common.settings.Setting;
2018
import org.elasticsearch.common.settings.Setting.Property;
2119
import org.elasticsearch.common.xcontent.XContentHelper;
@@ -92,9 +90,6 @@ public final class IndexingSlowLog implements IndexingOperationListener {
9290
);
9391

9492
private static final Logger indexLogger = LogManager.getLogger(INDEX_INDEXING_SLOWLOG_PREFIX + ".index");
95-
static {
96-
Loggers.setLevel(indexLogger, Level.TRACE);
97-
}
9893

9994
private final Index index;
10095

server/src/main/java/org/elasticsearch/index/SearchSlowLog.java

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,9 @@
99

1010
package org.elasticsearch.index;
1111

12-
import org.apache.logging.log4j.Level;
1312
import org.apache.logging.log4j.LogManager;
1413
import org.apache.logging.log4j.Logger;
1514
import org.elasticsearch.common.logging.ESLogMessage;
16-
import org.elasticsearch.common.logging.Loggers;
1715
import org.elasticsearch.common.settings.Setting;
1816
import org.elasticsearch.common.settings.Setting.Property;
1917
import org.elasticsearch.core.TimeValue;
@@ -47,11 +45,6 @@ public final class SearchSlowLog implements SearchOperationListener {
4745
private static final Logger queryLogger = LogManager.getLogger(INDEX_SEARCH_SLOWLOG_PREFIX + ".query");
4846
private static final Logger fetchLogger = LogManager.getLogger(INDEX_SEARCH_SLOWLOG_PREFIX + ".fetch");
4947

50-
static {
51-
Loggers.setLevel(queryLogger, Level.TRACE);
52-
Loggers.setLevel(fetchLogger, Level.TRACE);
53-
}
54-
5548
private final SlowLogFieldProvider slowLogFieldProvider;
5649

5750
public static final Setting<Boolean> INDEX_SEARCH_SLOWLOG_INCLUDE_USER_SETTING = Setting.boolSetting(

server/src/test/java/org/elasticsearch/index/IndexingSlowLogTests.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,18 +58,23 @@ public class IndexingSlowLogTests extends ESTestCase {
5858
static MockAppender appender;
5959
static Releasable appenderRelease;
6060
static Logger testLogger1 = LogManager.getLogger(IndexingSlowLog.INDEX_INDEXING_SLOWLOG_PREFIX + ".index");
61+
static Level origLogLevel = testLogger1.getLevel();
6162

6263
@BeforeClass
6364
public static void init() throws IllegalAccessException {
6465
appender = new MockAppender("trace_appender");
6566
appender.start();
6667
Loggers.addAppender(testLogger1, appender);
68+
69+
Loggers.setLevel(testLogger1, Level.TRACE);
6770
}
6871

6972
@AfterClass
7073
public static void cleanup() {
71-
appender.stop();
7274
Loggers.removeAppender(testLogger1, appender);
75+
appender.stop();
76+
77+
Loggers.setLevel(testLogger1, origLogLevel);
7378
}
7479

7580
public void testLevelPrecedence() {

server/src/test/java/org/elasticsearch/index/SearchSlowLogTests.java

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,20 +50,28 @@ public class SearchSlowLogTests extends ESSingleNodeTestCase {
5050
static MockAppender appender;
5151
static Logger queryLog = LogManager.getLogger(SearchSlowLog.INDEX_SEARCH_SLOWLOG_PREFIX + ".query");
5252
static Logger fetchLog = LogManager.getLogger(SearchSlowLog.INDEX_SEARCH_SLOWLOG_PREFIX + ".fetch");
53+
static Level origQueryLogLevel = queryLog.getLevel();
54+
static Level origFetchLogLevel = fetchLog.getLevel();
5355

5456
@BeforeClass
5557
public static void init() throws IllegalAccessException {
5658
appender = new MockAppender("trace_appender");
5759
appender.start();
5860
Loggers.addAppender(queryLog, appender);
5961
Loggers.addAppender(fetchLog, appender);
62+
63+
Loggers.setLevel(queryLog, Level.TRACE);
64+
Loggers.setLevel(fetchLog, Level.TRACE);
6065
}
6166

6267
@AfterClass
6368
public static void cleanup() {
64-
appender.stop();
6569
Loggers.removeAppender(queryLog, appender);
6670
Loggers.removeAppender(fetchLog, appender);
71+
appender.stop();
72+
73+
Loggers.setLevel(queryLog, origQueryLogLevel);
74+
Loggers.setLevel(fetchLog, origFetchLogLevel);
6775
}
6876

6977
@Override

0 commit comments

Comments
 (0)