From a093f127d4be3864523b39f6c4eb7267081bc613 Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Thu, 12 Nov 2015 17:34:26 -0800 Subject: [PATCH 1/2] Add Histogram0..3 to track bytes sent The pack_bytes metric reports on the size of data sent to clients. Using a histogram allows the server to track the distribution of these files over time, as well as total amount sent. The response_bytes metric reports on size of REST API replies, broken down by view. Change-Id: I7c06cb9afb2f4a8da1ef1eec85b5831f75b88116 --- .../gerrit/httpd/restapi/RestApiMetrics.java | 9 ++ .../gerrit/httpd/restapi/RestApiServlet.java | 74 ++++++------ .../gerrit/metrics/DisabledMetricMaker.java | 35 ++++++ .../com/google/gerrit/metrics/Histogram0.java | 27 +++++ .../com/google/gerrit/metrics/Histogram1.java | 29 +++++ .../com/google/gerrit/metrics/Histogram2.java | 30 +++++ .../com/google/gerrit/metrics/Histogram3.java | 31 +++++ .../google/gerrit/metrics/MetricMaker.java | 12 ++ .../metrics/dropwizard/BucketedHistogram.java | 107 ++++++++++++++++++ .../dropwizard/DropWizardMetricMaker.java | 75 ++++++++++++ .../metrics/dropwizard/HistogramImpl1.java | 52 +++++++++ .../metrics/dropwizard/HistogramImplN.java | 75 ++++++++++++ .../gerrit/metrics/dropwizard/MetricJson.java | 21 ++++ .../server/git/UploadPackMetricsHook.java | 10 ++ .../server/plugins/PluginMetricMaker.java | 39 +++++++ 15 files changed, 594 insertions(+), 32 deletions(-) create mode 100644 gerrit-server/src/main/java/com/google/gerrit/metrics/Histogram0.java create mode 100644 gerrit-server/src/main/java/com/google/gerrit/metrics/Histogram1.java create mode 100644 gerrit-server/src/main/java/com/google/gerrit/metrics/Histogram2.java create mode 100644 gerrit-server/src/main/java/com/google/gerrit/metrics/Histogram3.java create mode 100644 gerrit-server/src/main/java/com/google/gerrit/metrics/dropwizard/BucketedHistogram.java create mode 100644 gerrit-server/src/main/java/com/google/gerrit/metrics/dropwizard/HistogramImpl1.java create mode 100644 gerrit-server/src/main/java/com/google/gerrit/metrics/dropwizard/HistogramImplN.java diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiMetrics.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiMetrics.java index daa587d44f..652acb9b23 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiMetrics.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiMetrics.java @@ -20,6 +20,7 @@ import com.google.gerrit.metrics.Counter1; import com.google.gerrit.metrics.Counter2; import com.google.gerrit.metrics.Description; import com.google.gerrit.metrics.Field; +import com.google.gerrit.metrics.Histogram1; import com.google.gerrit.metrics.MetricMaker; import com.google.gerrit.metrics.Timer1; import com.google.gerrit.metrics.Description.Units; @@ -36,6 +37,7 @@ public class RestApiMetrics { final Counter1 count; final Counter2 errorCount; final Timer1 serverLatency; + final Histogram1 responseBytes; @Inject RestApiMetrics(MetricMaker metrics) { @@ -59,6 +61,13 @@ public class RestApiMetrics { .setCumulative() .setUnit(Units.MILLISECONDS), view); + + responseBytes = metrics.newHistogram( + "http/server/rest_api/response_bytes", + new Description("Size of response on network (may be gzip compressed)") + .setCumulative() + .setUnit(Units.BYTES), + view); } String view(ViewData viewData) { diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java index f2d0e14e79..f2ca49dc9b 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java @@ -43,6 +43,7 @@ import com.google.common.collect.Maps; import com.google.common.collect.Multimap; import com.google.common.collect.Sets; import com.google.common.io.BaseEncoding; +import com.google.common.io.CountingOutputStream; import com.google.common.math.IntMath; import com.google.common.net.HttpHeaders; import com.google.gerrit.audit.AuditService; @@ -206,6 +207,7 @@ public class RestApiServlet extends HttpServlet { res.setHeader("Content-Disposition", "attachment"); res.setHeader("X-Content-Type-Options", "nosniff"); int status = SC_OK; + long responseBytes = -1; Object result = null; Multimap params = LinkedHashMultimap.create(); Object inputRequestBody = null; @@ -353,46 +355,47 @@ public class RestApiServlet extends HttpServlet { if (result != Response.none()) { result = Response.unwrap(result); if (result instanceof BinaryResult) { - replyBinaryResult(req, res, (BinaryResult) result); + responseBytes = replyBinaryResult(req, res, (BinaryResult) result); } else { - replyJson(req, res, config, result); + responseBytes = replyJson(req, res, config, result); } } } catch (MalformedJsonException e) { - replyError(req, res, status = SC_BAD_REQUEST, + responseBytes = replyError(req, res, status = SC_BAD_REQUEST, "Invalid " + JSON_TYPE + " in request", e); } catch (JsonParseException e) { - replyError(req, res, status = SC_BAD_REQUEST, + responseBytes = replyError(req, res, status = SC_BAD_REQUEST, "Invalid " + JSON_TYPE + " in request", e); } catch (BadRequestException e) { - replyError(req, res, status = SC_BAD_REQUEST, messageOr(e, "Bad Request"), - e.caching(), e); + responseBytes = replyError(req, res, status = SC_BAD_REQUEST, + messageOr(e, "Bad Request"), e.caching(), e); } catch (AuthException e) { - replyError(req, res, status = SC_FORBIDDEN, messageOr(e, "Forbidden"), - e.caching(), e); + responseBytes = replyError(req, res, status = SC_FORBIDDEN, + messageOr(e, "Forbidden"), e.caching(), e); } catch (AmbiguousViewException e) { - replyError(req, res, status = SC_NOT_FOUND, messageOr(e, "Ambiguous"), e); + responseBytes = replyError(req, res, status = SC_NOT_FOUND, + messageOr(e, "Ambiguous"), e); } catch (ResourceNotFoundException e) { - replyError(req, res, status = SC_NOT_FOUND, messageOr(e, "Not Found"), - e.caching(), e); + responseBytes = replyError(req, res, status = SC_NOT_FOUND, + messageOr(e, "Not Found"), e.caching(), e); } catch (MethodNotAllowedException e) { - replyError(req, res, status = SC_METHOD_NOT_ALLOWED, + responseBytes = replyError(req, res, status = SC_METHOD_NOT_ALLOWED, messageOr(e, "Method Not Allowed"), e.caching(), e); } catch (ResourceConflictException e) { - replyError(req, res, status = SC_CONFLICT, messageOr(e, "Conflict"), - e.caching(), e); + responseBytes = replyError(req, res, status = SC_CONFLICT, + messageOr(e, "Conflict"), e.caching(), e); } catch (PreconditionFailedException e) { - replyError(req, res, status = SC_PRECONDITION_FAILED, + responseBytes = replyError(req, res, status = SC_PRECONDITION_FAILED, messageOr(e, "Precondition Failed"), e.caching(), e); } catch (UnprocessableEntityException e) { - replyError(req, res, status = 422, messageOr(e, "Unprocessable Entity"), - e.caching(), e); + responseBytes = replyError(req, res, status = 422, + messageOr(e, "Unprocessable Entity"), e.caching(), e); } catch (NotImplementedException e) { - replyError(req, res, status = SC_NOT_IMPLEMENTED, + responseBytes = replyError(req, res, status = SC_NOT_IMPLEMENTED, messageOr(e, "Not Implemented"), e); } catch (Exception e) { status = SC_INTERNAL_SERVER_ERROR; - handleException(e, req, res); + responseBytes = handleException(e, req, res); } finally { String metric = viewData != null && viewData.view != null ? globals.metrics.view(viewData) @@ -401,6 +404,9 @@ public class RestApiServlet extends HttpServlet { if (status >= SC_BAD_REQUEST) { globals.metrics.errorCount.increment(metric, status); } + if (responseBytes != -1) { + globals.metrics.responseBytes.record(metric, responseBytes); + } globals.metrics.serverLatency.record( metric, System.nanoTime() - startNanos, @@ -667,7 +673,7 @@ public class RestApiServlet extends HttpServlet { throw new InstantiationException("Cannot make " + type); } - public static void replyJson(@Nullable HttpServletRequest req, + public static long replyJson(@Nullable HttpServletRequest req, HttpServletResponse res, Multimap config, Object result) @@ -683,7 +689,7 @@ public class RestApiServlet extends HttpServlet { } w.write('\n'); w.flush(); - replyBinaryResult(req, res, asBinaryResult(buf) + return replyBinaryResult(req, res, asBinaryResult(buf) .setContentType(JSON_TYPE) .setCharacterEncoding(UTF_8)); } @@ -752,7 +758,7 @@ public class RestApiServlet extends HttpServlet { } @SuppressWarnings("resource") - static void replyBinaryResult( + static long replyBinaryResult( @Nullable HttpServletRequest req, HttpServletResponse res, BinaryResult bin) throws IOException { @@ -783,10 +789,13 @@ public class RestApiServlet extends HttpServlet { } if (req == null || !"HEAD".equals(req.getMethod())) { - try (OutputStream dst = res.getOutputStream()) { + try (CountingOutputStream dst = + new CountingOutputStream(res.getOutputStream())) { bin.writeTo(dst); + return dst.getCount(); } } + return 0; } finally { appResult.close(); } @@ -993,7 +1002,7 @@ public class RestApiServlet extends HttpServlet { viewData.pluginName, viewData.view.getClass()); } - private static void handleException(Throwable err, HttpServletRequest req, + private static long handleException(Throwable err, HttpServletRequest req, HttpServletResponse res) throws IOException { String uri = req.getRequestURI(); if (!Strings.isNullOrEmpty(req.getQueryString())) { @@ -1003,16 +1012,17 @@ public class RestApiServlet extends HttpServlet { if (!res.isCommitted()) { res.reset(); - replyError(req, res, SC_INTERNAL_SERVER_ERROR, "Internal server error", err); + return replyError(req, res, SC_INTERNAL_SERVER_ERROR, "Internal server error", err); } + return 0; } - public static void replyError(HttpServletRequest req, HttpServletResponse res, + public static long replyError(HttpServletRequest req, HttpServletResponse res, int statusCode, String msg, @Nullable Throwable err) throws IOException { - replyError(req, res, statusCode, msg, CacheControl.NONE, err); + return replyError(req, res, statusCode, msg, CacheControl.NONE, err); } - public static void replyError(HttpServletRequest req, + public static long replyError(HttpServletRequest req, HttpServletResponse res, int statusCode, String msg, CacheControl c, @Nullable Throwable err) throws IOException { if (err != null) { @@ -1020,18 +1030,18 @@ public class RestApiServlet extends HttpServlet { } configureCaching(req, res, null, null, c); res.setStatus(statusCode); - replyText(req, res, msg); + return replyText(req, res, msg); } - static void replyText(@Nullable HttpServletRequest req, + static long replyText(@Nullable HttpServletRequest req, HttpServletResponse res, String text) throws IOException { if ((req == null || isGetOrHead(req)) && isMaybeHTML(text)) { - replyJson(req, res, ImmutableMultimap.of("pp", "0"), new JsonPrimitive(text)); + return replyJson(req, res, ImmutableMultimap.of("pp", "0"), new JsonPrimitive(text)); } else { if (!text.endsWith("\n")) { text += "\n"; } - replyBinaryResult(req, res, + return replyBinaryResult(req, res, BinaryResult.create(text).setContentType("text/plain")); } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/metrics/DisabledMetricMaker.java b/gerrit-server/src/main/java/com/google/gerrit/metrics/DisabledMetricMaker.java index 14b1fff191..1b05e2cc6e 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/metrics/DisabledMetricMaker.java +++ b/gerrit-server/src/main/java/com/google/gerrit/metrics/DisabledMetricMaker.java @@ -91,6 +91,41 @@ public class DisabledMetricMaker extends MetricMaker { }; } + @Override + public Histogram0 newHistogram(String name, Description desc) { + return new Histogram0() { + @Override public void record(long value) {} + @Override public void remove() {} + }; + } + + @Override + public Histogram1 newHistogram(String name, Description desc, + Field field1) { + return new Histogram1() { + @Override public void record(F1 field1, long value) {} + @Override public void remove() {} + }; + } + + @Override + public Histogram2 newHistogram(String name, Description desc, + Field field1, Field field2) { + return new Histogram2() { + @Override public void record(F1 field1, F2 field2, long value) {} + @Override public void remove() {} + }; + } + + @Override + public Histogram3 newHistogram(String name, + Description desc, Field field1, Field field2, Field field3) { + return new Histogram3() { + @Override public void record(F1 field1, F2 field2, F3 field3, long value) {} + @Override public void remove() {} + }; + } + @Override public CallbackMetric0 newCallbackMetric(String name, Class valueClass, Description desc) { diff --git a/gerrit-server/src/main/java/com/google/gerrit/metrics/Histogram0.java b/gerrit-server/src/main/java/com/google/gerrit/metrics/Histogram0.java new file mode 100644 index 0000000000..3a6fbd9b3a --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/metrics/Histogram0.java @@ -0,0 +1,27 @@ +// Copyright (C) 2015 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.google.gerrit.metrics; + +import com.google.gerrit.extensions.registration.RegistrationHandle; + +/** + * Measures the statistical distribution of values in a stream of data. + *

+ * Suitable uses are "response size in bytes", etc. + */ +public abstract class Histogram0 implements RegistrationHandle { + /** Record a sample of a specified amount. */ + public abstract void record(long value); +} diff --git a/gerrit-server/src/main/java/com/google/gerrit/metrics/Histogram1.java b/gerrit-server/src/main/java/com/google/gerrit/metrics/Histogram1.java new file mode 100644 index 0000000000..16df8e4a4e --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/metrics/Histogram1.java @@ -0,0 +1,29 @@ +// Copyright (C) 2015 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.google.gerrit.metrics; + +import com.google.gerrit.extensions.registration.RegistrationHandle; + +/** + * Measures the statistical distribution of values in a stream of data. + *

+ * Suitable uses are "response size in bytes", etc. + * + * @param type of the field. + */ +public abstract class Histogram1 implements RegistrationHandle { + /** Record a sample of a specified amount. */ + public abstract void record(F1 field1, long value); +} diff --git a/gerrit-server/src/main/java/com/google/gerrit/metrics/Histogram2.java b/gerrit-server/src/main/java/com/google/gerrit/metrics/Histogram2.java new file mode 100644 index 0000000000..2eca4027f3 --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/metrics/Histogram2.java @@ -0,0 +1,30 @@ +// Copyright (C) 2015 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.google.gerrit.metrics; + +import com.google.gerrit.extensions.registration.RegistrationHandle; + +/** + * Measures the statistical distribution of values in a stream of data. + *

+ * Suitable uses are "response size in bytes", etc. + * + * @param type of the field. + * @param type of the field. + */ +public abstract class Histogram2 implements RegistrationHandle { + /** Record a sample of a specified amount. */ + public abstract void record(F1 field1, F2 field2, long value); +} diff --git a/gerrit-server/src/main/java/com/google/gerrit/metrics/Histogram3.java b/gerrit-server/src/main/java/com/google/gerrit/metrics/Histogram3.java new file mode 100644 index 0000000000..09a756da23 --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/metrics/Histogram3.java @@ -0,0 +1,31 @@ +// Copyright (C) 2015 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.google.gerrit.metrics; + +import com.google.gerrit.extensions.registration.RegistrationHandle; + +/** + * Measures the statistical distribution of values in a stream of data. + *

+ * Suitable uses are "response size in bytes", etc. + * + * @param type of the field. + * @param type of the field. + * @param type of the field. + */ +public abstract class Histogram3 implements RegistrationHandle { + /** Record a sample of a specified amount. */ + public abstract void record(F1 field1, F2 field2, F3 field3, long value); +} diff --git a/gerrit-server/src/main/java/com/google/gerrit/metrics/MetricMaker.java b/gerrit-server/src/main/java/com/google/gerrit/metrics/MetricMaker.java index 03fc3336a0..21225940e0 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/metrics/MetricMaker.java +++ b/gerrit-server/src/main/java/com/google/gerrit/metrics/MetricMaker.java @@ -46,6 +46,18 @@ public abstract class MetricMaker { String name, Description desc, Field field1, Field field2, Field field3); + /** Metric statistical distribution of values. */ + public abstract Histogram0 newHistogram(String name, Description desc); + public abstract Histogram1 newHistogram( + String name, Description desc, + Field field1); + public abstract Histogram2 newHistogram( + String name, Description desc, + Field field1, Field field2); + public abstract Histogram3 newHistogram( + String name, Description desc, + Field field1, Field field2, Field field3); + /** * Constant value that does not change. * diff --git a/gerrit-server/src/main/java/com/google/gerrit/metrics/dropwizard/BucketedHistogram.java b/gerrit-server/src/main/java/com/google/gerrit/metrics/dropwizard/BucketedHistogram.java new file mode 100644 index 0000000000..51c5feaf59 --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/metrics/dropwizard/BucketedHistogram.java @@ -0,0 +1,107 @@ +// Copyright (C) 2015 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.google.gerrit.metrics.dropwizard; + +import com.google.common.base.Function; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.Maps; +import com.google.gerrit.metrics.Description; +import com.google.gerrit.metrics.Field; +import com.google.gerrit.metrics.dropwizard.DropWizardMetricMaker.HistogramImpl; + +import com.codahale.metrics.Metric; + +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; + +/** Abstract histogram broken down into buckets by {@link Field} values. */ +abstract class BucketedHistogram implements BucketedMetric { + private final DropWizardMetricMaker metrics; + private final String name; + private final Description.FieldOrdering ordering; + protected final Field[] fields; + protected final HistogramImpl total; + private final Map cells; + + BucketedHistogram(DropWizardMetricMaker metrics, String name, + Description desc, Field... fields) { + this.metrics = metrics; + this.name = name; + this.ordering = desc.getFieldOrdering(); + this.fields = fields; + this.total = metrics.newHistogramImpl(name + "_total"); + this.cells = new ConcurrentHashMap<>(); + } + + void doRemove() { + for (HistogramImpl c : cells.values()) { + c.remove(); + } + total.remove(); + metrics.remove(name); + } + + HistogramImpl forceCreate(Object f1, Object f2) { + return forceCreate(ImmutableList.of(f1, f2)); + } + + HistogramImpl forceCreate(Object f1, Object f2, Object f3) { + return forceCreate(ImmutableList.of(f1, f2, f3)); + } + + HistogramImpl forceCreate(Object key) { + HistogramImpl c = cells.get(key); + if (c != null) { + return c; + } + + synchronized (cells) { + c = cells.get(key); + if (c == null) { + c = metrics.newHistogramImpl(submetric(key)); + cells.put(key, c); + } + return c; + } + } + + private String submetric(Object key) { + return DropWizardMetricMaker.name(ordering, name, name(key)); + } + + abstract String name(Object key); + + @Override + public Metric getTotal() { + return total.metric; + } + + @Override + public Field[] getFields() { + return fields; + } + + @Override + public Map getCells() { + return Maps.transformValues( + cells, + new Function () { + @Override + public Metric apply(HistogramImpl in) { + return in.metric; + } + }); + } +} diff --git a/gerrit-server/src/main/java/com/google/gerrit/metrics/dropwizard/DropWizardMetricMaker.java b/gerrit-server/src/main/java/com/google/gerrit/metrics/dropwizard/DropWizardMetricMaker.java index ca0d50a3c1..d7c5f736c5 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/metrics/dropwizard/DropWizardMetricMaker.java +++ b/gerrit-server/src/main/java/com/google/gerrit/metrics/dropwizard/DropWizardMetricMaker.java @@ -32,6 +32,10 @@ import com.google.gerrit.metrics.Counter3; import com.google.gerrit.metrics.Description; import com.google.gerrit.metrics.Description.FieldOrdering; import com.google.gerrit.metrics.Field; +import com.google.gerrit.metrics.Histogram0; +import com.google.gerrit.metrics.Histogram1; +import com.google.gerrit.metrics.Histogram2; +import com.google.gerrit.metrics.Histogram3; import com.google.gerrit.metrics.MetricMaker; import com.google.gerrit.metrics.Timer0; import com.google.gerrit.metrics.Timer1; @@ -226,6 +230,56 @@ public class DropWizardMetricMaker extends MetricMaker { return new TimerImpl(name, registry.timer(name)); } + @Override + public synchronized Histogram0 newHistogram(String name, Description desc) { + checkHistogramDescription(name, desc); + define(name, desc); + return newHistogramImpl(name); + } + + @Override + public synchronized Histogram1 newHistogram(String name, + Description desc, Field field1) { + checkHistogramDescription(name, desc); + HistogramImpl1 m = new HistogramImpl1<>(this, name, desc, field1); + define(name, desc); + bucketed.put(name, m); + return m.histogram1(); + } + + @Override + public synchronized Histogram2 newHistogram(String name, + Description desc, Field field1, Field field2) { + checkHistogramDescription(name, desc); + HistogramImplN m = new HistogramImplN(this, name, desc, field1, field2); + define(name, desc); + bucketed.put(name, m); + return m.histogram2(); + } + + @Override + public synchronized Histogram3 newHistogram( + String name, Description desc, + Field field1, Field field2, Field field3) { + checkHistogramDescription(name, desc); + HistogramImplN m = new HistogramImplN(this, name, desc, field1, field2, field3); + define(name, desc); + bucketed.put(name, m); + return m.histogram3(); + } + + private static void checkHistogramDescription(String name, Description desc) { + checkMetricName(name); + checkArgument(!desc.isConstant(), "histogram must not be constant"); + checkArgument(!desc.isGauge(), "histogram must not be a gauge"); + checkArgument(!desc.isRate(), "histogram must not be a rate"); + checkArgument(desc.isCumulative(), "histogram must be cumulative"); + } + + HistogramImpl newHistogramImpl(String name) { + return new HistogramImpl(name, registry.histogram(name)); + } + @Override public CallbackMetric0 newCallbackMetric( String name, Class valueClass, Description desc) { @@ -339,4 +393,25 @@ public class DropWizardMetricMaker extends MetricMaker { registry.remove(name); } } + + class HistogramImpl extends Histogram0 { + private final String name; + final com.codahale.metrics.Histogram metric; + + private HistogramImpl(String name, com.codahale.metrics.Histogram metric) { + this.name = name; + this.metric = metric; + } + + @Override + public void record(long value) { + metric.update(value); + } + + @Override + public void remove() { + descriptions.remove(name); + registry.remove(name); + } + } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/metrics/dropwizard/HistogramImpl1.java b/gerrit-server/src/main/java/com/google/gerrit/metrics/dropwizard/HistogramImpl1.java new file mode 100644 index 0000000000..e3f9e1cf2f --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/metrics/dropwizard/HistogramImpl1.java @@ -0,0 +1,52 @@ +// Copyright (C) 2015 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.google.gerrit.metrics.dropwizard; + +import com.google.common.base.Function; +import com.google.gerrit.metrics.Description; +import com.google.gerrit.metrics.Field; +import com.google.gerrit.metrics.Histogram1; + +/** Optimized version of {@link BucketedHistogram} for single dimension. */ +class HistogramImpl1 extends BucketedHistogram implements BucketedMetric { + HistogramImpl1(DropWizardMetricMaker metrics, String name, + Description desc, Field field1) { + super(metrics, name, desc, field1); + } + + Histogram1 histogram1() { + return new Histogram1() { + @Override + public void record(F1 field1, long value) { + total.record(value); + forceCreate(field1).record(value); + } + + @Override + public void remove() { + doRemove(); + } + }; + } + + @Override + String name(Object field1) { + @SuppressWarnings("unchecked") + Function fmt = + (Function) fields[0].formatter(); + + return fmt.apply(field1).replace('/', '-'); + } +} diff --git a/gerrit-server/src/main/java/com/google/gerrit/metrics/dropwizard/HistogramImplN.java b/gerrit-server/src/main/java/com/google/gerrit/metrics/dropwizard/HistogramImplN.java new file mode 100644 index 0000000000..d832c6069a --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/metrics/dropwizard/HistogramImplN.java @@ -0,0 +1,75 @@ +// Copyright (C) 2015 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.google.gerrit.metrics.dropwizard; + +import com.google.common.base.Function; +import com.google.common.base.Joiner; +import com.google.common.collect.ImmutableList; +import com.google.gerrit.metrics.Description; +import com.google.gerrit.metrics.Field; +import com.google.gerrit.metrics.Histogram2; +import com.google.gerrit.metrics.Histogram3; + +/** Generalized implementation of N-dimensional Histogram metrics. */ +class HistogramImplN extends BucketedHistogram implements BucketedMetric { + HistogramImplN(DropWizardMetricMaker metrics, String name, + Description desc, Field... fields) { + super(metrics, name, desc, fields); + } + + Histogram2 histogram2() { + return new Histogram2() { + @Override + public void record(F1 field1, F2 field2, long value) { + total.record(value); + forceCreate(field1, field2).record(value); + } + + @Override + public void remove() { + doRemove(); + } + }; + } + + Histogram3 histogram3() { + return new Histogram3() { + @Override + public void record(F1 field1, F2 field2, F3 field3, long value) { + total.record(value); + forceCreate(field1, field2, field3).record(value); + } + + @Override + public void remove() { + doRemove(); + } + }; + } + + @SuppressWarnings("unchecked") + @Override + String name(Object key) { + ImmutableList keyList = (ImmutableList) key; + String[] parts = new String[fields.length]; + for (int i = 0; i < fields.length; i++) { + Function fmt = + (Function) fields[i].formatter(); + + parts[i] = fmt.apply(keyList.get(i)).replace('/', '-'); + } + return Joiner.on('/').join(parts); + } +} diff --git a/gerrit-server/src/main/java/com/google/gerrit/metrics/dropwizard/MetricJson.java b/gerrit-server/src/main/java/com/google/gerrit/metrics/dropwizard/MetricJson.java index 3dfcdfe6ea..b332262312 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/metrics/dropwizard/MetricJson.java +++ b/gerrit-server/src/main/java/com/google/gerrit/metrics/dropwizard/MetricJson.java @@ -22,6 +22,7 @@ import com.google.gerrit.metrics.Field; import com.codahale.metrics.Counter; import com.codahale.metrics.Gauge; +import com.codahale.metrics.Histogram; import com.codahale.metrics.Meter; import com.codahale.metrics.Metric; import com.codahale.metrics.Snapshot; @@ -56,7 +57,9 @@ class MetricJson { Double p99_9; Double min; + Double avg; Double max; + Double sum; Double std_dev; List fields; @@ -123,6 +126,24 @@ class MetricJson { min = s.getMin() / div; max = s.getMax() / div; std_dev = s.getStdDev() / div; + + } else if (metric instanceof Histogram) { + Histogram m = (Histogram) metric; + Snapshot s = m.getSnapshot(); + count = m.getCount(); + + p50 = s.getMedian(); + p75 = s.get75thPercentile(); + p95 = s.get95thPercentile(); + p98 = s.get98thPercentile(); + p99 = s.get99thPercentile(); + p99_9 = s.get999thPercentile(); + + min = (double) s.getMin(); + avg = (double) s.getMean(); + max = (double) s.getMax(); + sum = s.getMean() * m.getCount(); + std_dev = s.getStdDev(); } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/UploadPackMetricsHook.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/UploadPackMetricsHook.java index db0b393b48..fd4e7d3652 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/UploadPackMetricsHook.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/UploadPackMetricsHook.java @@ -20,6 +20,7 @@ import com.google.gerrit.metrics.Counter1; import com.google.gerrit.metrics.Description; import com.google.gerrit.metrics.Description.Units; import com.google.gerrit.metrics.Field; +import com.google.gerrit.metrics.Histogram1; import com.google.gerrit.metrics.MetricMaker; import com.google.gerrit.metrics.Timer1; import com.google.inject.Inject; @@ -39,6 +40,7 @@ public class UploadPackMetricsHook implements PostUploadHook { private final Timer1 counting; private final Timer1 compressing; private final Timer1 writing; + private final Histogram1 packBytes; @Inject UploadPackMetricsHook(MetricMaker metricMaker) { @@ -70,6 +72,13 @@ public class UploadPackMetricsHook implements PostUploadHook { .setCumulative() .setUnit(Units.MILLISECONDS), operation); + + packBytes = metricMaker.newHistogram( + "git/upload-pack/pack_bytes", + new Description("Distribution of sizes of packs sent to clients") + .setCumulative() + .setUnit(Units.BYTES), + operation); } @Override @@ -84,5 +93,6 @@ public class UploadPackMetricsHook implements PostUploadHook { counting.record(op, stats.getTimeCounting(), MILLISECONDS); compressing.record(op, stats.getTimeCompressing(), MILLISECONDS); writing.record(op, stats.getTimeWriting(), MILLISECONDS); + packBytes.record(op, stats.getTotalBytes()); } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginMetricMaker.java b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginMetricMaker.java index 3cbb0f5d38..8bb9b4f24e 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginMetricMaker.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginMetricMaker.java @@ -25,6 +25,10 @@ import com.google.gerrit.metrics.Counter2; import com.google.gerrit.metrics.Counter3; import com.google.gerrit.metrics.Description; import com.google.gerrit.metrics.Field; +import com.google.gerrit.metrics.Histogram0; +import com.google.gerrit.metrics.Histogram1; +import com.google.gerrit.metrics.Histogram2; +import com.google.gerrit.metrics.Histogram3; import com.google.gerrit.metrics.MetricMaker; import com.google.gerrit.metrics.Timer0; import com.google.gerrit.metrics.Timer1; @@ -117,6 +121,41 @@ class PluginMetricMaker extends MetricMaker implements LifecycleListener { return m; } + @Override + public Histogram0 newHistogram(String name, Description desc) { + Histogram0 m = root.newHistogram(prefix + name, desc); + cleanup.add(m); + return m; + } + + @Override + public Histogram1 newHistogram( + String name, Description desc, + Field field1) { + Histogram1 m = root.newHistogram(prefix + name, desc, field1); + cleanup.add(m); + return m; + } + + @Override + public Histogram2 newHistogram( + String name, Description desc, + Field field1, Field field2) { + Histogram2 m = root.newHistogram(prefix + name, desc, field1, field2); + cleanup.add(m); + return m; + } + + @Override + public Histogram3 newHistogram( + String name, Description desc, + Field field1, Field field2, Field field3) { + Histogram3 m = + root.newHistogram(prefix + name, desc, field1, field2, field3); + cleanup.add(m); + return m; + } + @Override public CallbackMetric0 newCallbackMetric( String name, Class valueClass, Description desc) { From 5ae5356717d39fc322571f1f94fd04c2925bd7b9 Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Thu, 12 Nov 2015 18:38:08 -0800 Subject: [PATCH 2/2] Add sql/connection_pool/connections metric Export the number of active (true) and idle (false) connections inside of the connection pool, if the connection pool was enabled. Change-Id: I42f4f24a27883a63730f6fe9e3649b93f59ebfc8 --- .../java/com/google/gerrit/pgm/Daemon.java | 3 +-- .../gerrit/pgm/util/BatchProgramModule.java | 3 --- .../SiteLibraryBasedDataSourceProvider.java | 4 ++- .../google/gerrit/pgm/util/SiteProgram.java | 22 ++++++++++++++- .../server/schema/DataSourceProvider.java | 27 +++++++++++++++++++ .../gerrit/httpd/WebAppInitializer.java | 2 +- 6 files changed, 53 insertions(+), 8 deletions(-) diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java index ebbb7658de..c9da6ddbe2 100644 --- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java +++ b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java @@ -276,7 +276,7 @@ public class Daemon extends SiteProgram { @VisibleForTesting public void start() throws IOException { if (dbInjector == null) { - dbInjector = createDbInjector(MULTI_USER); + dbInjector = createDbInjector(true /* enableMetrics */, MULTI_USER); } cfgInjector = createCfgInjector(); config = cfgInjector.getInstance( @@ -325,7 +325,6 @@ public class Daemon extends SiteProgram { private Injector createSysInjector() { final List modules = new ArrayList<>(); modules.add(SchemaVersionCheck.module()); - modules.add(new DropWizardMetricMaker.ApiModule()); modules.add(new DropWizardMetricMaker.RestModule()); modules.add(new LogFileCompressor.Module()); modules.add(new WorkQueue.Module()); diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/util/BatchProgramModule.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/util/BatchProgramModule.java index ba8c28f931..b6539f1331 100644 --- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/util/BatchProgramModule.java +++ b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/util/BatchProgramModule.java @@ -20,8 +20,6 @@ import com.google.common.cache.Cache; import com.google.gerrit.extensions.config.FactoryModule; import com.google.gerrit.extensions.registration.DynamicMap; import com.google.gerrit.extensions.registration.DynamicSet; -import com.google.gerrit.metrics.DisabledMetricMaker; -import com.google.gerrit.metrics.MetricMaker; import com.google.gerrit.reviewdb.client.AccountGroup; import com.google.gerrit.rules.PrologModule; import com.google.gerrit.server.CurrentUser; @@ -91,7 +89,6 @@ public class BatchProgramModule extends FactoryModule { install(reviewDbModule); install(new DiffExecutorModule()); install(PatchListCacheImpl.module()); - bind(MetricMaker.class).to(DisabledMetricMaker.class); // Plugins are not loaded and we're just running through each change // once, so don't worry about cache removal. diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/util/SiteLibraryBasedDataSourceProvider.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/util/SiteLibraryBasedDataSourceProvider.java index 048c2eed12..7f6313dd15 100644 --- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/util/SiteLibraryBasedDataSourceProvider.java +++ b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/util/SiteLibraryBasedDataSourceProvider.java @@ -15,6 +15,7 @@ package com.google.gerrit.pgm.util; import com.google.gerrit.common.SiteLibraryLoaderUtil; +import com.google.gerrit.metrics.MetricMaker; import com.google.gerrit.server.config.GerritServerConfig; import com.google.gerrit.server.config.SitePaths; import com.google.gerrit.server.schema.DataSourceProvider; @@ -37,9 +38,10 @@ public class SiteLibraryBasedDataSourceProvider extends DataSourceProvider { @Inject SiteLibraryBasedDataSourceProvider(SitePaths site, @GerritServerConfig Config cfg, + MetricMaker metrics, DataSourceProvider.Context ctx, DataSourceType dst) { - super(cfg, ctx, dst); + super(cfg, metrics, ctx, dst); libdir = site.lib_dir; } diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/util/SiteProgram.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/util/SiteProgram.java index a6f1f93d09..bf69d9bcbc 100644 --- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/util/SiteProgram.java +++ b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/util/SiteProgram.java @@ -22,6 +22,9 @@ import com.google.common.collect.Lists; import com.google.gerrit.common.Die; import com.google.gerrit.extensions.events.LifecycleListener; import com.google.gerrit.lifecycle.LifecycleModule; +import com.google.gerrit.metrics.DisabledMetricMaker; +import com.google.gerrit.metrics.MetricMaker; +import com.google.gerrit.metrics.dropwizard.DropWizardMetricMaker; import com.google.gerrit.server.config.GerritServerConfig; import com.google.gerrit.server.config.GerritServerConfigModule; import com.google.gerrit.server.config.SitePath; @@ -93,7 +96,13 @@ public abstract class SiteProgram extends AbstractProgram { } /** @return provides database connectivity and site path. */ - protected Injector createDbInjector(final DataSourceProvider.Context context) { + protected Injector createDbInjector(DataSourceProvider.Context context) { + return createDbInjector(false, context); + } + + /** @return provides database connectivity and site path. */ + protected Injector createDbInjector(final boolean enableMetrics, + final DataSourceProvider.Context context) { final Path sitePath = getSitePath(); final List modules = new ArrayList<>(); @@ -107,6 +116,17 @@ public abstract class SiteProgram extends AbstractProgram { }; modules.add(sitePathModule); + if (enableMetrics) { + modules.add(new DropWizardMetricMaker.ApiModule()); + } else { + modules.add(new AbstractModule() { + @Override + protected void configure() { + bind(MetricMaker.class).to(DisabledMetricMaker.class); + } + }); + } + modules.add(new LifecycleModule() { @Override protected void configure() { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/schema/DataSourceProvider.java b/gerrit-server/src/main/java/com/google/gerrit/server/schema/DataSourceProvider.java index 30ebaa72fe..e0482f9be8 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/schema/DataSourceProvider.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/schema/DataSourceProvider.java @@ -20,6 +20,10 @@ import static java.util.concurrent.TimeUnit.SECONDS; import com.google.common.base.Strings; import com.google.gerrit.extensions.events.LifecycleListener; import com.google.gerrit.extensions.persistence.DataSourceInterceptor; +import com.google.gerrit.metrics.CallbackMetric1; +import com.google.gerrit.metrics.Description; +import com.google.gerrit.metrics.Field; +import com.google.gerrit.metrics.MetricMaker; import com.google.gerrit.server.config.ConfigSection; import com.google.gerrit.server.config.ConfigUtil; import com.google.gerrit.server.config.GerritServerConfig; @@ -46,15 +50,18 @@ public class DataSourceProvider implements Provider, public static final int DEFAULT_POOL_LIMIT = 8; private final Config cfg; + private final MetricMaker metrics; private final Context ctx; private final DataSourceType dst; private DataSource ds; @Inject protected DataSourceProvider(@GerritServerConfig Config cfg, + MetricMaker metrics, Context ctx, DataSourceType dst) { this.cfg = cfg; + this.metrics = metrics; this.ctx = ctx; this.dst = dst; } @@ -126,6 +133,7 @@ public class DataSourceProvider implements Provider, ds.setMaxWait(ConfigUtil.getTimeUnit(cfg, "database", null, "poolmaxwait", MILLISECONDS.convert(30, SECONDS), MILLISECONDS)); ds.setInitialSize(ds.getMinIdle()); + exportPoolMetrics(ds); return intercept(interceptor, ds); } else { @@ -148,6 +156,25 @@ public class DataSourceProvider implements Provider, } } + private void exportPoolMetrics(final BasicDataSource pool) { + final CallbackMetric1 cnt = metrics.newCallbackMetric( + "sql/connection_pool/connections", + Integer.class, + new Description("SQL database connections") + .setGauge() + .setUnit("connections"), + Field.ofBoolean("active")); + metrics.newTrigger(cnt, new Runnable() { + @Override + public void run() { + synchronized (pool) { + cnt.set(true, pool.getNumActive()); + cnt.set(false, pool.getNumIdle()); + } + } + }); + } + private DataSource intercept(String interceptor, DataSource ds) { if (interceptor == null) { return ds; diff --git a/gerrit-war/src/main/java/com/google/gerrit/httpd/WebAppInitializer.java b/gerrit-war/src/main/java/com/google/gerrit/httpd/WebAppInitializer.java index 10e3f9c482..2ce42184cc 100644 --- a/gerrit-war/src/main/java/com/google/gerrit/httpd/WebAppInitializer.java +++ b/gerrit-war/src/main/java/com/google/gerrit/httpd/WebAppInitializer.java @@ -261,6 +261,7 @@ public class WebAppInitializer extends GuiceServletContextListener }); } modules.add(new DatabaseModule()); + modules.add(new DropWizardMetricMaker.ApiModule()); return Guice.createInjector(PRODUCTION, modules); } @@ -289,7 +290,6 @@ public class WebAppInitializer extends GuiceServletContextListener private Injector createSysInjector() { final List modules = new ArrayList<>(); - modules.add(new DropWizardMetricMaker.ApiModule()); modules.add(new DropWizardMetricMaker.RestModule()); modules.add(new WorkQueue.Module()); modules.add(new ChangeHookRunner.Module());