Merge branch 'stable-2.14'
* stable-2.14:
GerritMonitoringFilter: Format with google-java-format
Format BUILD file with buildifier
Add parameters to plugin configuration
Default to using plugin data dir for new installations
Finalize constants in JavamelodyFilter
Format all java files with google-java-formatter
Group Javamelody HTTP request statistics
Change-Id: I2c3bc53601803abf8e9161a0f397daf70f4aba33
diff --git a/BUILD b/BUILD
index f9a7878..e7686ad 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",
@@ -33,8 +40,21 @@
],
)
+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 = ["@javamelody_lib//jar"],
+ exports = PLUGIN_DEPS + PLUGIN_TEST_DEPS + [
+ ":javamelody__plugin",
+ "@javamelody_lib//jar",
+ ],
)
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 33e9a68..c23044f 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/javamelody/GerritMonitoringFilter.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/javamelody/GerritMonitoringFilter.java
@@ -14,10 +14,20 @@
package com.googlesource.gerrit.plugins.javamelody;
+import com.google.common.base.Strings;
+import com.google.gerrit.extensions.annotations.PluginData;
+import com.google.gerrit.extensions.annotations.PluginName;
import com.google.gerrit.httpd.AllRequestFilter;
+import com.google.gerrit.server.config.PluginConfig;
+import com.google.gerrit.server.config.PluginConfigFactory;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Optional;
+import java.util.StringJoiner;
import javax.servlet.FilterChain;
import javax.servlet.FilterConfig;
import javax.servlet.ServletException;
@@ -26,9 +36,12 @@
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import net.bull.javamelody.MonitoringFilter;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
@Singleton
class GerritMonitoringFilter extends AllRequestFilter {
+ private static final Logger log = LoggerFactory.getLogger(GerritMonitoringFilter.class);
private final JavamelodyFilter monitoring;
private final CapabilityChecker capabilityChecker;
@@ -74,8 +87,94 @@
}
static class JavamelodyFilter extends MonitoringFilter {
+ private static final String JAVAMELODY_PREFIX = "javamelody";
+ private static final String HTTP_TRANSFORM_PATTERN = "http-transform-pattern";
+ private static final String GLOBAL_HTTP_TRANSFORM_PATTERN =
+ String.format("%s.%s", JAVAMELODY_PREFIX, HTTP_TRANSFORM_PATTERN);
+ private static final String STORAGE_DIR = "storage-directory";
+ private static final String GLOBAL_STORAGE_DIR =
+ String.format("%s.%s", JAVAMELODY_PREFIX, STORAGE_DIR);
+
+ static final 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();
+
+ private final PluginConfig cfg;
+ private final Path defaultDataDir;
+
+ @Inject
+ JavamelodyFilter(
+ PluginConfigFactory cfgFactory,
+ @PluginName String pluginName,
+ @PluginData Path defaultDataDir) {
+ this.defaultDataDir = defaultDataDir;
+ this.cfg = cfgFactory.getFromGerritConfig(pluginName);
+ }
+
+ @Override
+ public void init(FilterConfig config) throws ServletException {
+ if (isPropertyInPluginConfig(HTTP_TRANSFORM_PATTERN)
+ || isPropertyUndefined(config, HTTP_TRANSFORM_PATTERN, GLOBAL_HTTP_TRANSFORM_PATTERN)) {
+ System.setProperty(GLOBAL_HTTP_TRANSFORM_PATTERN, getTransformPattern());
+ }
+
+ if (isPropertyInPluginConfig(STORAGE_DIR)
+ || isPropertyUndefined(config, STORAGE_DIR, GLOBAL_STORAGE_DIR)) {
+ System.setProperty(GLOBAL_STORAGE_DIR, getStorageDir());
+ }
+
+ super.init(config);
+ }
+
public String getJavamelodyUrl(HttpServletRequest httpRequest) {
return getMonitoringUrl(httpRequest);
}
+
+ private String getTransformPattern() {
+ return cfg.getString(HTTP_TRANSFORM_PATTERN, GERRIT_GROUPING);
+ }
+
+ private String getStorageDir() {
+ // default to old path for javamelody storage-directory if it exists
+ final Path tmp = Paths.get(System.getProperty("java.io.tmpdir")).resolve(JAVAMELODY_PREFIX);
+ if (Files.isDirectory(tmp)) {
+ log.warn(
+ "Javamelody data exists in 'tmp' [{}]. Configuration (if any) will be ignored.", tmp);
+ return tmp.toString();
+ }
+
+ // plugin config has the highest priority
+ Path storageDir =
+ Optional.ofNullable(cfg.getString(STORAGE_DIR)).map(Paths::get).orElse(defaultDataDir);
+ if (!Files.isDirectory(storageDir)) {
+ try {
+ Files.createDirectories(storageDir);
+ } catch (IOException e) {
+ log.error("Creation of javamelody data dir [{}] failed.", storageDir, e);
+ throw new RuntimeException(e);
+ }
+ }
+ return storageDir.toString();
+ }
+
+ private boolean isPropertyInPluginConfig(String name) {
+ return !Strings.isNullOrEmpty(cfg.getString(name));
+ }
+
+ private boolean isPropertyUndefined(FilterConfig config, String name, String globalName) {
+ return System.getProperty(globalName) == null
+ && config.getServletContext().getInitParameter(globalName) == null
+ && config.getInitParameter(name) == null;
+ }
}
}
diff --git a/src/main/resources/Documentation/build.md b/src/main/resources/Documentation/build.md
index 3247bf9..3ca3dbd 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
+```
+
[IMPORTANT]
Both targets above are required and must be deployed to the right
locations: `javamelody.jar` to `<gerrit_site>/plugins` directory
diff --git a/src/main/resources/Documentation/config.md b/src/main/resources/Documentation/config.md
index 789350d..dd74e42 100644
--- a/src/main/resources/Documentation/config.md
+++ b/src/main/resources/Documentation/config.md
@@ -9,8 +9,44 @@
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.
- By default true.
+: Whether it is allowed to show top menu in Gerrit UI.
+ By default true.
+
+<a id="storage-directory">
+`plugin.@PLUGIN@.storage-directory`
+: The directory in which to store data files. Javamelody, by default,
+ stores data under `/tmp/javamelody` directory but it gets wiped out
+ upon system restart. Therefore for fresh install (or when it was just
+ wiped out after restart) it is defaulted to `GERRIT_SITE/data/@PLUGIN@`.
+ Note that, in order to preserve existing configuration through
+ `-Djavamelody.storage-directory` value from `container.javaOptions`,
+ it has lower priority than `plugin.@PLUGIN@.storage-directory` but higher
+ than default.
+
+<a id="http-transform-pattern">
+`plugin.@PLUGIN@.http-transform-pattern`
+: Grouping pattern for HTTP requests statistics. Without groupping pattern
+ javamelody treats each HTTP requests as distinctive therefore it is not
+ possible to deduct overal site performance and what is more, on busy server,
+ it may lead to
+ [issue with too many open RRD files](https://stackoverflow.com/questions/19147762/javamelody-crashing-the-server-with-thousands-of-rrd-files).
+ If not specified this parameter takes the value that allows javamelody to
+ group all REST and GIT HTTP (incuding LFS) requests over project, account,
+ SHA-1, Long Object Id (LFS), account etc. ids. However one can provide own
+ regexp to cover for instance plugin extensions.
+ Note that, in order to preserve existing configuration through
+ `-Djavamelody.http-transform-pattern` value from `container.javaOptions`,
+ it has lower priority than `plugin.@PLUGIN@.http-transform-pattern` but higher
+ than default.
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",
],
)
