From 9819b2a6d15e1745fa4e665d59f7f7e321690263 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Georg=20=C3=96ttl?= Date: Wed, 26 Apr 2017 15:28:10 +0200 Subject: [PATCH 1/5] Add Prometheus client lib dependencies --- .classpath | 6 +++++ build.moxie | 5 ++++ gitblit.iml | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 77 insertions(+) diff --git a/.classpath b/.classpath index 42cbfbc87..1ddd14a99 100644 --- a/.classpath +++ b/.classpath @@ -80,6 +80,12 @@ + + + + + + diff --git a/build.moxie b/build.moxie index f21241d1b..d6880b0ba 100644 --- a/build.moxie +++ b/build.moxie @@ -181,6 +181,11 @@ dependencies: - compile 'ro.fortsoft.pf4j:pf4j:0.9.0' :war - compile 'org.apache.tika:tika-core:1.5' :war - compile 'org.jsoup:jsoup:1.7.3' :war +- compile 'io.prometheus:simpleclient:0.0.21' :war +- compile 'io.prometheus:simpleclient_hotspot:0.0.21' :war +- compile 'io.prometheus:simpleclient_log4j:0.0.21' :war +- compile 'io.prometheus:simpleclient_guava:0.0.21' :war +- compile 'io.prometheus:simpleclient_servlet:0.0.21' :war - test 'junit:junit:4.12' # Dependencies for Selenium web page testing - test 'org.seleniumhq.selenium:selenium-java:${selenium.version}' @jar diff --git a/gitblit.iml b/gitblit.iml index 1f4aa2483..b25cffbeb 100644 --- a/gitblit.iml +++ b/gitblit.iml @@ -824,6 +824,72 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + From 103e4e4a8cb2232d185220e7c0653b41b36b514b Mon Sep 17 00:00:00 2001 From: "georg.oettl@gmail.com" Date: Tue, 2 May 2017 12:52:29 +0200 Subject: [PATCH 2/5] Expose prometheus metrics in path /prometheus /metrics was already taken --- src/main/java/com/gitblit/guice/WebModule.java | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/main/java/com/gitblit/guice/WebModule.java b/src/main/java/com/gitblit/guice/WebModule.java index 7c83e455b..7d5f2d452 100644 --- a/src/main/java/com/gitblit/guice/WebModule.java +++ b/src/main/java/com/gitblit/guice/WebModule.java @@ -44,7 +44,10 @@ import com.gitblit.servlet.SyndicationServlet; import com.gitblit.wicket.GitblitWicketFilter; import com.google.common.base.Joiner; +import com.google.inject.Scopes; import com.google.inject.servlet.ServletModule; +import io.prometheus.client.exporter.MetricsServlet; +import io.prometheus.client.hotspot.DefaultExports; /** * Defines all the web servlets & filters. @@ -79,6 +82,11 @@ protected void configureServlets() { serve("/robots.txt").with(RobotsTxtServlet.class); serve("/logo.png").with(LogoServlet.class); + // Prometheus + bind(MetricsServlet.class).in(Scopes.SINGLETON); + serve("/prometheus").with(MetricsServlet.class); + DefaultExports.initialize(); + /* Prevent accidental access to 'resources' such as GitBlit java classes * * In the GO setup the JAR containing the application and the WAR injected From e2203a56e710735487ff131500d56489cd389ca5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Georg=20=C3=96ttl?= Date: Sat, 29 Apr 2017 15:03:14 +0200 Subject: [PATCH 3/5] Add prometheus metrics for log4j log entries Enabled default log4j metrics collector. The number of log4j log entries will be counted. This enable the following measurable metrics (captured after clean startup gitblit): # HELP log4j_appender_total Log4j log statements at various log levels # TYPE log4j_appender_total counter log4j_appender_total{level="debug",} 0.0 log4j_appender_total{level="warn",} 1.0 log4j_appender_total{level="trace",} 0.0 log4j_appender_total{level="error",} 0.0 log4j_appender_total{level="fatal",} 0.0 log4j_appender_total{level="info",} 82.0 --- src/main/java/log4j.properties | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/main/java/log4j.properties b/src/main/java/log4j.properties index 43d31d80b..f315fa084 100644 --- a/src/main/java/log4j.properties +++ b/src/main/java/log4j.properties @@ -17,7 +17,7 @@ # FATAL, ERROR, WARN, INFO, DEBUG # #------------------------------------------------------------------------------ -log4j.rootCategory=INFO, S +log4j.rootCategory=INFO, S, METRICS #log4j.rootLogger=INFO #log4j.logger.org=INFO @@ -67,3 +67,6 @@ log4j.appender.H.File = logs/gitblit.html log4j.appender.H.MaxFileSize = 100KB log4j.appender.H.Append = false log4j.appender.H.layout = org.apache.log4j.HTMLLayout + +log4j.appender.METRICS = io.prometheus.client.log4j.InstrumentedAppender +log4j.appender.METRICS.Append = false From 5b0acb2f1cee2c6b73cfaf7c7a849c852c62bf2d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Georg=20=C3=96ttl?= Date: Sat, 29 Apr 2017 15:22:13 +0200 Subject: [PATCH 4/5] Add first two gitblit specific metrics Added metrics for: 1. gitblit_garbage_collects_total 2. gitblit_ldap_sync_latency_seconds. Add gitblit_ prefix to gitblit specific metrics --- .../com/gitblit/service/GarbageCollectorService.java | 8 +++++++- src/main/java/com/gitblit/service/LdapSyncService.java | 7 +++++++ .../java/com/gitblit/service/PrometheusMetrics.java | 10 ++++++++++ 3 files changed, 24 insertions(+), 1 deletion(-) create mode 100644 src/main/java/com/gitblit/service/PrometheusMetrics.java diff --git a/src/main/java/com/gitblit/service/GarbageCollectorService.java b/src/main/java/com/gitblit/service/GarbageCollectorService.java index b98560fd9..fce5c5112 100644 --- a/src/main/java/com/gitblit/service/GarbageCollectorService.java +++ b/src/main/java/com/gitblit/service/GarbageCollectorService.java @@ -23,6 +23,7 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicBoolean; +import io.prometheus.client.Counter; import org.eclipse.jgit.api.GarbageCollectCommand; import org.eclipse.jgit.api.Git; import org.eclipse.jgit.lib.Repository; @@ -35,6 +36,8 @@ import com.gitblit.models.RepositoryModel; import com.gitblit.utils.FileUtils; +import static com.gitblit.service.PrometheusMetrics.GIT_GARBAGE_COLLECTS_TOTAL; + /** * The Garbage Collector Service handles periodic garbage collection in repositories. * @@ -129,6 +132,9 @@ public void close() { forceClose.set(true); } + private final Counter garbageCollectsTotal = Counter.build() + .name(GIT_GARBAGE_COLLECTS_TOTAL).help(GIT_GARBAGE_COLLECTS_TOTAL).register(); + @Override public void run() { if (!isReady()) { @@ -200,7 +206,7 @@ public void run() { // do the deed gc.call(); - + garbageCollectsTotal.inc(); garbageCollected = true; } } catch (Exception e) { diff --git a/src/main/java/com/gitblit/service/LdapSyncService.java b/src/main/java/com/gitblit/service/LdapSyncService.java index 7ae19aad8..4b1cb04ff 100644 --- a/src/main/java/com/gitblit/service/LdapSyncService.java +++ b/src/main/java/com/gitblit/service/LdapSyncService.java @@ -17,6 +17,7 @@ import java.util.concurrent.atomic.AtomicBoolean; +import io.prometheus.client.Histogram; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -24,6 +25,8 @@ import com.gitblit.Keys; import com.gitblit.auth.LdapAuthProvider; +import static com.gitblit.service.PrometheusMetrics.LDAP_SYNC_LATENCY_SECONDS; + /** * @author Alfred Schmid * @@ -31,6 +34,8 @@ public final class LdapSyncService implements Runnable { private final Logger logger = LoggerFactory.getLogger(LdapSyncService.class); + private final Histogram ldapSyncLatency = Histogram.build().name(LDAP_SYNC_LATENCY_SECONDS). + help(LDAP_SYNC_LATENCY_SECONDS).register(); private final IStoredSettings settings; @@ -51,12 +56,14 @@ public LdapSyncService(IStoredSettings settings, LdapAuthProvider ldapAuthProvid public void run() { logger.info("Starting user and group sync with ldap service"); if (!running.getAndSet(true)) { + Histogram.Timer requestTimer = ldapSyncLatency.startTimer(); try { ldapAuthProvider.sync(); } catch (Exception e) { logger.error("Failed to synchronize with ldap", e); } finally { running.getAndSet(false); + requestTimer.observeDuration(); } } logger.info("Finished user and group sync with ldap service"); diff --git a/src/main/java/com/gitblit/service/PrometheusMetrics.java b/src/main/java/com/gitblit/service/PrometheusMetrics.java new file mode 100644 index 000000000..53c4cbd9e --- /dev/null +++ b/src/main/java/com/gitblit/service/PrometheusMetrics.java @@ -0,0 +1,10 @@ +package com.gitblit.service; + +class PrometheusMetrics { + + /** Number of garbage collects */ + static final String GIT_GARBAGE_COLLECTS_TOTAL = "gitblit_garbage_collects_total"; + + /** Ldap Sync Latency in second */ + static final String LDAP_SYNC_LATENCY_SECONDS = "gitblit_ldap_sync_latency_seconds"; +} From 354920c46964cb3ccdc3182be2dca96f2c6cf968 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Georg=20=C3=96ttl?= Date: Fri, 5 May 2017 11:45:22 +0200 Subject: [PATCH 5/5] Add http metrics http_request_duration_seconds and http_requests_total .) http_request_duration_seconds (histogram) -> Labels (path,method) .) http_requests_total (counter) -> Labels (path,method,status) Where path is the REST path of the ressource, method is the HTTP method (GET,PUT..) and status is the HTTP response code (2xx, 3xx ...) --- .../java/com/gitblit/guice/WebModule.java | 13 +- .../com/gitblit/servlet/MetricsFilter.java | 134 ++++++++++++++++++ .../com/gitblit/tests/MetricsFilterTest.java | 50 +++++++ 3 files changed, 195 insertions(+), 2 deletions(-) create mode 100644 src/main/java/com/gitblit/servlet/MetricsFilter.java create mode 100644 src/test/java/com/gitblit/tests/MetricsFilterTest.java diff --git a/src/main/java/com/gitblit/guice/WebModule.java b/src/main/java/com/gitblit/guice/WebModule.java index 7d5f2d452..af2545806 100644 --- a/src/main/java/com/gitblit/guice/WebModule.java +++ b/src/main/java/com/gitblit/guice/WebModule.java @@ -30,6 +30,7 @@ import com.gitblit.servlet.GitFilter; import com.gitblit.servlet.GitServlet; import com.gitblit.servlet.LogoServlet; +import com.gitblit.servlet.MetricsFilter; import com.gitblit.servlet.PagesFilter; import com.gitblit.servlet.PagesServlet; import com.gitblit.servlet.ProxyFilter; @@ -43,7 +44,9 @@ import com.gitblit.servlet.SyndicationFilter; import com.gitblit.servlet.SyndicationServlet; import com.gitblit.wicket.GitblitWicketFilter; + import com.google.common.base.Joiner; +import com.google.common.collect.ImmutableMap; import com.google.inject.Scopes; import com.google.inject.servlet.ServletModule; import io.prometheus.client.exporter.MetricsServlet; @@ -84,6 +87,7 @@ protected void configureServlets() { // Prometheus bind(MetricsServlet.class).in(Scopes.SINGLETON); + bind(MetricsFilter.class).in(Scopes.SINGLETON); serve("/prometheus").with(MetricsServlet.class); DefaultExports.initialize(); @@ -99,8 +103,13 @@ protected void configureServlets() { serve(fuzzy("/com/")).with(AccessDeniedServlet.class); // global filters - filter(ALL).through(ProxyFilter.class); - filter(ALL).through(EnforceAuthenticationFilter.class); + filter(ALL).through(MetricsFilter.class, + ImmutableMap.of( + MetricsFilter.PARAM_DURATION_HIST_BUCKET_CONFIG, "0.005,0.01,0.025,0.05,0.075,0.1,0.25,0.5,0.75,1,2.5,5,7.5,10", + MetricsFilter.PARAM_PATH_MAX_DEPTH, "16" + )); + filter(ALL).through(ProxyFilter.class); + filter(ALL).through(EnforceAuthenticationFilter.class); // security filters filter(fuzzy(Constants.R_PATH), fuzzy(Constants.GIT_PATH)).through(GitFilter.class); diff --git a/src/main/java/com/gitblit/servlet/MetricsFilter.java b/src/main/java/com/gitblit/servlet/MetricsFilter.java new file mode 100644 index 000000000..1595bf9ed --- /dev/null +++ b/src/main/java/com/gitblit/servlet/MetricsFilter.java @@ -0,0 +1,134 @@ +package com.gitblit.servlet; + +import io.prometheus.client.Counter; +import io.prometheus.client.Histogram; + +import javax.servlet.*; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import java.io.IOException; + +/** + */ +public class MetricsFilter implements Filter { + public static final String PARAM_PATH_MAX_DEPTH = "max-path-depth"; + public static final String PARAM_DURATION_HIST_BUCKET_CONFIG = "request-duration-histogram-buckets"; + + private Histogram httpRequestDuration = null; + private Counter requests = null; + + // Package-level for testing purposes. + int pathDepth = 1; + private double[] buckets = null; + + public MetricsFilter() { + } + + public MetricsFilter( + Integer maxPathDepth, + double[] buckets + ) throws ServletException { + this.buckets = buckets; + if (maxPathDepth != null) { + this.pathDepth = maxPathDepth; + } + this.init(null); + } + + private boolean isEmpty(String s) { + return s == null || s.length() == 0; + } + + @Override + public void init(FilterConfig filterConfig) throws ServletException { + + Histogram.Builder httpRequestDurationBuilder = Histogram.build() + .name("http_request_duration_seconds") + .labelNames("path", "method") + .help("The time taken fulfilling servlet requests"); + + Counter.Builder requestsBuilder = Counter.build() + .name("http_requests_total") + .help("Total requests.") + .labelNames("path", "method", "status"); + + if (filterConfig == null && isEmpty("http_request_duration")) { + throw new ServletException("No configuration object provided, and no metricName passed via constructor"); + } + + if (filterConfig != null) { + + // Allow overriding of the path "depth" to track + if (!isEmpty(filterConfig.getInitParameter(PARAM_PATH_MAX_DEPTH))) { + pathDepth = Integer.valueOf(filterConfig.getInitParameter(PARAM_PATH_MAX_DEPTH)); + } + + // Allow users to override the default bucket configuration + if (!isEmpty(filterConfig.getInitParameter(PARAM_DURATION_HIST_BUCKET_CONFIG))) { + String[] bucketParams = filterConfig.getInitParameter(PARAM_DURATION_HIST_BUCKET_CONFIG).split(","); + buckets = new double[bucketParams.length]; + + for (int i = 0; i < bucketParams.length; i++) { + buckets[i] = Double.parseDouble(bucketParams[i]); + } + } + } + + requests = requestsBuilder.register(); + + if (buckets != null) { + httpRequestDurationBuilder = httpRequestDurationBuilder.buckets(buckets); + } + + httpRequestDuration = httpRequestDurationBuilder.register(); + } + + @Override + public void doFilter(ServletRequest servletRequest, ServletResponse servletResponse, FilterChain filterChain) throws IOException, ServletException { + if (!(servletRequest instanceof HttpServletRequest)) { + filterChain.doFilter(servletRequest, servletResponse); + return; + } + + HttpServletRequest request = (HttpServletRequest) servletRequest; + HttpServletResponse response = (HttpServletResponse) servletResponse; + + String path = request.getRequestURI(); + String normalizedPath = extractPathFrom(path, pathDepth); + + Histogram.Timer timer = httpRequestDuration + .labels(normalizedPath, request.getMethod()) + .startTimer(); + try { + filterChain.doFilter(servletRequest, servletResponse); + requests.labels(normalizedPath, request.getMethod().toUpperCase(), String.valueOf(response.getStatus())).inc(); + } finally { + timer.observeDuration(); + } + } + + public String extractPathFrom(String requestUri, int maxPathDepth) { + if (maxPathDepth < 0 || requestUri == null) { + throw new IllegalArgumentException("Path depth has to >= 0"); + } + + int count = 0; + int pathPosition = -1; + do { + int lastPathPosition = pathPosition; + pathPosition = requestUri.indexOf("/", pathPosition + 1); + if (count > maxPathDepth || pathPosition < 0) { + return requestUri.substring(0, lastPathPosition + 1); + } + count++; + } while (count <= maxPathDepth); + + return requestUri.substring(0, pathPosition + 1); + } + + @Override + public void destroy() { + } + +} + diff --git a/src/test/java/com/gitblit/tests/MetricsFilterTest.java b/src/test/java/com/gitblit/tests/MetricsFilterTest.java new file mode 100644 index 000000000..5b7623294 --- /dev/null +++ b/src/test/java/com/gitblit/tests/MetricsFilterTest.java @@ -0,0 +1,50 @@ +package com.gitblit.tests; + +import com.gitblit.servlet.MetricsFilter; +import org.junit.Test; + +import static org.hamcrest.CoreMatchers.*; +import static org.junit.Assert.*; + + +public class MetricsFilterTest { + + @Test + public void alwaysExtractRootPathForZeroPathLength() { + MetricsFilter metricsFilter = new MetricsFilter(); + String path = metricsFilter.extractPathFrom("/index.html", 0); + assertThat(path, equalTo("/")); + } + + @Test + public void useAlwaysRootPathForLongPathLength() { + MetricsFilter metricsFilter = new MetricsFilter(); + String path = metricsFilter.extractPathFrom("/index.html", 1); + assertThat(path, equalTo("/")); + } + + @Test + public void pathDepthOneuseAlwaysRootPathForZeroPathLength() { + MetricsFilter metricsFilter = new MetricsFilter(); + String path = metricsFilter.extractPathFrom("/test/index.html", 1); + assertThat(path, equalTo("/test/")); + } + + @Test + public void cutsPathsLongerThanPathDepth() { + MetricsFilter metricsFilter = new MetricsFilter(); + String path = metricsFilter.extractPathFrom("/test/tralala/index.html", 1); + assertThat(path, equalTo("/test/")); + } + + @Test(expected = IllegalArgumentException.class) + public void throwsExceptionForNegativePathDepth() { + new MetricsFilter().extractPathFrom("/index.html", -1); + } + + @Test(expected = IllegalArgumentException.class) + public void throwsExceptionForNullRequestPath() { + new MetricsFilter().extractPathFrom(null, 1); + } + +} \ No newline at end of file