Merge "Make tracing.performanceLogging default disabled" into stable-3.5

6 files changed, 18 insertions, 9 deletions

diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index 868d32cbc1..be801f3e00 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -5262,13 +5262,13 @@ operations are collected in memory while a request is running. At the
 end of the request the performance events are handed over to the
 link:dev-plugins.html#performance-logger[PerformanceLogger] plugins.
 This means if performance logging is enabled, the memory footprint of
-requests is slightly increased.
+requests can be markedly increased.
+In one recorded case the impact was an overall heap increase of 40%
+(using the metrics-reporter-graphite plugin), in other instances the
+heap increase wasn't nearly as dramatic and the impact is most likely
+dependent on which plugin is used.
 +
-This setting has no effect if no
-link:dev-plugins.html#performance-logger[PerformanceLogger] plugins are
-installed, because then performance logging is always disabled.
-+
-By default, true.
+By default, false.
 
 [[tracing.exportPerformanceMetrics]]tracing.exportPerformanceMetrics::
 +
diff --git a/java/com/google/gerrit/server/logging/PerformanceLogContext.java b/java/com/google/gerrit/server/logging/PerformanceLogContext.java
index 65e033b158..90e716f9a3 100644
--- a/java/com/google/gerrit/server/logging/PerformanceLogContext.java
+++ b/java/com/google/gerrit/server/logging/PerformanceLogContext.java
@@ -56,7 +56,7 @@ public class PerformanceLogContext implements AutoCloseable {
     // Do not create performance log entries if performance logging is disabled or if no
     // PerformanceLogger is registered.
     boolean enablePerformanceLogging =
-        gerritConfig.getBoolean("tracing", "performanceLogging", true);
+        gerritConfig.getBoolean("tracing", "performanceLogging", false);
     LoggingContext.getInstance()
         .performanceLogging(
             enablePerformanceLogging && !Iterables.isEmpty(performanceLoggers.entries()));
diff --git a/javatests/com/google/gerrit/acceptance/rest/TraceIT.java b/javatests/com/google/gerrit/acceptance/rest/TraceIT.java
index 7e40b2b912..64e37629ac 100644
--- a/javatests/com/google/gerrit/acceptance/rest/TraceIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/TraceIT.java
@@ -374,6 +374,7 @@ public class TraceIT extends AbstractDaemonTest {
   }
 
   @Test
+  @GerritConfig(name = "tracing.performanceLogging", value = "true")
   public void performanceLoggingForRestCall() throws Exception {
     PerformanceLogger testPerformanceLogger = mock(PerformanceLogger.class);
     try (Registration registration =
@@ -385,6 +386,7 @@ public class TraceIT extends AbstractDaemonTest {
   }
 
   @Test
+  @GerritConfig(name = "tracing.performanceLogging", value = "true")
   public void performanceLoggingForPush() throws Exception {
     PerformanceLogger testPerformanceLogger = mock(PerformanceLogger.class);
     try (Registration registration =
@@ -397,8 +399,7 @@ public class TraceIT extends AbstractDaemonTest {
   }
 
   @Test
-  @GerritConfig(name = "tracing.performanceLogging", value = "false")
-  public void noPerformanceLoggingIfDisabled() throws Exception {
+  public void noPerformanceLoggingByDefault() throws Exception {
     PerformanceLogger testPerformanceLogger = mock(PerformanceLogger.class);
     try (Registration registration =
         extensionRegistry.newRegistration().add(testPerformanceLogger)) {
diff --git a/javatests/com/google/gerrit/acceptance/ssh/SshTraceIT.java b/javatests/com/google/gerrit/acceptance/ssh/SshTraceIT.java
index ae45d90338..a2d4a06eae 100644
--- a/javatests/com/google/gerrit/acceptance/ssh/SshTraceIT.java
+++ b/javatests/com/google/gerrit/acceptance/ssh/SshTraceIT.java
@@ -23,6 +23,7 @@ import com.google.gerrit.acceptance.AbstractDaemonTest;
 import com.google.gerrit.acceptance.ExtensionRegistry;
 import com.google.gerrit.acceptance.ExtensionRegistry.Registration;
 import com.google.gerrit.acceptance.UseSsh;
+import com.google.gerrit.acceptance.config.GerritConfig;
 import com.google.gerrit.server.logging.LoggingContext;
 import com.google.gerrit.server.logging.Metadata;
 import com.google.gerrit.server.logging.PerformanceLogger;
@@ -94,6 +95,7 @@ public class SshTraceIT extends AbstractDaemonTest {
   }
 
   @Test
+  @GerritConfig(name = "tracing.performanceLogging", value = "true")
   public void performanceLoggingForSshCall() throws Exception {
     TestPerformanceLogger testPerformanceLogger = new TestPerformanceLogger();
     try (Registration registration =
diff --git a/javatests/com/google/gerrit/server/logging/LoggingContextAwareExecutorServiceTest.java b/javatests/com/google/gerrit/server/logging/LoggingContextAwareExecutorServiceTest.java
index 8d019f31aa..1f0da16fba 100644
--- a/javatests/com/google/gerrit/server/logging/LoggingContextAwareExecutorServiceTest.java
+++ b/javatests/com/google/gerrit/server/logging/LoggingContextAwareExecutorServiceTest.java
@@ -56,6 +56,9 @@ public class LoggingContextAwareExecutorServiceTest {
           }
         };
     performanceLoggerRegistrationHandle = performanceLoggers.add("gerrit", testPerformanceLogger);
+
+    // Enable performance logging
+    config.setBoolean("tracing", null, "performanceLogging", true);
   }
 
   @After
diff --git a/javatests/com/google/gerrit/server/logging/PerformanceLogContextTest.java b/javatests/com/google/gerrit/server/logging/PerformanceLogContextTest.java
index fefa066972..981e284492 100644
--- a/javatests/com/google/gerrit/server/logging/PerformanceLogContextTest.java
+++ b/javatests/com/google/gerrit/server/logging/PerformanceLogContextTest.java
@@ -58,6 +58,9 @@ public class PerformanceLogContextTest {
 
     testPerformanceLogger = new TestPerformanceLogger();
     performanceLoggerRegistrationHandle = performanceLoggers.add("gerrit", testPerformanceLogger);
+
+    // Enable performance logging
+    config.setBoolean("tracing", null, "performanceLogging", true);
   }
 
   @After