diff options
author | Bjørn Christian Seime <bjorncs@verizonmedia.com> | 2019-04-04 17:20:01 +0200 |
---|---|---|
committer | Bjørn Christian Seime <bjorncs@verizonmedia.com> | 2019-04-04 17:20:01 +0200 |
commit | 677f00f54c5d624eee9900ecc0bc9ac8085135a2 (patch) | |
tree | 06507486db70d2eb338eec49cacfd8dac7d0a10d /jdisc_http_service | |
parent | 8d32689ff1b06c554c245231840e5d2b3695e092 (diff) |
Add scheme as dimension to response metrics
Diffstat (limited to 'jdisc_http_service')
3 files changed, 112 insertions, 65 deletions
diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpResponseStatisticsCollector.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpResponseStatisticsCollector.java index 1e92fbef967..4239d2120cf 100644 --- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpResponseStatisticsCollector.java +++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpResponseStatisticsCollector.java @@ -16,10 +16,9 @@ import javax.servlet.AsyncListener; import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; - import java.io.IOException; -import java.util.HashMap; -import java.util.Map; +import java.util.ArrayList; +import java.util.List; import java.util.concurrent.Future; import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicLong; @@ -40,19 +39,25 @@ public class HttpResponseStatisticsCollector extends HandlerWrapper implements G GET, PATCH, POST, PUT, DELETE, OPTIONS, HEAD, OTHER } + public enum HttpScheme { + HTTP, HTTPS, OTHER + } + private static final String[] HTTP_RESPONSE_GROUPS = { Metrics.RESPONSES_1XX, Metrics.RESPONSES_2XX, Metrics.RESPONSES_3XX, Metrics.RESPONSES_4XX, Metrics.RESPONSES_5XX, Metrics.RESPONSES_401, Metrics.RESPONSES_403}; private final AtomicLong inFlight = new AtomicLong(); - private final LongAdder statistics[][]; + private final LongAdder statistics[][][]; public HttpResponseStatisticsCollector() { super(); - statistics = new LongAdder[HttpMethod.values().length][]; - for (int method = 0; method < statistics.length; method++) { - statistics[method] = new LongAdder[HTTP_RESPONSE_GROUPS.length]; - for (int group = 0; group < HTTP_RESPONSE_GROUPS.length; group++) { - statistics[method][group] = new LongAdder(); + statistics = new LongAdder[HttpScheme.values().length][HttpMethod.values().length][]; + for (int scheme = 0; scheme < HttpScheme.values().length; ++scheme) { + for (int method = 0; method < HttpMethod.values().length; method++) { + statistics[scheme][method] = new LongAdder[HTTP_RESPONSE_GROUPS.length]; + for (int group = 0; group < HTTP_RESPONSE_GROUPS.length; group++) { + statistics[scheme][method][group] = new LongAdder(); + } } } } @@ -110,10 +115,11 @@ public class HttpResponseStatisticsCollector extends HandlerWrapper implements G private void observeEndOfRequest(Request request, HttpServletResponse flushableResponse) throws IOException { int group = groupIndex(request); if (group >= 0) { + HttpScheme scheme = getScheme(request); HttpMethod method = getMethod(request); - statistics[method.ordinal()][group].increment(); + statistics[scheme.ordinal()][method.ordinal()][group].increment(); if (group == 5 || group == 6) { // if 401/403, also increment 4xx - statistics[method.ordinal()][3].increment(); + statistics[scheme.ordinal()][method.ordinal()][3].increment(); } } @@ -146,6 +152,17 @@ public class HttpResponseStatisticsCollector extends HandlerWrapper implements G } } + private HttpScheme getScheme(Request request) { + switch (request.getScheme()) { + case "http": + return HttpScheme.HTTP; + case "https": + return HttpScheme.HTTPS; + default: + return HttpScheme.OTHER; + } + } + private HttpMethod getMethod(Request request) { switch (request.getMethod()) { case "GET": @@ -167,17 +184,18 @@ public class HttpResponseStatisticsCollector extends HandlerWrapper implements G } } - public Map<String, Map<String, Long>> takeStatisticsByMethod() { - Map<String, Map<String, Long>> ret = new HashMap<>(); - - for (HttpMethod method : HttpMethod.values()) { - int methodIndex = method.ordinal(); - Map<String, Long> methodStats = new HashMap<>(); - ret.put(method.toString(), methodStats); - - for (int group = 0; group < HTTP_RESPONSE_GROUPS.length; group++) { - long value = statistics[methodIndex][group].sumThenReset(); - methodStats.put(HTTP_RESPONSE_GROUPS[group], value); + public List<StatisticsEntry> takeStatistics() { + var ret = new ArrayList<StatisticsEntry>(); + for (HttpScheme scheme : HttpScheme.values()) { + int schemeIndex = scheme.ordinal(); + for (HttpMethod method : HttpMethod.values()) { + int methodIndex = method.ordinal(); + for (int group = 0; group < HTTP_RESPONSE_GROUPS.length; group++) { + long value = statistics[schemeIndex][methodIndex][group].sumThenReset(); + if (value > 0) { + ret.add(new StatisticsEntry(scheme.name().toLowerCase(), method.name(), HTTP_RESPONSE_GROUPS[group], value)); + } + } } } return ret; @@ -216,4 +234,19 @@ public class HttpResponseStatisticsCollector extends HandlerWrapper implements G FutureCallback futureCallback = shutdown.get(); return futureCallback != null && futureCallback.isDone(); } + + public static class StatisticsEntry { + public final String scheme; + public final String method; + public final String name; + public final long value; + + + public StatisticsEntry(String scheme, String method, String name, long value) { + this.scheme = scheme; + this.method = method; + this.name = name; + this.value = value; + } + } } diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java index 49ae54f7db3..30a1b1d885c 100644 --- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java +++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java @@ -8,7 +8,6 @@ import com.yahoo.component.ComponentId; import com.yahoo.component.provider.ComponentRegistry; import com.yahoo.container.logging.AccessLog; import com.yahoo.jdisc.Metric; -import com.yahoo.jdisc.Metric.Context; import com.yahoo.jdisc.application.OsgiFramework; import com.yahoo.jdisc.http.ServerConfig; import com.yahoo.jdisc.http.ServletPathsConfig; @@ -358,13 +357,12 @@ public class JettyHttpServer extends AbstractServerProvider { } private void addResponseMetrics(HttpResponseStatisticsCollector statisticsCollector) { - Map<String, Map<String, Long>> statistics = statisticsCollector.takeStatisticsByMethod(); - statistics.forEach((httpMethod, statsByResponseType) -> { + for (var metricEntry : statisticsCollector.takeStatistics()) { Map<String, Object> dimensions = new HashMap<>(); - dimensions.put(Metrics.METHOD_DIMENSION, httpMethod); - Context ctx = metric.createContext(dimensions); - statsByResponseType.forEach((group, value) -> metric.add(group, value, ctx)); - }); + dimensions.put(Metrics.METHOD_DIMENSION, metricEntry.method); + dimensions.put(Metrics.SCHEME_DIMENSION, metricEntry.scheme); + metric.add(metricEntry.name, metricEntry.value, metric.createContext(dimensions)); + } } private void setConnectorMetrics(JDiscServerConnector connector) { diff --git a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpResponseStatisticsCollectorTest.java b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpResponseStatisticsCollectorTest.java index 3c23a2b0937..df2308f6dd0 100644 --- a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpResponseStatisticsCollectorTest.java +++ b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpResponseStatisticsCollectorTest.java @@ -1,6 +1,7 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.jdisc.http.server.jetty; +import com.yahoo.jdisc.http.server.jetty.HttpResponseStatisticsCollector.StatisticsEntry; import com.yahoo.jdisc.http.server.jetty.JettyHttpServer.Metrics; import org.eclipse.jetty.http.HttpFields; import org.eclipse.jetty.http.HttpURI; @@ -22,10 +23,9 @@ import org.testng.annotations.Test; import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; - import java.io.IOException; import java.nio.ByteBuffer; -import java.util.Map; +import java.util.List; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.equalTo; @@ -40,55 +40,62 @@ public class HttpResponseStatisticsCollectorTest { @Test public void statistics_are_aggregated_by_category() throws Exception { - testRequest(300, "GET"); - testRequest(301, "GET"); - testRequest(200, "GET"); + testRequest("http", 300, "GET"); + testRequest("http", 301, "GET"); + testRequest("http", 200, "GET"); - Map<String, Map<String, Long>> stats = collector.takeStatisticsByMethod(); - assertThat(stats.get("GET").get(Metrics.RESPONSES_2XX), equalTo(1L)); - assertThat(stats.get("GET").get(Metrics.RESPONSES_3XX), equalTo(2L)); + var stats = collector.takeStatistics(); + assertStatisticsEntryPresent(stats, "http", "GET", Metrics.RESPONSES_2XX, 1L); + assertStatisticsEntryPresent(stats, "http", "GET", Metrics.RESPONSES_3XX, 2L); } @Test - public void statistics_are_grouped_by_http_method() throws Exception { - testRequest(200, "GET"); - testRequest(200, "PUT"); - testRequest(200, "POST"); - testRequest(200, "POST"); - testRequest(404, "GET"); - - Map<String, Map<String, Long>> stats = collector.takeStatisticsByMethod(); - assertThat(stats.get("GET").get(Metrics.RESPONSES_2XX), equalTo(1L)); - assertThat(stats.get("GET").get(Metrics.RESPONSES_4XX), equalTo(1L)); - assertThat(stats.get("PUT").get(Metrics.RESPONSES_2XX), equalTo(1L)); - assertThat(stats.get("POST").get(Metrics.RESPONSES_2XX), equalTo(2L)); + public void statistics_are_grouped_by_http_method_and_scheme() throws Exception { + testRequest("http", 200, "GET"); + testRequest("http", 200, "PUT"); + testRequest("http", 200, "POST"); + testRequest("http", 200, "POST"); + testRequest("http", 404, "GET"); + testRequest("https", 404, "GET"); + testRequest("https", 200, "POST"); + testRequest("https", 200, "POST"); + testRequest("https", 200, "POST"); + testRequest("https", 200, "POST"); + + var stats = collector.takeStatistics(); + assertStatisticsEntryPresent(stats, "http", "GET", Metrics.RESPONSES_2XX, 1L); + assertStatisticsEntryPresent(stats, "http", "GET", Metrics.RESPONSES_4XX, 1L); + assertStatisticsEntryPresent(stats, "http", "PUT", Metrics.RESPONSES_2XX, 1L); + assertStatisticsEntryPresent(stats, "http", "POST", Metrics.RESPONSES_2XX, 2L); + assertStatisticsEntryPresent(stats, "https", "GET", Metrics.RESPONSES_4XX, 1L); + assertStatisticsEntryPresent(stats, "https", "POST", Metrics.RESPONSES_2XX, 4L); } @Test public void statistics_include_grouped_and_single_statuscodes() throws Exception { - testRequest(401, "GET"); - testRequest(404, "GET"); - testRequest(403, "GET"); + testRequest("http", 401, "GET"); + testRequest("http", 404, "GET"); + testRequest("http", 403, "GET"); - Map<String, Map<String, Long>> stats = collector.takeStatisticsByMethod(); - assertThat(stats.get("GET").get(Metrics.RESPONSES_4XX), equalTo(3L)); - assertThat(stats.get("GET").get(Metrics.RESPONSES_401), equalTo(1L)); - assertThat(stats.get("GET").get(Metrics.RESPONSES_403), equalTo(1L)); + var stats = collector.takeStatistics(); + assertStatisticsEntryPresent(stats, "http", "GET", Metrics.RESPONSES_4XX, 3L); + assertStatisticsEntryPresent(stats, "http", "GET", Metrics.RESPONSES_401, 1L); + assertStatisticsEntryPresent(stats, "http", "GET", Metrics.RESPONSES_403, 1L); } @Test public void retrieving_statistics_resets_the_counters() throws Exception { - testRequest(200, "GET"); - testRequest(200, "GET"); + testRequest("http", 200, "GET"); + testRequest("http", 200, "GET"); - Map<String, Map<String, Long>> stats = collector.takeStatisticsByMethod(); - assertThat(stats.get("GET").get(Metrics.RESPONSES_2XX), equalTo(2L)); + var stats = collector.takeStatistics(); + assertStatisticsEntryPresent(stats, "http", "GET", Metrics.RESPONSES_2XX, 2L); - testRequest(200, "GET"); + testRequest("http", 200, "GET"); - stats = collector.takeStatisticsByMethod(); - assertThat(stats.get("GET").get(Metrics.RESPONSES_2XX), equalTo(1L)); + stats = collector.takeStatistics(); + assertStatisticsEntryPresent(stats, "http", "GET", Metrics.RESPONSES_2XX, 1L); } @BeforeTest @@ -116,9 +123,9 @@ public class HttpResponseStatisticsCollectorTest { server.start(); } - private Request testRequest(int responseCode, String httpMethod) throws Exception { + private Request testRequest(String scheme, int responseCode, String httpMethod) throws Exception { HttpChannel channel = new HttpChannel(connector, new HttpConfiguration(), null, new DummyTransport()); - MetaData.Request metaData = new MetaData.Request(httpMethod, new HttpURI("http://foo/bar"), HttpVersion.HTTP_1_1, new HttpFields()); + MetaData.Request metaData = new MetaData.Request(httpMethod, new HttpURI(scheme + "://foo/bar"), HttpVersion.HTTP_1_1, new HttpFields()); Request req = channel.getRequest(); req.setMetaData(metaData); @@ -127,6 +134,15 @@ public class HttpResponseStatisticsCollectorTest { return req; } + private static void assertStatisticsEntryPresent(List<StatisticsEntry> result, String scheme, String method, String name, long expectedValue) { + long value = result.stream() + .filter(entry -> entry.method.equals(method) && entry.scheme.equals(scheme) && entry.name.equals(name)) + .mapToLong(entry -> entry.value) + .findAny() + .orElseThrow(() -> new AssertionError(String.format("Not matching entry in result (scheme=%s, method=%s, name=%s)", scheme, method, name))); + assertThat(value, equalTo(expectedValue)); + } + private final class DummyTransport implements HttpTransport { @Override public void send(Response info, boolean head, ByteBuffer content, boolean lastContent, Callback callback) { |