diff options
author | gjoranv <gv@verizonmedia.com> | 2021-01-29 12:02:40 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-01-29 12:02:40 +0100 |
commit | 6ebeb3a650c88cb3dedfc57c57728c8376f21f44 (patch) | |
tree | 587ff0caaa096c87af0a7bac5c31309b00f46456 | |
parent | b520261986e8a0444f2902396b15bf059a21004f (diff) | |
parent | 7c00b99fe9cfc3132ddad0226c2613adf2ea4d5d (diff) |
Merge pull request #16227 from vespa-engine/gjoranv/remove-StateMetricConsumer
Gjoranv/remove state metric consumer
17 files changed, 207 insertions, 647 deletions
diff --git a/container-core/src/main/java/com/yahoo/container/jdisc/state/MetricsPacketsHandler.java b/container-core/src/main/java/com/yahoo/container/jdisc/state/MetricsPacketsHandler.java index c1a6f650a9c..3d3f0e4b677 100644 --- a/container-core/src/main/java/com/yahoo/container/jdisc/state/MetricsPacketsHandler.java +++ b/container-core/src/main/java/com/yahoo/container/jdisc/state/MetricsPacketsHandler.java @@ -143,7 +143,8 @@ public class MetricsPacketsHandler extends AbstractRequestHandler { private MetricSnapshot getSnapshot() { if (snapshotPreprocessor == null) { - return monitor.snapshot(); + // TODO: throw exception in ctor instead + return new MetricSnapshot(0L, 0L, TimeUnit.MILLISECONDS); } else { return snapshotPreprocessor.latestSnapshot(); } @@ -173,6 +174,8 @@ public class MetricsPacketsHandler extends AbstractRequestHandler { } private void addDimensions(MetricDimensions metricDimensions, JSONObjectWithLegibleException packet) throws JSONException { + if (metricDimensions == null) return; + Iterator<Map.Entry<String, String>> dimensionsIterator = metricDimensions.iterator(); if (dimensionsIterator.hasNext()) { JSONObject jsonDim = new JSONObjectWithLegibleException(); diff --git a/container-core/src/main/java/com/yahoo/container/jdisc/state/StateHandler.java b/container-core/src/main/java/com/yahoo/container/jdisc/state/StateHandler.java index 8858465e846..b14dc50edcb 100644 --- a/container-core/src/main/java/com/yahoo/container/jdisc/state/StateHandler.java +++ b/container-core/src/main/java/com/yahoo/container/jdisc/state/StateHandler.java @@ -63,11 +63,7 @@ public class StateHandler extends AbstractRequestHandler { static SnapshotProvider getSnapshotPreprocessor(ComponentRegistry<SnapshotProvider> preprocessors, MetricsPresentationConfig presentation) { List<SnapshotProvider> allPreprocessors = preprocessors.allComponents(); - if (presentation.slidingwindow() && allPreprocessors.size() > 0) { - return allPreprocessors.get(0); - } else { - return null; - } + return (allPreprocessors.size() > 0) ? allPreprocessors.get(0) : null; } @Override @@ -205,7 +201,8 @@ public class StateHandler extends AbstractRequestHandler { private MetricSnapshot getSnapshot() { if (snapshotPreprocessor == null) { - return monitor.snapshot(); + // TODO: throw exception in ctor instead + return new MetricSnapshot(0L, 0L, TimeUnit.MILLISECONDS); } else { return snapshotPreprocessor.latestSnapshot(); } @@ -254,14 +251,16 @@ public class StateHandler extends AbstractRequestHandler { } else { throw new UnsupportedOperationException(tuple.val.getClass().getName()); } - Iterator<Map.Entry<String, String>> it = tuple.dim.iterator(); - if (it.hasNext() && includeDimensions) { - JSONObjectWithLegibleException jsonDim = new JSONObjectWithLegibleException(); - while (it.hasNext()) { - Map.Entry<String, String> entry = it.next(); - jsonDim.put(entry.getKey(), entry.getValue()); + if (tuple.dim != null) { + Iterator<Map.Entry<String, String>> it = tuple.dim.iterator(); + if (it.hasNext() && includeDimensions) { + JSONObjectWithLegibleException jsonDim = new JSONObjectWithLegibleException(); + while (it.hasNext()) { + Map.Entry<String, String> entry = it.next(); + jsonDim.put(entry.getKey(), entry.getValue()); + } + jsonTuple.put("dimensions", jsonDim); } - jsonTuple.put("dimensions", jsonDim); } jsonMetric.append("values", jsonTuple); } diff --git a/container-core/src/main/java/com/yahoo/container/jdisc/state/StateMetricConsumer.java b/container-core/src/main/java/com/yahoo/container/jdisc/state/StateMetricConsumer.java deleted file mode 100644 index dfa791304e0..00000000000 --- a/container-core/src/main/java/com/yahoo/container/jdisc/state/StateMetricConsumer.java +++ /dev/null @@ -1,50 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.container.jdisc.state; - -import com.yahoo.jdisc.Metric; -import com.yahoo.jdisc.application.MetricConsumer; - -import java.util.Map; - -/** - * @author Simon Thoresen Hult - */ -final class StateMetricConsumer implements MetricConsumer { - - final static Metric.Context NULL_CONTEXT = StateMetricContext.newInstance(null); - private final Object lock = new Object(); - private MetricSnapshot metricSnapshot = new MetricSnapshot(); - - @Override - public void set(String key, Number val, Metric.Context ctx) { - synchronized (lock) { - metricSnapshot.set(dimensionsOrDefault(ctx), key, val); - } - } - - private MetricDimensions dimensionsOrDefault(Metric.Context ctx) { - return (MetricDimensions)(ctx != null ? ctx : NULL_CONTEXT); - } - - @Override - public void add(String key, Number val, Metric.Context ctx) { - synchronized (lock) { - metricSnapshot.add(dimensionsOrDefault(ctx), key, val); - } - } - - @Override - public Metric.Context createContext(Map<String, ?> properties) { - return StateMetricContext.newInstance(properties); - } - - MetricSnapshot createSnapshot() { - MetricSnapshot metricSnapshot; - synchronized (lock) { - metricSnapshot = this.metricSnapshot; - this.metricSnapshot = this.metricSnapshot.createSnapshot(); - } - return metricSnapshot; - } - -} diff --git a/container-core/src/main/java/com/yahoo/container/jdisc/state/StateMonitor.java b/container-core/src/main/java/com/yahoo/container/jdisc/state/StateMonitor.java index 40a0ef10fbc..027ce02a2aa 100644 --- a/container-core/src/main/java/com/yahoo/container/jdisc/state/StateMonitor.java +++ b/container-core/src/main/java/com/yahoo/container/jdisc/state/StateMonitor.java @@ -4,25 +4,12 @@ package com.yahoo.container.jdisc.state; import com.google.inject.Inject; import com.yahoo.component.AbstractComponent; import com.yahoo.container.jdisc.config.HealthMonitorConfig; -import com.yahoo.jdisc.Timer; -import com.yahoo.jdisc.application.MetricConsumer; -import com.yahoo.jdisc.core.SystemTimer; -import java.util.Map; -import java.util.Optional; -import java.util.TreeSet; -import java.util.concurrent.CopyOnWriteArrayList; -import java.util.concurrent.Executors; -import java.util.concurrent.ScheduledExecutorService; -import java.util.concurrent.ThreadFactory; -import java.util.concurrent.TimeUnit; import java.util.logging.Level; import java.util.logging.Logger; /** - * A state monitor keeps track of the current health and metrics state of a container. - * It is used by jDisc to hand out metric update API endpoints to workers through {@link #newMetricConsumer}, - * and to inspect the current accumulated state of metrics through {@link #snapshot}. + * A state monitor keeps track of the current health state of a container. * * @author Simon Thoresen Hult */ @@ -32,44 +19,15 @@ public class StateMonitor extends AbstractComponent { public enum Status {up, down, initializing} - private final CopyOnWriteArrayList<StateMetricConsumer> consumers = new CopyOnWriteArrayList<>(); - private final Optional<ScheduledExecutorService> executor; - private final Timer timer; - private final long snapshotIntervalMs; - private volatile long lastSnapshotTimeMs; - private volatile MetricSnapshot snapshot; private volatile Status status; - private final TreeSet<String> valueNames = new TreeSet<>(); @Inject - public StateMonitor(HealthMonitorConfig config, Timer timer) { - this(config, timer, runnable -> { - Thread thread = new Thread(runnable, "StateMonitor"); - thread.setDaemon(true); - return thread; - }); - } - - public static StateMonitor createForTesting() { - return new StateMonitor(new HealthMonitorConfig.Builder().build(), new SystemTimer()); - } - - /** Non-private only for unit testing this class. */ - StateMonitor(HealthMonitorConfig config, Timer timer, ThreadFactory threadFactory) { - this.timer = timer; - this.snapshotIntervalMs = (long)(config.snapshot_interval() * TimeUnit.SECONDS.toMillis(1)); - this.lastSnapshotTimeMs = timer.currentTimeMillis(); + public StateMonitor(HealthMonitorConfig config) { this.status = Status.valueOf(config.initialStatus()); - this.executor = Optional.ofNullable(threadFactory).map(Executors::newSingleThreadScheduledExecutor); - this.executor.ifPresent(exec -> exec.scheduleAtFixedRate(this::updateSnapshot, snapshotIntervalMs, - snapshotIntervalMs, TimeUnit.MILLISECONDS)); } - /** Returns a metric consumer for jDisc which will write metrics back to this */ - public MetricConsumer newMetricConsumer() { - StateMetricConsumer consumer = new StateMetricConsumer(); - consumers.add(consumer); - return consumer; + public static StateMonitor createForTesting() { + return new StateMonitor(new HealthMonitorConfig.Builder().build()); } public void status(Status status) { @@ -81,60 +39,4 @@ public class StateMonitor extends AbstractComponent { public Status status() { return status; } - /** Returns the last snapshot taken of the metrics in this system */ - public MetricSnapshot snapshot() { - return snapshot; - } - - /** Returns the interval between each metrics snapshot used by this */ - public long getSnapshotIntervalMillis() { return snapshotIntervalMs; } - - /** NOTE: Non-private for unit testing only. **/ - void updateSnapshot() { - long now = timer.currentTimeMillis(); - snapshot = createSnapshot(lastSnapshotTimeMs, now); - lastSnapshotTimeMs = now; - } - - private MetricSnapshot createSnapshot(long fromMillis, long toMillis) { - MetricSnapshot snapshot = new MetricSnapshot(fromMillis, toMillis, TimeUnit.MILLISECONDS); - for (StateMetricConsumer consumer : consumers) { - snapshot.add(consumer.createSnapshot()); - } - updateNames(snapshot); - return snapshot; - } - - private void updateNames(MetricSnapshot current) { - TreeSet<String> seen = new TreeSet<>(); - for (Map.Entry<MetricDimensions, MetricSet> dimensionAndMetric : current) { - for (Map.Entry<String, MetricValue> nameAndMetric : dimensionAndMetric.getValue()) { - seen.add(nameAndMetric.getKey()); - } - } - synchronized (valueNames) { - for (String name : valueNames) { - if (!seen.contains(name)) { - current.add((MetricDimensions) StateMetricConsumer.NULL_CONTEXT, name, 0); - } - } - valueNames.addAll(seen); - } - } - - @Override - public void deconstruct() { - executor.ifPresent(exec -> { - exec.shutdown(); - try { - exec.awaitTermination(5, TimeUnit.SECONDS); - } catch (InterruptedException e) { - } - - if (!exec.isTerminated()) { - log.warning("StateMonitor failed to terminate within 5 seconds of interrupt signal. Ignoring."); - } - }); - } - } diff --git a/container-core/src/main/java/com/yahoo/container/jdisc/state/package-info.java b/container-core/src/main/java/com/yahoo/container/jdisc/state/package-info.java index 4d83d69a402..52b7af2a02c 100644 --- a/container-core/src/main/java/com/yahoo/container/jdisc/state/package-info.java +++ b/container-core/src/main/java/com/yahoo/container/jdisc/state/package-info.java @@ -1,12 +1,10 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +/** + * Metrics implementation for jDisc. This consumes metrics over the jDisc metric API and + * makes these available for in-process consumption, and off-process through a jDisc handler. + */ @ExportPackage package com.yahoo.container.jdisc.state; import com.yahoo.osgi.annotation.ExportPackage; - -/** - * Metrics implementation for jDisc. This consumes metrics over the jDisc metric API - * and makes these available for in-process consumption through - * {@link com.yahoo.container.jdisc.state.StateMonitor#snapshot}, - * and off-process through a jDisc handler. - */ diff --git a/container-core/src/main/resources/configdefinitions/metrics.metrics-presentation.def b/container-core/src/main/resources/configdefinitions/metrics.metrics-presentation.def index b6c40993ef5..aeb597ed326 100644 --- a/container-core/src/main/resources/configdefinitions/metrics.metrics-presentation.def +++ b/container-core/src/main/resources/configdefinitions/metrics.metrics-presentation.def @@ -2,6 +2,5 @@ namespace=metrics -## Sliding window means present the last n minutes of data, as opposed to -## presenting the newest completed n minute interval. +## TODO Vespa 8: remove slidingwindow bool default=true diff --git a/container-core/src/test/java/com/yahoo/container/jdisc/state/MetricConsumerProviders.java b/container-core/src/test/java/com/yahoo/container/jdisc/state/MetricConsumerProviders.java deleted file mode 100644 index 777f43c5607..00000000000 --- a/container-core/src/test/java/com/yahoo/container/jdisc/state/MetricConsumerProviders.java +++ /dev/null @@ -1,21 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.container.jdisc.state; - -import com.google.inject.Provider; -import com.yahoo.jdisc.application.MetricConsumer; - -/** - * @author Simon Thoresen Hult - */ -class MetricConsumerProviders { - - public static Provider<MetricConsumer> wrap(final StateMonitor statetMonitor) { - return new Provider<MetricConsumer>() { - - @Override - public MetricConsumer get() { - return statetMonitor.newMetricConsumer(); - } - }; - } -} diff --git a/container-core/src/test/java/com/yahoo/container/jdisc/state/MetricsPacketsHandlerTest.java b/container-core/src/test/java/com/yahoo/container/jdisc/state/MetricsPacketsHandlerTest.java index ef700597537..89bde3eeb5b 100644 --- a/container-core/src/test/java/com/yahoo/container/jdisc/state/MetricsPacketsHandlerTest.java +++ b/container-core/src/test/java/com/yahoo/container/jdisc/state/MetricsPacketsHandlerTest.java @@ -3,12 +3,13 @@ package com.yahoo.container.jdisc.state; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; -import com.yahoo.jdisc.Metric; +import com.yahoo.container.jdisc.RequestHandlerTestDriver; +import org.junit.Before; import org.junit.Test; import java.util.ArrayList; -import java.util.Collections; import java.util.List; +import java.util.Map; import static com.yahoo.container.jdisc.state.MetricsPacketsHandler.APPLICATION_KEY; import static com.yahoo.container.jdisc.state.MetricsPacketsHandler.DIMENSIONS_KEY; @@ -25,9 +26,21 @@ import static org.junit.Assert.assertTrue; */ public class MetricsPacketsHandlerTest extends StateHandlerTestBase { + private static final String APPLICATION_NAME = "state-handler-test-base"; + + private static MetricsPacketsHandler metricsPacketsHandler; + + @Before + public void setupHandler() { + metricsPacketsHandlerConfig = new MetricsPacketsHandlerConfig(new MetricsPacketsHandlerConfig.Builder() + .application(APPLICATION_NAME)); + metricsPacketsHandler = new MetricsPacketsHandler(monitor, timer, snapshotProviderRegistry, + metricsPresentationConfig, metricsPacketsHandlerConfig); + testDriver = new RequestHandlerTestDriver(metricsPacketsHandler); + } + @Test - public void only_status_packet_is_returned_prior_to_first_snapshot() throws Exception { - metric.add("not_included", 1, null); + public void status_packet_is_returned_prior_to_first_snapshot() throws Exception { String response = requestAsString("http://localhost/metrics-packets"); List<JsonNode> packets = toJsonPackets(response); @@ -41,7 +54,7 @@ public class MetricsPacketsHandlerTest extends StateHandlerTestBase { @Test public void metrics_are_included_after_snapshot() throws Exception { - metric.add("counter", 1, null); + createSnapshotWithCountMetric("counter", 1, null); List<JsonNode> packets = incrementTimeAndGetJsonPackets(); assertEquals(2, packets.size()); @@ -51,7 +64,7 @@ public class MetricsPacketsHandlerTest extends StateHandlerTestBase { @Test public void metadata_is_included_in_each_metrics_packet() throws Exception { - metric.add("counter", 1, null); + createSnapshotWithCountMetric("counter", 1, null); List<JsonNode> packets = incrementTimeAndGetJsonPackets(); JsonNode counterPacket = packets.get(1); @@ -62,7 +75,7 @@ public class MetricsPacketsHandlerTest extends StateHandlerTestBase { @Test public void timestamp_resolution_is_in_seconds() throws Exception { - metric.add("counter", 1, null); + createSnapshotWithCountMetric("counter", 1, null); List<JsonNode> packets = incrementTimeAndGetJsonPackets(); JsonNode counterPacket = packets.get(1); @@ -71,8 +84,10 @@ public class MetricsPacketsHandlerTest extends StateHandlerTestBase { @Test public void expected_aggregators_are_output_for_gauge_metrics() throws Exception{ - Metric.Context context = metric.createContext(Collections.singletonMap("dim1", "value1")); - metric.set("gauge", 0.2, null); + var context = StateMetricContext.newInstance(Map.of("dim1", "value1")); + var snapshot = new MetricSnapshot(); + snapshot.set(context, "gauge", 0.2); + snapshotProvider.setSnapshot(snapshot); List<JsonNode> packets = incrementTimeAndGetJsonPackets(); JsonNode gaugeMetric = packets.get(1).get(METRICS_KEY); @@ -84,8 +99,8 @@ public class MetricsPacketsHandlerTest extends StateHandlerTestBase { @Test public void dimensions_from_context_are_included() throws Exception { - Metric.Context context = metric.createContext(Collections.singletonMap("dim1", "value1")); - metric.add("counter", 1, context); + var context = StateMetricContext.newInstance(Map.of("dim1", "value1")); + createSnapshotWithCountMetric("counter", 1, context); List<JsonNode> packets = incrementTimeAndGetJsonPackets(); JsonNode counterPacket = packets.get(1); @@ -97,9 +112,11 @@ public class MetricsPacketsHandlerTest extends StateHandlerTestBase { @Test public void metrics_with_identical_dimensions_are_contained_in_the_same_packet() throws Exception { - Metric.Context context = metric.createContext(Collections.singletonMap("dim1", "value1")); - metric.add("counter1", 1, context); - metric.add("counter2", 2, context); + var context = StateMetricContext.newInstance(Map.of("dim1", "value1")); + var snapshot = new MetricSnapshot(); + snapshot.add(context, "counter1", 1); + snapshot.add(context, "counter2", 2); + snapshotProvider.setSnapshot(snapshot); List<JsonNode> packets = incrementTimeAndGetJsonPackets(); assertEquals(2, packets.size()); @@ -112,10 +129,12 @@ public class MetricsPacketsHandlerTest extends StateHandlerTestBase { @Test public void metrics_with_different_dimensions_get_separate_packets() throws Exception { - Metric.Context context1 = metric.createContext(Collections.singletonMap("dim1", "value1")); - Metric.Context context2 = metric.createContext(Collections.singletonMap("dim2", "value2")); - metric.add("counter1", 1, context1); - metric.add("counter2", 1, context2); + var context1 = StateMetricContext.newInstance(Map.of("dim1", "value1")); + var context2 = StateMetricContext.newInstance(Map.of("dim2", "value2")); + var snapshot = new MetricSnapshot(); + snapshot.add(context1, "counter1", 1); + snapshot.add(context2, "counter2", 2); + snapshotProvider.setSnapshot(snapshot); List<JsonNode> packets = incrementTimeAndGetJsonPackets(); assertEquals(3, packets.size()); @@ -145,4 +164,10 @@ public class MetricsPacketsHandlerTest extends StateHandlerTestBase { assertEquals(expected, counterMetrics.get(metricName).asLong()); } + private void createSnapshotWithCountMetric(String name, Number value, MetricDimensions context) { + var snapshot = new MetricSnapshot(); + snapshot.add(context, name, value); + snapshotProvider.setSnapshot(snapshot); + } + } diff --git a/container-core/src/test/java/com/yahoo/container/jdisc/state/MockSnapshotProvider.java b/container-core/src/test/java/com/yahoo/container/jdisc/state/MockSnapshotProvider.java new file mode 100644 index 00000000000..5cc65be855d --- /dev/null +++ b/container-core/src/test/java/com/yahoo/container/jdisc/state/MockSnapshotProvider.java @@ -0,0 +1,23 @@ +package com.yahoo.container.jdisc.state; + +import java.io.PrintStream; + +/** + * @author gjoranv + */ +public class MockSnapshotProvider implements SnapshotProvider { + + private MetricSnapshot snapshot; + + public void setSnapshot(MetricSnapshot snapshot) { + this.snapshot = snapshot; + } + + @Override + public MetricSnapshot latestSnapshot() { + return snapshot; + } + + @Override + public void histogram(PrintStream output) { } +} diff --git a/container-core/src/test/java/com/yahoo/container/jdisc/state/StateHandlerTest.java b/container-core/src/test/java/com/yahoo/container/jdisc/state/StateHandlerTest.java index 26a2f817acc..1258ecdc46f 100644 --- a/container-core/src/test/java/com/yahoo/container/jdisc/state/StateHandlerTest.java +++ b/container-core/src/test/java/com/yahoo/container/jdisc/state/StateHandlerTest.java @@ -3,14 +3,13 @@ package com.yahoo.container.jdisc.state; import com.fasterxml.jackson.databind.JsonNode; import com.yahoo.component.Vtag; -import com.yahoo.jdisc.Metric; +import com.yahoo.container.jdisc.RequestHandlerTestDriver; import com.yahoo.vespa.defaults.Defaults; +import org.junit.Before; import org.junit.Test; -import java.util.Collections; -import java.util.HashMap; import java.util.Map; -import java.util.TreeMap; +import java.util.concurrent.TimeUnit; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -21,98 +20,82 @@ import static org.junit.Assert.assertTrue; */ public class StateHandlerTest extends StateHandlerTestBase { + private static final String V1_URI = URI_BASE + "/state/v1/"; + private static StateHandler stateHandler; + + @Before + public void setupHandler() { + stateHandler = new StateHandler(monitor, timer, applicationMetadataConfig, snapshotProviderRegistry , metricsPresentationConfig); + testDriver = new RequestHandlerTestDriver(stateHandler); + } + @Test - public void testReportPriorToFirstSnapshot() throws Exception { - metric.add("foo", 1, null); - metric.set("bar", 4, null); - JsonNode json = requestAsJson("http://localhost/state/v1/all"); + public void testStatusReportPriorToFirstSnapshot() throws Exception { + JsonNode json = requestAsJson(V1_URI + "/all"); assertEquals(json.toString(), "up", json.get("status").get("code").asText()); assertFalse(json.toString(), json.get("metrics").has("values")); } @Test public void testReportIncludesMetricsAfterSnapshot() throws Exception { - metric.add("foo", 1, null); - metric.set("bar", 4, null); - advanceToNextSnapshot(); - JsonNode json1 = requestAsJson("http://localhost/state/v1/metrics"); + var snapshot = new MetricSnapshot(); + snapshot.add(null, "foo", 1); + snapshot.set(null, "bar", 4); + snapshotProvider.setSnapshot(snapshot); + + JsonNode json1 = requestAsJson(V1_URI + "metrics"); assertEquals(json1.toString(), "up", json1.get("status").get("code").asText()); assertEquals(json1.toString(), 2, json1.get("metrics").get("values").size()); - - metric.add("fuz", 1, metric.createContext(new HashMap<>(0))); - advanceToNextSnapshot(); - JsonNode json2 = requestAsJson("http://localhost/state/v1/metrics"); - assertEquals(json2.toString(), "up", json2.get("status").get("code").asText()); - assertEquals(json2.toString(), 3, json2.get("metrics").get("values").size()); } - /** - * Tests that we restart an metric when it changes type from gauge to counter or back. - * This may happen in practice on config reloads. - */ @Test - public void testMetricTypeChangeIsAllowed() { - String metricName = "myMetric"; - Metric.Context metricContext = null; - - { - // Add a count metric - metric.add(metricName, 1, metricContext); - metric.add(metricName, 2, metricContext); - // Change it to a gauge metric - metric.set(metricName, 9, metricContext); - advanceToNextSnapshot(); - MetricValue resultingMetric = monitor.snapshot().iterator().next().getValue().get(metricName); - assertEquals(GaugeMetric.class, resultingMetric.getClass()); - assertEquals("Value was reset and produces the last gauge value", - 9.0, ((GaugeMetric) resultingMetric).getLast(), 0.000001); - } - - { - // Add a gauge metric - metric.set(metricName, 9, metricContext); - // Change it to a count metric - metric.add(metricName, 1, metricContext); - metric.add(metricName, 2, metricContext); - advanceToNextSnapshot(); - MetricValue resultingMetric = monitor.snapshot().iterator().next().getValue().get(metricName); - assertEquals(CountMetric.class, resultingMetric.getClass()); - assertEquals("Value was reset, and changed to add semantics giving 1+2", - 3, ((CountMetric) resultingMetric).getCount()); - } - } + public void testCountMetricCount() throws Exception { + var snapshot = new MetricSnapshot(); + snapshot.add(null, "foo", 4); + snapshot.add(null, "foo", 2); + snapshotProvider.setSnapshot(snapshot); - @Test - public void testAverageAggregationOfValues() throws Exception { - metric.set("bar", 4, null); - metric.set("bar", 5, null); - metric.set("bar", 7, null); - metric.set("bar", 2, null); - advanceToNextSnapshot(); - JsonNode json = requestAsJson("http://localhost/state/v1/all"); + JsonNode json = requestAsJson(V1_URI + "all"); assertEquals(json.toString(), "up", json.get("status").get("code").asText()); assertEquals(json.toString(), 1, json.get("metrics").get("values").size()); - assertEquals(json.toString(), 4.5, - json.get("metrics").get("values").get(0).get("values").get("average").asDouble(), 0.001); + assertEquals(json.toString(), 6, + json.get("metrics").get("values").get(0).get("values").get("count").asDouble(), 0.001); } @Test - public void testSumAggregationOfCounts() throws Exception { - metric.add("foo", 1, null); - metric.add("foo", 1, null); - metric.add("foo", 2, null); - metric.add("foo", 1, null); - advanceToNextSnapshot(); - JsonNode json = requestAsJson("http://localhost/state/v1/all"); - assertEquals(json.toString(), "up", json.get("status").get("code").asText()); - assertEquals(json.toString(), 1, json.get("metrics").get("values").size()); - assertEquals(json.toString(), 5, - json.get("metrics").get("values").get(0).get("values").get("count").asDouble(), 0.001); + public void gaugeSnapshotsTracksCountMinMaxAvgPerPeriod() throws Exception { + var snapshot = new MetricSnapshot(); + snapshot.set(null, "bar", 20); + snapshot.set(null, "bar", 40); + snapshotProvider.setSnapshot(snapshot); + + JsonNode json = requestAsJson(V1_URI + "all"); + JsonNode metricValues = getFirstMetricValueNode(json); + assertEquals(json.toString(), 40, metricValues.get("last").asDouble(), 0.001); + // Last snapshot had explicit values set + assertEquals(json.toString(), 30, metricValues.get("average").asDouble(), 0.001); + assertEquals(json.toString(), 20, metricValues.get("min").asDouble(), 0.001); + assertEquals(json.toString(), 40, metricValues.get("max").asDouble(), 0.001); + assertEquals(json.toString(), 2, metricValues.get("count").asInt()); + } + + private JsonNode getFirstMetricValueNode(JsonNode root) { + assertEquals(root.toString(), 1, root.get("metrics").get("values").size()); + JsonNode metricValues = root.get("metrics").get("values").get(0).get("values"); + assertTrue(root.toString(), metricValues.has("last")); + return metricValues; } @Test - public void testReadabilityOfJsonReport() throws Exception { - metric.add("foo", 1, null); + public void testReadabilityOfJsonReport() { + var snapshot = new MetricSnapshot(0L, SNAPSHOT_INTERVAL, TimeUnit.MILLISECONDS); + snapshot.add(null, "foo", 1); + var ctx = StateMetricContext.newInstance(Map.of("component", "test")); + snapshot.set(ctx, "bar", 2); + snapshot.set(ctx, "bar", 3); + snapshot.set(ctx, "bar", 4); + snapshot.set(ctx, "bar", 5); + snapshotProvider.setSnapshot(snapshot); advanceToNextSnapshot(); assertEquals("{\n" + " \"metrics\": {\n" + @@ -120,37 +103,12 @@ public class StateHandlerTest extends StateHandlerTestBase { " \"from\": 0,\n" + " \"to\": 300\n" + " },\n" + - " \"values\": [{\n" + - " \"name\": \"foo\",\n" + - " \"values\": {\n" + - " \"count\": 1,\n" + - " \"rate\": 0.0033333333333333335\n" + - " }\n" + - " }]\n" + - " },\n" + - " \"status\": {\"code\": \"up\"},\n" + - " \"time\": 300000\n" + - "}", - requestAsString("http://localhost/state/v1/all")); - - Metric.Context ctx = metric.createContext(Collections.singletonMap("component", "test")); - metric.set("bar", 2, ctx); - metric.set("bar", 3, ctx); - metric.set("bar", 4, ctx); - metric.set("bar", 5, ctx); - advanceToNextSnapshot(); - assertEquals("{\n" + - " \"metrics\": {\n" + - " \"snapshot\": {\n" + - " \"from\": 300,\n" + - " \"to\": 600\n" + - " },\n" + " \"values\": [\n" + " {\n" + " \"name\": \"foo\",\n" + " \"values\": {\n" + - " \"count\": 0,\n" + - " \"rate\": 0\n" + + " \"count\": 1,\n" + + " \"rate\": 0.0033333333333333335\n" + " }\n" + " },\n" + " {\n" + @@ -169,131 +127,24 @@ public class StateHandlerTest extends StateHandlerTestBase { " ]\n" + " },\n" + " \"status\": {\"code\": \"up\"},\n" + - " \"time\": 600000\n" + + " \"time\": 300000\n" + "}", - requestAsString("http://localhost/state/v1/all")); - } - - @Test - public void testNotAggregatingCountsBeyondSnapshots() throws Exception { - metric.add("foo", 1, null); - metric.add("foo", 1, null); - advanceToNextSnapshot(); - metric.add("foo", 2, null); - metric.add("foo", 1, null); - advanceToNextSnapshot(); - JsonNode json = requestAsJson("http://localhost/state/v1/all"); - assertEquals(json.toString(), "up", json.get("status").get("code").asText()); - assertEquals(json.toString(), 1, json.get("metrics").get("values").size()); - assertEquals(json.toString(), 3, - json.get("metrics").get("values").get(0).get("values").get("count").asDouble(), 0.001); - } - - @Test - public void testSnapshottingTimes() throws Exception { - metric.add("foo", 1, null); - metric.set("bar", 3, null); - // At this time we should not have done any snapshotting - { - JsonNode json = requestAsJson("http://localhost/state/v1/all"); - assertFalse(json.toString(), json.get("metrics").has("snapshot")); - } - // At this time first snapshot should have been generated - advanceToNextSnapshot(); - { - JsonNode json = requestAsJson("http://localhost/state/v1/all"); - assertTrue(json.toString(), json.get("metrics").has("snapshot")); - assertEquals(0.0, json.get("metrics").get("snapshot").get("from").asDouble(), 0.00001); - assertEquals(300.0, json.get("metrics").get("snapshot").get("to").asDouble(), 0.00001); - } - // No new snapshot - { - JsonNode json = requestAsJson("http://localhost/state/v1/all"); - assertTrue(json.toString(), json.get("metrics").has("snapshot")); - assertEquals(0.0, json.get("metrics").get("snapshot").get("from").asDouble(), 0.00001); - assertEquals(300.0, json.get("metrics").get("snapshot").get("to").asDouble(), 0.00001); - } - // A new snapshot - advanceToNextSnapshot(); - { - JsonNode json = requestAsJson("http://localhost/state/v1/all"); - assertTrue(json.toString(), json.get("metrics").has("snapshot")); - assertEquals(300.0, json.get("metrics").get("snapshot").get("from").asDouble(), 0.00001); - assertEquals(600.0, json.get("metrics").get("snapshot").get("to").asDouble(), 0.00001); - } - } - - @Test - public void testFreshStartOfValuesBeyondSnapshot() throws Exception { - metric.set("bar", 4, null); - metric.set("bar", 5, null); - advanceToNextSnapshot(); - metric.set("bar", 4, null); - metric.set("bar", 2, null); - advanceToNextSnapshot(); - JsonNode json = requestAsJson("http://localhost/state/v1/all"); - assertEquals(json.toString(), "up", json.get("status").get("code").asText()); - assertEquals(json.toString(), 1, json.get("metrics").get("values").size()); - assertEquals(json.toString(), 3, - json.get("metrics").get("values").get(0).get("values").get("average").asDouble(), 0.001); - } - - @Test - public void snapshotsPreserveLastGaugeValue() throws Exception { - metric.set("bar", 4, null); - advanceToNextSnapshot(); - advanceToNextSnapshot(); - JsonNode json = requestAsJson("http://localhost/state/v1/all"); - JsonNode metricValues = getFirstMetricValueNode(json); - assertEquals(json.toString(), 4, metricValues.get("last").asDouble(), 0.001); - // Use 'last' as avg/min/max when none has been set explicitly during snapshot period - assertEquals(json.toString(), 4, metricValues.get("average").asDouble(), 0.001); - assertEquals(json.toString(), 4, metricValues.get("min").asDouble(), 0.001); - assertEquals(json.toString(), 4, metricValues.get("max").asDouble(), 0.001); - // Count is tracked per period. - assertEquals(json.toString(), 0, metricValues.get("count").asInt()); - } - - private JsonNode getFirstMetricValueNode(JsonNode root) { - assertEquals(root.toString(), 1, root.get("metrics").get("values").size()); - JsonNode metricValues = root.get("metrics").get("values").get(0).get("values"); - assertTrue(root.toString(), metricValues.has("last")); - return metricValues; - } - - @Test - public void gaugeSnapshotsTracksCountMinMaxAvgPerPeriod() throws Exception { - metric.set("bar", 10000, null); // Ensure any cross-snapshot noise is visible - advanceToNextSnapshot(); - metric.set("bar", 20, null); - metric.set("bar", 40, null); - advanceToNextSnapshot(); - JsonNode json = requestAsJson("http://localhost/state/v1/all"); - JsonNode metricValues = getFirstMetricValueNode(json); - assertEquals(json.toString(), 40, metricValues.get("last").asDouble(), 0.001); - // Last snapshot had explicit values set - assertEquals(json.toString(), 30, metricValues.get("average").asDouble(), 0.001); - assertEquals(json.toString(), 20, metricValues.get("min").asDouble(), 0.001); - assertEquals(json.toString(), 40, metricValues.get("max").asDouble(), 0.001); - assertEquals(json.toString(), 2, metricValues.get("count").asInt()); + requestAsString(V1_URI + "all")); } @Test public void testHealthAggregation() throws Exception { - Map<String, String> dimensions1 = new TreeMap<>(); - dimensions1.put("port", String.valueOf(Defaults.getDefaults().vespaWebServicePort())); - Metric.Context context1 = metric.createContext(dimensions1); - Map<String, String> dimensions2 = new TreeMap<>(); - dimensions2.put("port", "80"); - Metric.Context context2 = metric.createContext(dimensions2); - - metric.add("serverNumSuccessfulResponses", 4, context1); - metric.add("serverNumSuccessfulResponses", 2, context2); - metric.set("serverTotalSuccessfulResponseLatency", 20, context1); - metric.set("serverTotalSuccessfulResponseLatency", 40, context2); - metric.add("random", 3, context1); - advanceToNextSnapshot(); - JsonNode json = requestAsJson("http://localhost/state/v1/health"); + var context1 = StateMetricContext.newInstance(Map.of("port", Defaults.getDefaults().vespaWebServicePort())); + var context2 = StateMetricContext.newInstance(Map.of("port", 80)); + var snapshot = new MetricSnapshot(); + snapshot.add(context1, "serverNumSuccessfulResponses", 4); + snapshot.add(context2, "serverNumSuccessfulResponses", 2); + snapshot.set(context1, "serverTotalSuccessfulResponseLatency", 20); + snapshot.set(context2, "serverTotalSuccessfulResponseLatency", 40); + snapshot.add(context1, "random", 3); + snapshotProvider.setSnapshot(snapshot); + + JsonNode json = requestAsJson(V1_URI + "health"); assertEquals(json.toString(), "up", json.get("status").get("code").asText()); assertEquals(json.toString(), 2, json.get("metrics").get("values").size()); assertEquals(json.toString(), "requestsPerSecond", @@ -308,7 +159,7 @@ public class StateHandlerTest extends StateHandlerTestBase { @Test public void testStateConfig() throws Exception { - JsonNode root = requestAsJson("http://localhost/state/v1/config"); + JsonNode root = requestAsJson(V1_URI + "config"); JsonNode config = root.get("config"); JsonNode container = config.get("container"); @@ -317,7 +168,7 @@ public class StateHandlerTest extends StateHandlerTestBase { @Test public void testStateVersion() throws Exception { - JsonNode root = requestAsJson("http://localhost/state/v1/version"); + JsonNode root = requestAsJson(V1_URI + "version"); JsonNode version = root.get("version"); assertEquals(Vtag.currentVersion.toString(), version.asText()); diff --git a/container-core/src/test/java/com/yahoo/container/jdisc/state/StateHandlerTestBase.java b/container-core/src/test/java/com/yahoo/container/jdisc/state/StateHandlerTestBase.java index 57a9ba4abdb..68c1ae10ec6 100644 --- a/container-core/src/test/java/com/yahoo/container/jdisc/state/StateHandlerTestBase.java +++ b/container-core/src/test/java/com/yahoo/container/jdisc/state/StateHandlerTestBase.java @@ -3,103 +3,63 @@ package com.yahoo.container.jdisc.state; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; -import com.google.inject.AbstractModule; +import com.yahoo.component.ComponentId; +import com.yahoo.component.provider.ComponentRegistry; import com.yahoo.container.core.ApplicationMetadataConfig; +import com.yahoo.container.jdisc.RequestHandlerTestDriver; import com.yahoo.container.jdisc.config.HealthMonitorConfig; -import com.yahoo.jdisc.Metric; -import com.yahoo.jdisc.Response; import com.yahoo.jdisc.Timer; -import com.yahoo.jdisc.application.ContainerBuilder; -import com.yahoo.jdisc.application.MetricConsumer; -import com.yahoo.jdisc.handler.BufferedContentChannel; -import com.yahoo.jdisc.handler.ContentChannel; -import com.yahoo.jdisc.handler.ResponseHandler; -import com.yahoo.jdisc.test.TestDriver; import com.yahoo.metrics.MetricsPresentationConfig; -import org.junit.After; import org.junit.Before; +import org.junit.BeforeClass; -import java.io.InputStreamReader; -import java.io.Reader; -import java.nio.charset.StandardCharsets; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicLong; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertTrue; /** - * @author Simon Thoresen Hult * @author gjoranv */ public class StateHandlerTestBase { final static long SNAPSHOT_INTERVAL = TimeUnit.SECONDS.toMillis(300); final static long META_GENERATION = 69; - static final String APPLICATION_NAME = "state-handler-test-base"; - TestDriver driver; - StateMonitor monitor; - Metric metric; + + static final String URI_BASE = "http://localhost"; + + static StateMonitor monitor; + static RequestHandlerTestDriver testDriver; + + static HealthMonitorConfig healthMonitorConfig; + static ApplicationMetadataConfig applicationMetadataConfig; + static MetricsPresentationConfig metricsPresentationConfig; + static MetricsPacketsHandlerConfig metricsPacketsHandlerConfig; + final AtomicLong currentTimeMillis = new AtomicLong(0); + Timer timer; - @Before - public void startTestDriver() { - Timer timer = this.currentTimeMillis::get; - this.driver = TestDriver.newSimpleApplicationInstanceWithoutOsgi(new AbstractModule() { - @Override - protected void configure() { - bind(Timer.class).toInstance(timer); - } - }); - ContainerBuilder builder = driver.newContainerBuilder(); - HealthMonitorConfig healthMonitorConfig = - new HealthMonitorConfig( - new HealthMonitorConfig.Builder() - .snapshot_interval(TimeUnit.MILLISECONDS.toSeconds(SNAPSHOT_INTERVAL)) - .initialStatus("up")); - this.monitor = new StateMonitor(healthMonitorConfig, timer, null); - builder.guiceModules().install(new AbstractModule() { + MockSnapshotProvider snapshotProvider; + ComponentRegistry<SnapshotProvider> snapshotProviderRegistry; - @Override - protected void configure() { - bind(StateMonitor.class).toInstance(monitor); - bind(MetricConsumer.class).toProvider(MetricConsumerProviders.wrap(monitor)); - bind(ApplicationMetadataConfig.class).toInstance(new ApplicationMetadataConfig( - new ApplicationMetadataConfig.Builder().generation(META_GENERATION))); - bind(MetricsPresentationConfig.class) - .toInstance(new MetricsPresentationConfig(new MetricsPresentationConfig.Builder())); - bind(MetricsPacketsHandlerConfig.class).toInstance(new MetricsPacketsHandlerConfig( - new MetricsPacketsHandlerConfig.Builder().application(APPLICATION_NAME))); - } - }); - builder.serverBindings().bind("http://*/*", builder.getInstance(StateHandler.class)); - builder.serverBindings().bind("http://*/metrics-packets", builder.getInstance(MetricsPacketsHandler.class)); - driver.activateContainer(builder); - metric = builder.getInstance(Metric.class); + @BeforeClass + public static void setupClass() { + metricsPresentationConfig = new MetricsPresentationConfig(new MetricsPresentationConfig.Builder()); + healthMonitorConfig = new HealthMonitorConfig(new HealthMonitorConfig.Builder() + .initialStatus("up")); + applicationMetadataConfig = new ApplicationMetadataConfig(new ApplicationMetadataConfig.Builder() + .generation(META_GENERATION)); } - @After - public void stopTestDriver() { - assertTrue(driver.close()); + @Before + public void setupSnapshotProvider() { + timer = currentTimeMillis::get; + snapshotProvider = new MockSnapshotProvider(); + snapshotProviderRegistry = new ComponentRegistry<>(); + snapshotProviderRegistry.register(new ComponentId("foo"), snapshotProvider); + monitor = new StateMonitor(healthMonitorConfig); } - String requestAsString(String requestUri) throws Exception { - final BufferedContentChannel content = new BufferedContentChannel(); - Response response = driver.dispatchRequest(requestUri, new ResponseHandler() { - - @Override - public ContentChannel handleResponse(Response response) { - return content; - } - }).get(60, TimeUnit.SECONDS); - assertNotNull(response); - assertEquals(Response.Status.OK, response.getStatus()); - StringBuilder str = new StringBuilder(); - Reader in = new InputStreamReader(content.toStream(), StandardCharsets.UTF_8); - for (int c; (c = in.read()) != -1; ) { - str.append((char)c); - } - return str.toString(); + String requestAsString(String requestUri) { + return testDriver.sendRequest(requestUri).readAll(); } JsonNode requestAsJson(String requestUri) throws Exception { @@ -109,7 +69,6 @@ public class StateHandlerTestBase { void advanceToNextSnapshot() { currentTimeMillis.addAndGet(SNAPSHOT_INTERVAL); - monitor.updateSnapshot(); } } diff --git a/container-core/src/test/java/com/yahoo/container/jdisc/state/StateMonitorBenchmarkTest.java b/container-core/src/test/java/com/yahoo/container/jdisc/state/StateMonitorBenchmarkTest.java deleted file mode 100644 index 22a2fc274c7..00000000000 --- a/container-core/src/test/java/com/yahoo/container/jdisc/state/StateMonitorBenchmarkTest.java +++ /dev/null @@ -1,77 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.container.jdisc.state; - -import com.google.inject.Provider; -import com.yahoo.jdisc.Metric; -import com.yahoo.jdisc.application.ContainerThread; -import com.yahoo.jdisc.application.MetricConsumer; -import com.yahoo.jdisc.application.MetricProvider; -import org.junit.Test; - -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; -import java.util.concurrent.Callable; -import java.util.concurrent.ExecutorService; -import java.util.concurrent.Executors; -import java.util.concurrent.Future; -import java.util.concurrent.ThreadFactory; -import java.util.concurrent.TimeUnit; - -import static org.junit.Assert.assertTrue; - -/** - * @author Simon Thoresen Hult - */ -public class StateMonitorBenchmarkTest { - - private final static int NUM_THREADS = 32; - private final static int NUM_UPDATES = 1000;//0000; - - @Test - public void requireThatHealthMonitorDoesNotBlockMetricThreads() throws Exception { - StateMonitor monitor = StateMonitor.createForTesting(); - Provider<MetricConsumer> provider = MetricConsumerProviders.wrap(monitor); - performUpdates(provider, 8); - for (int i = 1; i <= NUM_THREADS; i *= 2) { - long millis = performUpdates(provider, i); - System.err.format("%2d threads, %5d millis => %9d ups\n", - i, millis, (int)((i * NUM_UPDATES) / (millis / 1000.0))); - } - monitor.deconstruct(); - } - - private long performUpdates(Provider<MetricConsumer> metricProvider, int numThreads) throws Exception { - ThreadFactory threadFactory = new ContainerThread.Factory(metricProvider); - ExecutorService executor = Executors.newFixedThreadPool(numThreads, threadFactory); - List<Callable<Boolean>> tasks = new ArrayList<>(numThreads); - for (int i = 0; i < numThreads; ++i) { - tasks.add(new UpdateTask(new MetricProvider(metricProvider).get())); - } - long before = System.nanoTime(); - List<Future<Boolean>> results = executor.invokeAll(tasks); - long after = System.nanoTime(); - for (Future<Boolean> result : results) { - assertTrue(result.get()); - } - return TimeUnit.NANOSECONDS.toMillis(after - before); - } - - public static class UpdateTask implements Callable<Boolean> { - - final Metric metric; - - UpdateTask(Metric metric) { - this.metric = metric; - } - - @Override - public Boolean call() throws Exception { - Metric.Context ctx = metric.createContext(Collections.<String, Object>emptyMap()); - for (int i = 0; i < NUM_UPDATES; ++i) { - metric.add("foo", 69L, ctx); - } - return true; - } - } -} diff --git a/container-disc/src/main/java/com/yahoo/container/jdisc/metric/MetricConsumerProvider.java b/container-disc/src/main/java/com/yahoo/container/jdisc/metric/MetricConsumerProvider.java index a300364d848..9b907639e57 100644 --- a/container-disc/src/main/java/com/yahoo/container/jdisc/metric/MetricConsumerProvider.java +++ b/container-disc/src/main/java/com/yahoo/container/jdisc/metric/MetricConsumerProvider.java @@ -4,10 +4,7 @@ package com.yahoo.container.jdisc.metric; import com.google.inject.Inject; import com.yahoo.component.provider.ComponentRegistry; import com.yahoo.container.jdisc.MetricConsumerFactory; -import com.yahoo.container.jdisc.metric.state.StateMetricConsumerFactory; -import com.yahoo.container.jdisc.state.StateMonitor; import com.yahoo.jdisc.application.MetricConsumer; -import com.yahoo.metrics.MetricsPresentationConfig; /** @@ -27,17 +24,9 @@ public class MetricConsumerProvider { private final MetricConsumerFactory[] factories; @Inject - public MetricConsumerProvider(ComponentRegistry<MetricConsumerFactory> factoryRegistry, - MetricsPresentationConfig presentationConfig, - StateMonitor stateMonitor) { - MetricConsumerFactory[] factories; - if (factoryRegistry.getComponentCount() == 0 || ! presentationConfig.slidingwindow()) { - factories = new MetricConsumerFactory[1]; - factories[0] = new StateMetricConsumerFactory(stateMonitor); - } else { - factories = new MetricConsumerFactory[factoryRegistry.getComponentCount()]; - factoryRegistry.allComponents().toArray(factories); - } + public MetricConsumerProvider(ComponentRegistry<MetricConsumerFactory> factoryRegistry) { + MetricConsumerFactory[] factories = new MetricConsumerFactory[factoryRegistry.getComponentCount()]; + factoryRegistry.allComponents().toArray(factories); this.factories = factories; } diff --git a/container-disc/src/main/java/com/yahoo/container/jdisc/metric/MetricConsumerProviderProvider.java b/container-disc/src/main/java/com/yahoo/container/jdisc/metric/MetricConsumerProviderProvider.java index a44650a153d..ca3d47bc338 100644 --- a/container-disc/src/main/java/com/yahoo/container/jdisc/metric/MetricConsumerProviderProvider.java +++ b/container-disc/src/main/java/com/yahoo/container/jdisc/metric/MetricConsumerProviderProvider.java @@ -20,10 +20,8 @@ public class MetricConsumerProviderProvider implements Provider<MetricConsumerPr private final MetricConsumerProvider provided; @Inject - public MetricConsumerProviderProvider(ComponentRegistry<MetricConsumerFactory> factoryRegistry, - MetricsPresentationConfig presentationConfig, - StateMonitor stateMonitor) { - provided = new MetricConsumerProvider(factoryRegistry, presentationConfig, stateMonitor); + public MetricConsumerProviderProvider(ComponentRegistry<MetricConsumerFactory> factoryRegistry) { + provided = new MetricConsumerProvider(factoryRegistry); } @Override diff --git a/container-disc/src/main/java/com/yahoo/container/jdisc/metric/state/StateMetricConsumerFactory.java b/container-disc/src/main/java/com/yahoo/container/jdisc/metric/state/StateMetricConsumerFactory.java deleted file mode 100644 index d1cf660f571..00000000000 --- a/container-disc/src/main/java/com/yahoo/container/jdisc/metric/state/StateMetricConsumerFactory.java +++ /dev/null @@ -1,24 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.container.jdisc.metric.state; - -import com.yahoo.container.jdisc.MetricConsumerFactory; -import com.yahoo.container.jdisc.state.StateMonitor; -import com.yahoo.jdisc.application.MetricConsumer; - -/** - * @author Simon Thoresen Hult - */ -public class StateMetricConsumerFactory implements MetricConsumerFactory { - - private final StateMonitor stateMonitor; - - public StateMetricConsumerFactory(StateMonitor stateMonitor) { - this.stateMonitor = stateMonitor; - } - - @Override - public MetricConsumer newInstance() { - return stateMonitor.newMetricConsumer(); - } - -} diff --git a/container-disc/src/test/java/com/yahoo/container/jdisc/metric/MetricConsumerProviderTest.java b/container-disc/src/test/java/com/yahoo/container/jdisc/metric/MetricConsumerProviderTest.java index 5e3a49c2dda..0acbb701612 100644 --- a/container-disc/src/test/java/com/yahoo/container/jdisc/metric/MetricConsumerProviderTest.java +++ b/container-disc/src/test/java/com/yahoo/container/jdisc/metric/MetricConsumerProviderTest.java @@ -34,10 +34,4 @@ public class MetricConsumerProviderTest { Mockito.verify(bar, Mockito.times(1)).add("foo", 6, null); } - @Test - public void requireThatDefaultConsumerFactoryIsStateMetric() { - MetricConsumer consumer = MetricConsumerProviders.newDefaultInstance().newInstance(); - assertEquals("StateMetricConsumer", consumer.getClass().getSimpleName()); - } - } diff --git a/container-disc/src/test/java/com/yahoo/container/jdisc/metric/MetricConsumerProviders.java b/container-disc/src/test/java/com/yahoo/container/jdisc/metric/MetricConsumerProviders.java index dd96414bbc7..4e576d2cd8c 100644 --- a/container-disc/src/test/java/com/yahoo/container/jdisc/metric/MetricConsumerProviders.java +++ b/container-disc/src/test/java/com/yahoo/container/jdisc/metric/MetricConsumerProviders.java @@ -22,15 +22,7 @@ class MetricConsumerProviders { } public static MetricConsumerProvider newInstance(MetricConsumerFactory... factories) { - return new MetricConsumerProvider(newComponentRegistry(factories), - new MetricsPresentationConfig(new MetricsPresentationConfig.Builder()), - StateMonitor.createForTesting()); - } - - public static MetricConsumerProvider newDefaultInstance() { - return new MetricConsumerProvider(newComponentRegistry(), - new MetricsPresentationConfig(new MetricsPresentationConfig.Builder()), - StateMonitor.createForTesting()); + return new MetricConsumerProvider(newComponentRegistry(factories)); } public static ComponentRegistry<MetricConsumerFactory> newComponentRegistry(MetricConsumerFactory... factories) { |