Refactoring and cleanup in the Metrics module:
* Removed logging related code inside class MetricsManager * Removed @Component related code, because it is not suppose to be used as an component. * Added a new class-wrapper so the Metrics can be used as a loadable service: MetricsManagerComponent. The name and the location of this class will be refactored in the future. * Added new method MetricsManager.removeMetric() * Line formatting * Changed "interface MetricsService" to public
Showing
4 changed files
with
69 additions
and
48 deletions
... | @@ -68,7 +68,6 @@ private static Logger log = LoggerFactory.getLogger(SimpleNettyClient.class); | ... | @@ -68,7 +68,6 @@ private static Logger log = LoggerFactory.getLogger(SimpleNettyClient.class); |
68 | metrics = new MetricsManager(); | 68 | metrics = new MetricsManager(); |
69 | Endpoint endpoint = new Endpoint(host, port); | 69 | Endpoint endpoint = new Endpoint(host, port); |
70 | messaging.activate(); | 70 | messaging.activate(); |
71 | - metrics.activate(); | ||
72 | MetricsFeature feature = new MetricsFeature("latency"); | 71 | MetricsFeature feature = new MetricsFeature("latency"); |
73 | MetricsComponent component = metrics.registerComponent("NettyMessaging"); | 72 | MetricsComponent component = metrics.registerComponent("NettyMessaging"); |
74 | log.info("connecting " + host + ":" + port + " warmup:" + warmup + " iterations:" + iterations); | 73 | log.info("connecting " + host + ":" + port + " warmup:" + warmup + " iterations:" + iterations); |
... | @@ -117,7 +116,6 @@ private static Logger log = LoggerFactory.getLogger(SimpleNettyClient.class); | ... | @@ -117,7 +116,6 @@ private static Logger log = LoggerFactory.getLogger(SimpleNettyClient.class); |
117 | public static void stop() { | 116 | public static void stop() { |
118 | try { | 117 | try { |
119 | messaging.deactivate(); | 118 | messaging.deactivate(); |
120 | - metrics.deactivate(); | ||
121 | } catch (Exception e) { | 119 | } catch (Exception e) { |
122 | log.info("Unable to stop client %s", e); | 120 | log.info("Unable to stop client %s", e); |
123 | } | 121 | } | ... | ... |
1 | +package org.onlab.onos.impl; | ||
2 | + | ||
3 | +import org.apache.felix.scr.annotations.Activate; | ||
4 | +import org.apache.felix.scr.annotations.Component; | ||
5 | +import org.apache.felix.scr.annotations.Deactivate; | ||
6 | +import org.apache.felix.scr.annotations.Service; | ||
7 | + | ||
8 | +import org.onlab.metrics.MetricsManager; | ||
9 | + | ||
10 | +/** | ||
11 | + * Metrics service implementation. | ||
12 | + */ | ||
13 | +@Component(immediate = true) | ||
14 | +@Service | ||
15 | +public class MetricsManagerComponent extends MetricsManager { | ||
16 | + | ||
17 | + @Activate | ||
18 | + protected void activate() { | ||
19 | + super.clear(); | ||
20 | + } | ||
21 | + | ||
22 | + @Deactivate | ||
23 | + protected void deactivate() { | ||
24 | + super.clear(); | ||
25 | + } | ||
26 | +} |
... | @@ -3,15 +3,7 @@ package org.onlab.metrics; | ... | @@ -3,15 +3,7 @@ package org.onlab.metrics; |
3 | import java.util.Map; | 3 | import java.util.Map; |
4 | import java.util.concurrent.ConcurrentHashMap; | 4 | import java.util.concurrent.ConcurrentHashMap; |
5 | import java.util.concurrent.ConcurrentMap; | 5 | import java.util.concurrent.ConcurrentMap; |
6 | -import java.util.concurrent.TimeUnit; | ||
7 | 6 | ||
8 | -import org.apache.felix.scr.annotations.Activate; | ||
9 | -import org.apache.felix.scr.annotations.Component; | ||
10 | -import org.apache.felix.scr.annotations.Deactivate; | ||
11 | -import org.slf4j.Logger; | ||
12 | -import org.slf4j.LoggerFactory; | ||
13 | - | ||
14 | -import com.codahale.metrics.ConsoleReporter; | ||
15 | import com.codahale.metrics.Counter; | 7 | import com.codahale.metrics.Counter; |
16 | import com.codahale.metrics.Gauge; | 8 | import com.codahale.metrics.Gauge; |
17 | import com.codahale.metrics.Histogram; | 9 | import com.codahale.metrics.Histogram; |
... | @@ -53,48 +45,25 @@ import com.codahale.metrics.Timer; | ... | @@ -53,48 +45,25 @@ import com.codahale.metrics.Timer; |
53 | * </code> | 45 | * </code> |
54 | * </pre> | 46 | * </pre> |
55 | */ | 47 | */ |
56 | -@Component(immediate = true) | 48 | +public class MetricsManager implements MetricsService { |
57 | -public final class MetricsManager implements MetricsService { | ||
58 | 49 | ||
59 | - private final Logger log = LoggerFactory.getLogger(getClass()); | ||
60 | /** | 50 | /** |
61 | * Registry to hold the Components defined in the system. | 51 | * Registry to hold the Components defined in the system. |
62 | */ | 52 | */ |
63 | - private ConcurrentMap<String, MetricsComponent> componentsRegistry; | 53 | + private ConcurrentMap<String, MetricsComponent> componentsRegistry = |
54 | + new ConcurrentHashMap<>(); | ||
64 | 55 | ||
65 | /** | 56 | /** |
66 | * Registry for the Metrics objects created in the system. | 57 | * Registry for the Metrics objects created in the system. |
67 | */ | 58 | */ |
68 | - private final MetricRegistry metricsRegistry; | 59 | + private MetricRegistry metricsRegistry = new MetricRegistry(); |
69 | 60 | ||
70 | /** | 61 | /** |
71 | - * Default Reporter for this metrics manager. | 62 | + * Clears the internal state. |
72 | */ | 63 | */ |
73 | - //private final Slf4jReporter reporter; | 64 | + protected void clear() { |
74 | - private final ConsoleReporter reporter; | ||
75 | - | ||
76 | - public MetricsManager() { | ||
77 | - this.metricsRegistry = new MetricRegistry(); | ||
78 | -// this.reporter = Slf4jReporter.forRegistry(this.metricsRegistry) | ||
79 | -// .outputTo(log) | ||
80 | -// .convertRatesTo(TimeUnit.SECONDS) | ||
81 | -// .convertDurationsTo(TimeUnit.MICROSECONDS) | ||
82 | -// .build(); | ||
83 | - this.reporter = ConsoleReporter.forRegistry(this.metricsRegistry) | ||
84 | - .convertRatesTo(TimeUnit.SECONDS) | ||
85 | - .convertDurationsTo(TimeUnit.MICROSECONDS) | ||
86 | - .build(); | ||
87 | - } | ||
88 | - | ||
89 | - @Activate | ||
90 | - public void activate() { | ||
91 | this.componentsRegistry = new ConcurrentHashMap<>(); | 65 | this.componentsRegistry = new ConcurrentHashMap<>(); |
92 | - reporter.start(10, TimeUnit.SECONDS); | 66 | + this.metricsRegistry = new MetricRegistry(); |
93 | - } | ||
94 | - | ||
95 | - @Deactivate | ||
96 | - public void deactivate() { | ||
97 | - reporter.stop(); | ||
98 | } | 67 | } |
99 | 68 | ||
100 | /** | 69 | /** |
... | @@ -107,7 +76,8 @@ public final class MetricsManager implements MetricsService { | ... | @@ -107,7 +76,8 @@ public final class MetricsManager implements MetricsService { |
107 | public MetricsComponent registerComponent(final String name) { | 76 | public MetricsComponent registerComponent(final String name) { |
108 | MetricsComponent component = componentsRegistry.get(name); | 77 | MetricsComponent component = componentsRegistry.get(name); |
109 | if (component == null) { | 78 | if (component == null) { |
110 | - final MetricsComponent createdComponent = new MetricsComponent(name); | 79 | + final MetricsComponent createdComponent = |
80 | + new MetricsComponent(name); | ||
111 | component = componentsRegistry.putIfAbsent(name, createdComponent); | 81 | component = componentsRegistry.putIfAbsent(name, createdComponent); |
112 | if (component == null) { | 82 | if (component == null) { |
113 | component = createdComponent; | 83 | component = createdComponent; |
... | @@ -221,6 +191,22 @@ public final class MetricsManager implements MetricsService { | ... | @@ -221,6 +191,22 @@ public final class MetricsManager implements MetricsService { |
221 | } | 191 | } |
222 | 192 | ||
223 | /** | 193 | /** |
194 | + * Removes the metric with the given name. | ||
195 | + * | ||
196 | + * @param component component the Metric is defined in | ||
197 | + * @param feature feature the Metric is defined in | ||
198 | + * @param metricName local name of the metric | ||
199 | + * @return true if the metric existed and was removed, otherwise false | ||
200 | + */ | ||
201 | + @Override | ||
202 | + public boolean removeMetric(final MetricsComponent component, | ||
203 | + final MetricsFeature feature, | ||
204 | + final String metricName) { | ||
205 | + final String name = generateName(component, feature, metricName); | ||
206 | + return metricsRegistry.remove(name); | ||
207 | + } | ||
208 | + | ||
209 | + /** | ||
224 | * Fetches the existing Timers. | 210 | * Fetches the existing Timers. |
225 | * | 211 | * |
226 | * @param filter filter to use to select Timers | 212 | * @param filter filter to use to select Timers |
... | @@ -272,8 +258,8 @@ public final class MetricsManager implements MetricsService { | ... | @@ -272,8 +258,8 @@ public final class MetricsManager implements MetricsService { |
272 | * Fetches the existing Histograms. | 258 | * Fetches the existing Histograms. |
273 | * | 259 | * |
274 | * @param filter filter to use to select Histograms | 260 | * @param filter filter to use to select Histograms |
275 | - * @return a map of the Histograms that match the filter, with the key as the | 261 | + * @return a map of the Histograms that match the filter, with the key as |
276 | - * name String to the Histogram. | 262 | + * the name String to the Histogram. |
277 | */ | 263 | */ |
278 | @Override | 264 | @Override |
279 | public Map<String, Histogram> getHistograms(final MetricFilter filter) { | 265 | public Map<String, Histogram> getHistograms(final MetricFilter filter) { |
... | @@ -290,4 +276,3 @@ public final class MetricsManager implements MetricsService { | ... | @@ -290,4 +276,3 @@ public final class MetricsManager implements MetricsService { |
290 | metricsRegistry.removeMatching(filter); | 276 | metricsRegistry.removeMatching(filter); |
291 | } | 277 | } |
292 | } | 278 | } |
293 | - | ... | ... |
... | @@ -13,7 +13,7 @@ import com.codahale.metrics.Timer; | ... | @@ -13,7 +13,7 @@ import com.codahale.metrics.Timer; |
13 | /** | 13 | /** |
14 | * Metrics Service to collect metrics. | 14 | * Metrics Service to collect metrics. |
15 | */ | 15 | */ |
16 | -interface MetricsService { | 16 | +public interface MetricsService { |
17 | 17 | ||
18 | /** | 18 | /** |
19 | * Registers a component. | 19 | * Registers a component. |
... | @@ -90,6 +90,18 @@ interface MetricsService { | ... | @@ -90,6 +90,18 @@ interface MetricsService { |
90 | T metric); | 90 | T metric); |
91 | 91 | ||
92 | /** | 92 | /** |
93 | + * Removes the metric with the given name. | ||
94 | + * | ||
95 | + * @param component component the Metric is defined in | ||
96 | + * @param feature feature the Metric is defined in | ||
97 | + * @param metricName local name of the metric | ||
98 | + * @return true if the metric existed and was removed, otherwise false | ||
99 | + */ | ||
100 | + boolean removeMetric(MetricsComponent component, | ||
101 | + MetricsFeature feature, | ||
102 | + String metricName); | ||
103 | + | ||
104 | + /** | ||
93 | * Fetches the existing Timers. | 105 | * Fetches the existing Timers. |
94 | * | 106 | * |
95 | * @param filter filter to use to select Timers | 107 | * @param filter filter to use to select Timers |
... | @@ -129,15 +141,15 @@ interface MetricsService { | ... | @@ -129,15 +141,15 @@ interface MetricsService { |
129 | * Fetches the existing Histograms. | 141 | * Fetches the existing Histograms. |
130 | * | 142 | * |
131 | * @param filter filter to use to select Histograms | 143 | * @param filter filter to use to select Histograms |
132 | - * @return a map of the Histograms that match the filter, with the key as the | 144 | + * @return a map of the Histograms that match the filter, with the key as |
133 | - * name String to the Histogram. | 145 | + * the name String to the Histogram. |
134 | */ | 146 | */ |
135 | Map<String, Histogram> getHistograms(MetricFilter filter); | 147 | Map<String, Histogram> getHistograms(MetricFilter filter); |
148 | + | ||
136 | /** | 149 | /** |
137 | * Removes all Metrics that match a given filter. | 150 | * Removes all Metrics that match a given filter. |
138 | * | 151 | * |
139 | * @param filter filter to use to select the Metrics to remove. | 152 | * @param filter filter to use to select the Metrics to remove. |
140 | */ | 153 | */ |
141 | void removeMatching(MetricFilter filter); | 154 | void removeMatching(MetricFilter filter); |
142 | - | ||
143 | } | 155 | } | ... | ... |
-
Please register or login to post a comment