summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorSven Selberg <svense@axis.com>2022-05-10 16:04:25 +0200
committerSven Selberg <svense@axis.com>2022-05-10 16:04:25 +0200
commitcd42803acebed7ad55b2304b7d105d9ebb416cfb (patch)
tree3d34d09bbeb6ef49a80a7c8bcd9d242c7310bdb2
parent40477c2d35b03a3bbca753157dda7a01865d6fc4 (diff)
parent062c24418d7ff6e8c0057d647cb180840bf7185f (diff)
Merge branch 'stable-3.5' into stable-3.6
* stable-3.5: Fix the gr-autocomplete. Consider non-empty performance log-records as non-empty LoggingContext: Consider empty aclLogRecords as emtpy Make tracing.performanceLogging default disabled Fix bug in gr-avatar which wasn't hiding avatars when "hidden" is set Release-Notes: skip Change-Id: I8a696ca92f1698bb61fbf30ddf32897fe78646ff
-rw-r--r--Documentation/config-gerrit.txt12
-rw-r--r--java/com/google/gerrit/server/logging/LoggingContext.java3
-rw-r--r--java/com/google/gerrit/server/logging/MutableAclLogRecords.java4
-rw-r--r--java/com/google/gerrit/server/logging/MutablePerformanceLogRecords.java4
-rw-r--r--java/com/google/gerrit/server/logging/PerformanceLogContext.java2
-rw-r--r--javatests/com/google/gerrit/acceptance/rest/TraceIT.java5
-rw-r--r--javatests/com/google/gerrit/acceptance/ssh/SshTraceIT.java2
-rw-r--r--javatests/com/google/gerrit/server/logging/LoggingContextAwareExecutorServiceTest.java3
-rw-r--r--javatests/com/google/gerrit/server/logging/PerformanceLogContextTest.java3
9 files changed, 28 insertions, 10 deletions
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index 2b20cab1ef..d1f2786681 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -5256,13 +5256,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/LoggingContext.java b/java/com/google/gerrit/server/logging/LoggingContext.java
index 3907da56a1..eac96a676e 100644
--- a/java/com/google/gerrit/server/logging/LoggingContext.java
+++ b/java/com/google/gerrit/server/logging/LoggingContext.java
@@ -90,8 +90,9 @@ public class LoggingContext extends com.google.common.flogger.backend.system.Log
return tags.get() == null
&& forceLogging.get() == null
&& performanceLogging.get() == null
+ && (performanceLogRecords.get() == null || performanceLogRecords.get().isEmtpy())
&& aclLogging.get() == null
- && aclLogRecords.get() == null;
+ && (aclLogRecords.get() == null || aclLogRecords.get().isEmpty());
}
public void clear() {
diff --git a/java/com/google/gerrit/server/logging/MutableAclLogRecords.java b/java/com/google/gerrit/server/logging/MutableAclLogRecords.java
index baa9b1f245..a692d2b178 100644
--- a/java/com/google/gerrit/server/logging/MutableAclLogRecords.java
+++ b/java/com/google/gerrit/server/logging/MutableAclLogRecords.java
@@ -45,6 +45,10 @@ public class MutableAclLogRecords {
return ImmutableList.copyOf(aclLogRecords);
}
+ public boolean isEmpty() {
+ return aclLogRecords.isEmpty();
+ }
+
@Override
public String toString() {
return MoreObjects.toStringHelper(this).add("aclLogRecords", aclLogRecords).toString();
diff --git a/java/com/google/gerrit/server/logging/MutablePerformanceLogRecords.java b/java/com/google/gerrit/server/logging/MutablePerformanceLogRecords.java
index 4ee70d7c19..2965719f15 100644
--- a/java/com/google/gerrit/server/logging/MutablePerformanceLogRecords.java
+++ b/java/com/google/gerrit/server/logging/MutablePerformanceLogRecords.java
@@ -46,6 +46,10 @@ public class MutablePerformanceLogRecords {
return ImmutableList.copyOf(performanceLogRecords);
}
+ public boolean isEmtpy() {
+ return performanceLogRecords.isEmpty();
+ }
+
@Override
public String toString() {
return MoreObjects.toStringHelper(this)
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 ce5cff731d..84c39362c6 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;
@@ -92,6 +93,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 e60d6b4b3b..b1cd8fb3fd 100644
--- a/javatests/com/google/gerrit/server/logging/PerformanceLogContextTest.java
+++ b/javatests/com/google/gerrit/server/logging/PerformanceLogContextTest.java
@@ -56,6 +56,9 @@ public class PerformanceLogContextTest {
testPerformanceLogger = new TestPerformanceLogger();
performanceLoggerRegistrationHandle = performanceLoggers.add("gerrit", testPerformanceLogger);
+
+ // Enable performance logging
+ config.setBoolean("tracing", null, "performanceLogging", true);
}
@After