Group Javamelody HTTP request statistics
Currently Javamelody treats every request as specific therefore it
generates statistics only when one requests the same operation again
e.g. opens the same review. On busy servers it may additionally lead to
the issue described in [1].
If no grouping is configured with -Djavamelody.http-transform-pattern
parameter to container.javaOptions of gerrit.config then the default
pattern is applied and ids like SHA-1, change-id, project, filename,
etc. are replaced with '$' and grouped by Javamelody in HTTP statistics
'Details' view.
[1] https://stackoverflow.com/questions/19147762/javamelody-crashing-the-server-with-thousands-of-rrd-files
Change-Id: Ie7edf0e7a1482713dc6fd01db8c53bc589c6290f
Signed-off-by: Jacek Centkowski <jcentkowski@collab.net>
diff --git a/BUILD b/BUILD
index 0316bc3..a4280eb 100644
--- a/BUILD
+++ b/BUILD
@@ -1,4 +1,11 @@
-load("//tools/bzl:plugin.bzl", "gerrit_plugin", "PLUGIN_DEPS_NEVERLINK")
+load("//tools/bzl:junit.bzl", "junit_tests")
+load(
+ "//tools/bzl:plugin.bzl",
+ "gerrit_plugin",
+ "PLUGIN_DEPS",
+ "PLUGIN_DEPS_NEVERLINK",
+ "PLUGIN_TEST_DEPS",
+)
gerrit_plugin(
name = "javamelody",
@@ -32,3 +39,22 @@
"@jrobin_lib//jar",
],
)
+
+junit_tests(
+ name = "javamelody_tests",
+ srcs = glob(["src/test/java/**/*.java"]),
+ tags = ["javamelody"],
+ deps = [
+ ":javamelody__plugin_test_deps",
+ ],
+)
+
+java_library(
+ name = "javamelody__plugin_test_deps",
+ testonly = 1,
+ visibility = ["//visibility:public"],
+ exports = PLUGIN_DEPS + PLUGIN_TEST_DEPS + [
+ ":javamelody__plugin",
+ "@javamelody_lib//jar",
+ ],
+)
\ No newline at end of file
diff --git a/src/main/java/com/googlesource/gerrit/plugins/javamelody/GerritMonitoringFilter.java b/src/main/java/com/googlesource/gerrit/plugins/javamelody/GerritMonitoringFilter.java
index 2919502..39ef068 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/javamelody/GerritMonitoringFilter.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/javamelody/GerritMonitoringFilter.java
@@ -21,6 +21,7 @@
import net.bull.javamelody.MonitoringFilter;
import java.io.IOException;
+import java.util.StringJoiner;
import javax.servlet.FilterChain;
import javax.servlet.FilterConfig;
@@ -81,8 +82,37 @@
}
static class JavamelodyFilter extends MonitoringFilter {
+ private static String HTTP_TRANSFORM_PATTERN = "http-transform-pattern";
+ private static String GLOBAL_HTTP_TRANSFORM_PATTERN = "javamelody." + HTTP_TRANSFORM_PATTERN;
+ static String GERRIT_GROUPING = new StringJoiner("|")
+ .add("[0-9a-f]{64}") // Long SHA for LFS
+ .add("[0-9a-f]{40}") // SHA-1
+ .add("[0-9A-F]{32}") // GWT cache ID
+ .add("(?<=files/)[^/]+") //review files part
+ .add("(?<=/projects/)[^/]+") //project name
+ .add("(?<=/accounts/)[^/]+") // account id
+ .add("(.+)(?=/git-upload-pack)") // Git fetch/clone
+ .add("(.+)(?=/git-receive-pack)") // Git push
+ .add("(.+)(?=/info/)") // Git and LFS operations
+ .add("\\d+") // various ids e.g. change id
+ .toString();
+
+ @Override
+ public void init(FilterConfig config) throws ServletException {
+ if (isHttpTransformPatternrUndefined(config)) {
+ System.setProperty(GLOBAL_HTTP_TRANSFORM_PATTERN, GERRIT_GROUPING);
+ }
+ super.init(config);
+ }
+
public String getJavamelodyUrl(HttpServletRequest httpRequest) {
return getMonitoringUrl(httpRequest);
}
+
+ private boolean isHttpTransformPatternrUndefined(FilterConfig config) {
+ return System.getProperty(GLOBAL_HTTP_TRANSFORM_PATTERN) == null
+ && config.getServletContext().getInitParameter(GLOBAL_HTTP_TRANSFORM_PATTERN) == null
+ && config.getInitParameter(HTTP_TRANSFORM_PATTERN) == null;
+ }
}
}
diff --git a/src/main/resources/Documentation/build.md b/src/main/resources/Documentation/build.md
index b78dc63..33d9e99 100644
--- a/src/main/resources/Documentation/build.md
+++ b/src/main/resources/Documentation/build.md
@@ -59,6 +59,12 @@
bazel-bin/lib@PLUGIN@__plugin-src.jar
```
+To execute the tests run:
+
+```
+ bazel test @PLUGIN@_tests
+```
+
This project can be imported into the Eclipse IDE:
```
@@ -102,6 +108,12 @@
bazel-bin/plugins/javamelody/javamelody-deps_deploy.jar
```
+To execute the tests run:
+
+```
+ bazel test plugins/@PLUGIN@:@PLUGIN@_tests
+```
+
This project can be imported into the Eclipse IDE.
Add the plugin name to the `CUSTOM_PLUGINS` set in
Gerrit core in `tools/bzl/plugins.bzl`, and execute:
diff --git a/src/main/resources/Documentation/config.md b/src/main/resources/Documentation/config.md
index 789350d..b42bc36 100644
--- a/src/main/resources/Documentation/config.md
+++ b/src/main/resources/Documentation/config.md
@@ -9,6 +9,15 @@
allowTopMenu = false
```
+Note that [JavaMelody Optional Parameters](https://github.com/javamelody/javamelody/wiki/UserGuide#6-optional-parameters)
+can be provided to `gerrit.config` as part of `container.javaOptions`
+parameter e.g.:
+
+```
+ [container]
+ javaOptions = -Djavamelody.log=true
+```
+
<a id="allowTopMenu">
`plugin.@PLUGIN@.allowTopMenu`
: Whether it is allowed to show top menu in Gerrit UI.
diff --git a/src/test/java/com/googlesource/gerrit/plugins/javamelody/GerritMonitoringFilterTest.java b/src/test/java/com/googlesource/gerrit/plugins/javamelody/GerritMonitoringFilterTest.java
new file mode 100644
index 0000000..1686414
--- /dev/null
+++ b/src/test/java/com/googlesource/gerrit/plugins/javamelody/GerritMonitoringFilterTest.java
@@ -0,0 +1,92 @@
+// Copyright (C) 2017 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.googlesource.gerrit.plugins.javamelody;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import java.util.regex.Pattern;
+import org.junit.Test;
+
+public class GerritMonitoringFilterTest {
+ private static final Pattern GROUPING_PATTERN =
+ Pattern.compile(
+ GerritMonitoringFilter.JavamelodyFilter.GERRIT_GROUPING,
+ // The following flags were added to have as close to javamelody env as possible.
+ // In [1] javamelody it adds them to pattern before compiling.
+ // [1]
+ // https://github.com/javamelody/javamelody/blob/3863b31a86c0df04f4063615f72fc6117aecf909/javamelody-core/src/main/java/net/bull/javamelody/FilterContext.java#L209
+ Pattern.MULTILINE | Pattern.DOTALL);
+
+ @Test
+ public void testGroupingPattern() throws Exception {
+ String dollar = "\\$";
+ String result =
+ GROUPING_PATTERN
+ .matcher(
+ "/plugins/lfs/content/default/a55dc67374da05f4e1eb736f8ad2147d0a6964ed41d28462fd7e2fe86bea78ed")
+ .replaceAll(dollar);
+ assertThat(result).named("Long SHA (LFS) grouping").isEqualTo("/plugins/lfs/content/default/$");
+
+ result =
+ GROUPING_PATTERN
+ .matcher("/changes/?q=05810c7a315ba6c52c150c237d84d05b2ebc3086&n=")
+ .replaceAll(dollar);
+ assertThat(result).named("SHA-1 grouping").isEqualTo("/changes/?q=$&n=");
+
+ result =
+ GROUPING_PATTERN
+ .matcher("/gerrit_ui/gwt/chrome/7CF1DE6EF2AABFEFAE4D469A16D60071.cache.css")
+ .replaceAll(dollar);
+ assertThat(result).named("GWT cache grouping").isEqualTo("/gerrit_ui/gwt/chrome/$.cache.css");
+
+ result = GROUPING_PATTERN.matcher("/files/test_dir%2Funder_dir.file/diff").replaceAll(dollar);
+ assertThat(result).named("Grouping by file").isEqualTo("/files/$/diff");
+
+ result = GROUPING_PATTERN.matcher("/projects/plugins%2Fjavamelody/config").replaceAll(dollar);
+ assertThat(result).named("Grouping by projects").isEqualTo("/projects/$/config");
+
+ result = GROUPING_PATTERN.matcher("/accounts/self/avatar.change.url").replaceAll(dollar);
+ assertThat(result).named("Grouping by account").isEqualTo("/accounts/$/avatar.change.url");
+
+ result = GROUPING_PATTERN.matcher("/test_repo/git-upload-pack").replaceAll(dollar);
+ assertThat(result).named("Grouping git-upload-pack").isEqualTo("$/git-upload-pack");
+
+ result = GROUPING_PATTERN.matcher("/test_repo/git-receive-pack").replaceAll(dollar);
+ assertThat(result).named("Grouping git-receive-pack").isEqualTo("$/git-receive-pack");
+
+ result = GROUPING_PATTERN.matcher("/test_repo.git/info/lfs/locks").replaceAll(dollar);
+ assertThat(result).named("Grouping Git and LFS operations").isEqualTo("$/info/lfs/locks");
+
+ result = GROUPING_PATTERN.matcher("/changes/30/revisions/1/commit").replaceAll(dollar);
+ assertThat(result).named("Grouping numbers").isEqualTo("/changes/$/revisions/$/commit");
+
+ // grouping multiple patterns in one input
+ result =
+ GROUPING_PATTERN
+ .matcher(
+ "/changes/30/revisions/05810c7a315ba6c52c150c237d84d05b2ebc3086/files/test_dir%2Funder_dir.file/diff")
+ .replaceAll(dollar);
+ assertThat(result)
+ .named("Grouping by number, SHA-1 and file")
+ .isEqualTo("/changes/$/revisions/$/files/$/diff");
+
+ result =
+ GROUPING_PATTERN
+ .matcher(
+ "/test_repo.git/info/lfs/locks/8020e4344e49a6928e03b6db69ead86624ffb4aeb1477a7d741a5e2fee9544cd/unlock")
+ .replaceAll(dollar);
+ assertThat(result).named("Grouping LFS with Long SHA").isEqualTo("$/info/lfs/locks/$/unlock");
+ }
+}
diff --git a/tools/bzl/junit.bzl b/tools/bzl/junit.bzl
new file mode 100644
index 0000000..3af7e58
--- /dev/null
+++ b/tools/bzl/junit.bzl
@@ -0,0 +1,4 @@
+load(
+ "@com_googlesource_gerrit_bazlets//tools:junit.bzl",
+ "junit_tests",
+)
diff --git a/tools/bzl/plugin.bzl b/tools/bzl/plugin.bzl
index 2a4dffb..3e8b08f 100644
--- a/tools/bzl/plugin.bzl
+++ b/tools/bzl/plugin.bzl
@@ -3,4 +3,5 @@
"gerrit_plugin",
"PLUGIN_DEPS",
"PLUGIN_DEPS_NEVERLINK",
+ "PLUGIN_TEST_DEPS",
)
diff --git a/tools/eclipse/BUILD b/tools/eclipse/BUILD
index 3122e9b..49f0b9c 100644
--- a/tools/eclipse/BUILD
+++ b/tools/eclipse/BUILD
@@ -1,10 +1,10 @@
load("//tools/bzl:classpath.bzl", "classpath_collector")
-load("//tools/bzl:plugin.bzl", "PLUGIN_DEPS")
+load("//tools/bzl:plugin.bzl", "PLUGIN_DEPS", "PLUGIN_TEST_DEPS")
classpath_collector(
name = "main_classpath_collect",
testonly = 1,
- deps = PLUGIN_DEPS + [
+ deps = PLUGIN_DEPS + PLUGIN_TEST_DEPS + [
"//:javamelody-datasource-interceptor-lib",
],
)
