From bf5ef21454cc1b6cde1aac5d85a99dbba1c7e84e Mon Sep 17 00:00:00 2001 From: Chris Price Date: Wed, 4 May 2016 16:48:29 -0700 Subject: [PATCH] (TK-316) Create subclasses for ClientTimer --- .../impl/metrics/ClientMetricFilter.java | 37 ++++++++++++--- .../http/client/impl/metrics/TimerUtils.java | 9 ++-- .../http/client/metrics/ClientMetricData.java | 8 ++-- .../http/client/metrics/ClientTimer.java | 43 ++---------------- .../client/metrics/MetricIdClientTimer.java | 19 ++++++++ .../http/client/metrics/Metrics.java | 45 ++++++++++--------- .../metrics/UrlAndMethodClientTimer.java | 15 +++++++ .../http/client/metrics/UrlClientTimer.java | 14 ++++++ 8 files changed, 117 insertions(+), 73 deletions(-) create mode 100644 src/java/com/puppetlabs/http/client/metrics/MetricIdClientTimer.java create mode 100644 src/java/com/puppetlabs/http/client/metrics/UrlAndMethodClientTimer.java create mode 100644 src/java/com/puppetlabs/http/client/metrics/UrlClientTimer.java diff --git a/src/java/com/puppetlabs/http/client/impl/metrics/ClientMetricFilter.java b/src/java/com/puppetlabs/http/client/impl/metrics/ClientMetricFilter.java index 0f55b6f..3ccd769 100644 --- a/src/java/com/puppetlabs/http/client/impl/metrics/ClientMetricFilter.java +++ b/src/java/com/puppetlabs/http/client/impl/metrics/ClientMetricFilter.java @@ -2,8 +2,11 @@ package com.puppetlabs.http.client.impl.metrics; import com.codahale.metrics.MetricFilter; import com.codahale.metrics.Metric; +import com.puppetlabs.http.client.metrics.MetricIdClientTimer; import com.puppetlabs.http.client.metrics.Metrics; import com.puppetlabs.http.client.metrics.ClientTimer; +import com.puppetlabs.http.client.metrics.UrlAndMethodClientTimer; +import com.puppetlabs.http.client.metrics.UrlClientTimer; import java.util.ArrayList; @@ -14,6 +17,8 @@ public class ClientMetricFilter implements MetricFilter{ private ArrayList metricId; private Metrics.MetricType metricType; + // TODO: break this class up into two or more filter classes; it's combining a lot of logic at the moment + public ClientMetricFilter(String category, Metrics.MetricType metricType) { this.category = category; this.metricType = metricType; @@ -32,20 +37,40 @@ public class ClientMetricFilter implements MetricFilter{ if ( metric.getMetricType().equals(metricType) ) { if ( category != null ) { switch (category) { + // TODO: we should be able to break this up into multiple methods that accept the more + // concrete types in their signatures case Metrics.ID_NAMESPACE: - return metric.getMetricId() != null; + return metric instanceof MetricIdClientTimer; case Metrics.URL_METHOD_NAMESPACE: - return metric.getMethod() != null; + return metric instanceof UrlAndMethodClientTimer; case Metrics.URL_NAMESPACE: - return metric.getUrl() != null && metric.getMethod() == null; + return (metric instanceof UrlClientTimer) && + !(metric instanceof UrlAndMethodClientTimer); } } else { if ( method != null ) { - return url.equals(metric.getUrl()) && method.equals(metric.getMethod()) ; + // TODO: we should be able to break this up into multiple methods that accept the more + if (metric instanceof UrlAndMethodClientTimer) { + UrlAndMethodClientTimer urlAndMethodClientTimer = (UrlAndMethodClientTimer) metric; + return url.equals(urlAndMethodClientTimer.getUrl()) && method.equals(urlAndMethodClientTimer.getMethod()); + } else { + return false; + } } else if ( url != null ) { - return url.equals(metric.getUrl()) && metric.getMethod() == null; + if ((metric instanceof UrlClientTimer) && + !(metric instanceof UrlAndMethodClientTimer)) { + UrlClientTimer urlClientTimer = (UrlClientTimer) metric; + return url.equals(urlClientTimer.getUrl()); + } else { + return false; + } } else { - return metricId.equals(metric.getMetricId()); + if (metric instanceof MetricIdClientTimer) { + MetricIdClientTimer metricIdClientTimer = (MetricIdClientTimer) metric; + return metricId.equals(metricIdClientTimer.getMetricId()); + } else { + return false; + } } } } diff --git a/src/java/com/puppetlabs/http/client/impl/metrics/TimerUtils.java b/src/java/com/puppetlabs/http/client/impl/metrics/TimerUtils.java index 877c2f3..d4567b2 100644 --- a/src/java/com/puppetlabs/http/client/impl/metrics/TimerUtils.java +++ b/src/java/com/puppetlabs/http/client/impl/metrics/TimerUtils.java @@ -4,7 +4,10 @@ import com.codahale.metrics.Metric; import com.codahale.metrics.MetricRegistry; import com.codahale.metrics.Timer; import com.puppetlabs.http.client.metrics.ClientTimer; +import com.puppetlabs.http.client.metrics.MetricIdClientTimer; import com.puppetlabs.http.client.metrics.Metrics; +import com.puppetlabs.http.client.metrics.UrlAndMethodClientTimer; +import com.puppetlabs.http.client.metrics.UrlClientTimer; import org.apache.http.HttpRequest; import org.apache.http.RequestLine; import org.slf4j.Logger; @@ -53,7 +56,7 @@ public class TimerUtils { String metric_name = MetricRegistry.name(Metrics.METRIC_NAMESPACE, currentIdWithNamespace.toArray(new String[currentIdWithNamespace.size()])); - ClientTimer timer = new ClientTimer(metric_name, currentId, Metrics.MetricType.FULL_RESPONSE); + ClientTimer timer = new MetricIdClientTimer(metric_name, currentId, Metrics.MetricType.FULL_RESPONSE); timerContexts.add(getOrAddTimer(registry, metric_name, timer).time()); } return timerContexts; @@ -72,10 +75,10 @@ public class TimerUtils { final String urlAndMethodName = MetricRegistry.name(Metrics.METRIC_NAMESPACE, Metrics.URL_METHOD_NAMESPACE, strippedUrl, method, Metrics.FULL_RESPONSE_STRING); - ClientTimer urlTimer = new ClientTimer(urlName, strippedUrl, Metrics.MetricType.FULL_RESPONSE); + ClientTimer urlTimer = new UrlClientTimer(urlName, strippedUrl, Metrics.MetricType.FULL_RESPONSE); timerContexts.add(getOrAddTimer(registry, urlName, urlTimer).time()); - ClientTimer urlMethodTimer = new ClientTimer(urlAndMethodName, strippedUrl, + ClientTimer urlMethodTimer = new UrlAndMethodClientTimer(urlAndMethodName, strippedUrl, method, Metrics.MetricType.FULL_RESPONSE); timerContexts.add(getOrAddTimer(registry, urlAndMethodName, urlMethodTimer).time()); } catch (URISyntaxException e) { diff --git a/src/java/com/puppetlabs/http/client/metrics/ClientMetricData.java b/src/java/com/puppetlabs/http/client/metrics/ClientMetricData.java index 09e8ff6..6d7a80e 100644 --- a/src/java/com/puppetlabs/http/client/metrics/ClientMetricData.java +++ b/src/java/com/puppetlabs/http/client/metrics/ClientMetricData.java @@ -1,6 +1,6 @@ package com.puppetlabs.http.client.metrics; -import java.util.ArrayList; +import java.util.List; public class ClientMetricData { private String metricName; @@ -9,10 +9,10 @@ public class ClientMetricData { private Long aggregate; private String url; private String method; - private ArrayList metricId; + private List metricId; ClientMetricData(String metricName, Long count, Long mean, Long aggregate, - String url, String method, ArrayList metricId) { + String url, String method, List metricId) { this.metricName = metricName; this.count = count; this.mean = mean; @@ -46,7 +46,7 @@ public class ClientMetricData { return method; } - public ArrayList getMetricId() { + public List getMetricId() { return metricId; } } diff --git a/src/java/com/puppetlabs/http/client/metrics/ClientTimer.java b/src/java/com/puppetlabs/http/client/metrics/ClientTimer.java index 605f961..f91cde7 100644 --- a/src/java/com/puppetlabs/http/client/metrics/ClientTimer.java +++ b/src/java/com/puppetlabs/http/client/metrics/ClientTimer.java @@ -2,37 +2,14 @@ package com.puppetlabs.http.client.metrics; import com.codahale.metrics.Timer; -import java.util.ArrayList; +public abstract class ClientTimer extends Timer { + private final String metricName; -public class ClientTimer extends Timer { - private String metricName; - private String url; - private String method; - private ArrayList metricId; + private final Metrics.MetricType metricType; - private Metrics.MetricType metricType; - - public ClientTimer(String metricName, String url, Metrics.MetricType metricType) { + ClientTimer(String metricName, Metrics.MetricType metricType) { super(); this.metricName = metricName; - this.url = url; - this.metricType = metricType; - } - - public ClientTimer(String metricName, String url, String method, - Metrics.MetricType metricType) { - super(); - this.metricName = metricName; - this.url = url; - this.method = method; - this.metricType = metricType; - } - - public ClientTimer(String metricName, ArrayList metricId, - Metrics.MetricType metricType) { - super(); - this.metricName = metricName; - this.metricId = metricId; this.metricType = metricType; } @@ -40,18 +17,6 @@ public class ClientTimer extends Timer { return metricName; } - public String getUrl() { - return url; - } - - public String getMethod() { - return method; - } - - public ArrayList getMetricId() { - return metricId; - } - public Metrics.MetricType getMetricType() { return metricType; } diff --git a/src/java/com/puppetlabs/http/client/metrics/MetricIdClientTimer.java b/src/java/com/puppetlabs/http/client/metrics/MetricIdClientTimer.java new file mode 100644 index 0000000..5ea65a1 --- /dev/null +++ b/src/java/com/puppetlabs/http/client/metrics/MetricIdClientTimer.java @@ -0,0 +1,19 @@ +package com.puppetlabs.http.client.metrics; + +import java.util.ArrayList; +import java.util.List; + +public class MetricIdClientTimer extends ClientTimer { + + private final List metricId; + + public MetricIdClientTimer(String metricName, List metricId, + Metrics.MetricType metricType) { + super(metricName, metricType); + this.metricId = metricId; + } + + public List getMetricId() { + return metricId; + } +} diff --git a/src/java/com/puppetlabs/http/client/metrics/Metrics.java b/src/java/com/puppetlabs/http/client/metrics/Metrics.java index a85662b..9921714 100644 --- a/src/java/com/puppetlabs/http/client/metrics/Metrics.java +++ b/src/java/com/puppetlabs/http/client/metrics/Metrics.java @@ -9,6 +9,7 @@ import java.net.URISyntaxException; import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.concurrent.TimeUnit; @@ -27,8 +28,8 @@ public class Metrics { return convertedUri.toString(); } - public static ArrayList getClientTimerArray(Map timers){ - ArrayList timerArray = new ArrayList<>(); + public static List getClientTimerArray(Map timers){ + List timerArray = new ArrayList<>(); for (Map.Entry entry : timers.entrySet()) { ClientTimer timer = (ClientTimer)entry.getValue(); timerArray.add(timer); @@ -36,9 +37,9 @@ public class Metrics { return timerArray; } - public static Map> getClientMetrics(MetricRegistry metricRegistry){ + public static Map> getClientMetrics(MetricRegistry metricRegistry){ if (metricRegistry != null) { - Map> timers = new HashMap<>(); + Map> timers = new HashMap<>(); timers.put("url", getClientTimerArray(metricRegistry.getTimers( new ClientMetricFilter(URL_NAMESPACE, MetricType.FULL_RESPONSE)))); timers.put("url-and-method", getClientTimerArray(metricRegistry.getTimers( @@ -51,7 +52,7 @@ public class Metrics { } } - public static ArrayList getClientMetricsByUrl(MetricRegistry metricRegistry, + public static List getClientMetricsByUrl(MetricRegistry metricRegistry, final String url){ if (metricRegistry != null) { Map timers = metricRegistry.getTimers( @@ -62,7 +63,7 @@ public class Metrics { } } - public static ArrayList getClientMetricsByUrlAndMethod(MetricRegistry metricRegistry, + public static List getClientMetricsByUrlAndMethod(MetricRegistry metricRegistry, final String url, final String method){ if (metricRegistry != null) { @@ -74,7 +75,7 @@ public class Metrics { } } - public static ArrayList getClientMetricsByMetricId(MetricRegistry metricRegistry, + public static List getClientMetricsByMetricId(MetricRegistry metricRegistry, final String[] metricId){ if (metricRegistry != null) { if (metricId.length == 0) { @@ -93,18 +94,20 @@ public class Metrics { } } - public static ArrayList computeClientMetricsData(ArrayList timers){ + public static List computeClientMetricsData(List timers){ if (timers != null) { - ArrayList metricsData = new ArrayList<>(); + List metricsData = new ArrayList<>(); for (ClientTimer timer: timers) { Double mean = timer.getSnapshot().getMean(); Long meanMillis = TimeUnit.NANOSECONDS.toMillis(mean.longValue()); Long count = timer.getCount(); Long aggregate = count * meanMillis; String metricName = timer.getMetricName(); - String url = timer.getUrl(); - String method = timer.getMethod(); - ArrayList metricId = timer.getMetricId(); + // TODO: create subclasses of ClientMetricData to prevent null values from being necessary, + // refactor into methods with types in signatures to get rid of instanceof stuff. + String url = timer instanceof UrlClientTimer ? ((UrlClientTimer)(timer)).getUrl() : null; + String method = timer instanceof UrlAndMethodClientTimer ? ((UrlAndMethodClientTimer)(timer)).getMethod() : null; + List metricId = timer instanceof MetricIdClientTimer ? ((MetricIdClientTimer)(timer)).getMetricId() : null; ClientMetricData data = new ClientMetricData(metricName, count, meanMillis, aggregate, url, method, metricId); @@ -116,10 +119,10 @@ public class Metrics { } } - public static Map> getClientMetricsData(MetricRegistry metricRegistry){ + public static Map> getClientMetricsData(MetricRegistry metricRegistry){ if ( metricRegistry != null ) { - Map> timers = getClientMetrics(metricRegistry); - Map> data = new HashMap<>(); + Map> timers = getClientMetrics(metricRegistry); + Map> data = new HashMap<>(); data.put("url", computeClientMetricsData(timers.get("url"))); data.put("url-and-method", computeClientMetricsData(timers.get("url-and-method"))); data.put("metric-id", computeClientMetricsData(timers.get("metric-id"))); @@ -129,22 +132,22 @@ public class Metrics { } } - public static ArrayList getClientMetricsDataByUrl(MetricRegistry metricRegistry, + public static List getClientMetricsDataByUrl(MetricRegistry metricRegistry, String url){ - ArrayList timers = getClientMetricsByUrl(metricRegistry, url); + List timers = getClientMetricsByUrl(metricRegistry, url); return computeClientMetricsData(timers); } - public static ArrayList getClientMetricsDataByUrlAndMethod(MetricRegistry metricRegistry, + public static List getClientMetricsDataByUrlAndMethod(MetricRegistry metricRegistry, String url, String method){ - ArrayList timers = getClientMetricsByUrlAndMethod(metricRegistry, url, method); + List timers = getClientMetricsByUrlAndMethod(metricRegistry, url, method); return computeClientMetricsData(timers); } - public static ArrayList getClientMetricsDataByMetricId(MetricRegistry metricRegistry, + public static List getClientMetricsDataByMetricId(MetricRegistry metricRegistry, String[] metricId){ - ArrayList timers = getClientMetricsByMetricId(metricRegistry, metricId); + List timers = getClientMetricsByMetricId(metricRegistry, metricId); return computeClientMetricsData(timers); } } diff --git a/src/java/com/puppetlabs/http/client/metrics/UrlAndMethodClientTimer.java b/src/java/com/puppetlabs/http/client/metrics/UrlAndMethodClientTimer.java new file mode 100644 index 0000000..5a67a24 --- /dev/null +++ b/src/java/com/puppetlabs/http/client/metrics/UrlAndMethodClientTimer.java @@ -0,0 +1,15 @@ +package com.puppetlabs.http.client.metrics; + +public class UrlAndMethodClientTimer extends UrlClientTimer { + private final String method; + + public UrlAndMethodClientTimer(String metricName, String url, String method, + Metrics.MetricType metricType) { + super(metricName, url, metricType); + this.method = method; + } + + public String getMethod() { + return method; + } +} diff --git a/src/java/com/puppetlabs/http/client/metrics/UrlClientTimer.java b/src/java/com/puppetlabs/http/client/metrics/UrlClientTimer.java new file mode 100644 index 0000000..e0d1ae2 --- /dev/null +++ b/src/java/com/puppetlabs/http/client/metrics/UrlClientTimer.java @@ -0,0 +1,14 @@ +package com.puppetlabs.http.client.metrics; + +public class UrlClientTimer extends ClientTimer { + private final String url; + + public UrlClientTimer(String metricName, String url, Metrics.MetricType metricType) { + super(metricName, metricType); + this.url = url; + } + + public String getUrl() { + return url; + } +}