From ada10218a6190254cfcfa367c242ae239b592397 Mon Sep 17 00:00:00 2001 From: gjoranv Date: Fri, 17 Jan 2020 13:04:20 +0100 Subject: Propagate node to allow adding more node properties to json output --- .../ai/vespa/metricsproxy/metric/model/json/GenericJsonUtil.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'metrics-proxy/src') diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/json/GenericJsonUtil.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/json/GenericJsonUtil.java index a2125abb6a9..437d6d300ef 100644 --- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/json/GenericJsonUtil.java +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/json/GenericJsonUtil.java @@ -42,7 +42,7 @@ public class GenericJsonUtil { var genericJsonModels = new ArrayList(); metricsByNode.forEach( - (node, metrics) -> genericJsonModels.add(toGenericJsonModel(metrics, node.getName()))); + (node, metrics) -> genericJsonModels.add(toGenericJsonModel(metrics, node))); applicationModel.nodes = genericJsonModels; return applicationModel; @@ -52,7 +52,7 @@ public class GenericJsonUtil { return toGenericJsonModel(metricsPackets, null); } - public static GenericJsonModel toGenericJsonModel(List metricsPackets, String nodeName) { + public static GenericJsonModel toGenericJsonModel(List metricsPackets, Node node) { Map> packetsByService = metricsPackets.stream() .collect(Collectors.groupingBy(packet -> packet.service, LinkedHashMap::new, toList())); @@ -72,7 +72,7 @@ public class GenericJsonUtil { .get(); if (VESPA_NODE_SERVICE_ID.equals(serviceId)) { jsonModel.node = new GenericNode(genericService.timestamp, genericService.metrics); - jsonModel.name = nodeName; + jsonModel.name = node == null ? null : node.getName(); } else { genericServices.add(genericService); -- cgit v1.2.3 From 051d7d700d83f8b77ffa0efc8efd8319758d9b10 Mon Sep 17 00:00:00 2001 From: gjoranv Date: Fri, 17 Jan 2020 14:33:13 +0100 Subject: Expose hostname, and rename nodeId -> role. --- .../metricsproxy/MetricsNodesConfigGenerator.java | 4 +-- .../MetricsProxyContainerClusterTest.java | 2 +- .../vespa/metricsproxy/http/application/Node.java | 33 ++++++++++------------ .../metric/model/json/GenericJsonModel.java | 6 ++-- .../metric/model/json/GenericJsonUtil.java | 3 +- .../resources/configdefinitions/metrics-nodes.def | 2 +- .../application/ApplicationMetricsHandlerTest.java | 7 +++-- .../ApplicationMetricsRetrieverTest.java | 2 +- .../model/json/GenericApplicationModelTest.java | 12 ++++---- .../src/test/resources/generic-application.json | 4 +-- 10 files changed, 35 insertions(+), 40 deletions(-) (limited to 'metrics-proxy/src') diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsNodesConfigGenerator.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsNodesConfigGenerator.java index 0db48cb5e27..9a8747be09c 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsNodesConfigGenerator.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsNodesConfigGenerator.java @@ -22,7 +22,7 @@ public class MetricsNodesConfigGenerator { private static MetricsNodesConfig.Node.Builder toNodeBuilder(MetricsProxyContainer container) { var builder = new MetricsNodesConfig.Node.Builder() - .nodeId(container.getHost().getConfigId()) + .role(container.getHost().getConfigId()) .hostname(container.getHostName()) .metricsPort(MetricsProxyContainer.BASEPORT) .metricsPath(MetricsHandler.VALUES_PATH); @@ -30,7 +30,7 @@ public class MetricsNodesConfigGenerator { if (container.isHostedVespa) container.getHostResource().spec().membership() .map(ClusterMembership::stringValue) - .ifPresent(builder::nodeId); + .ifPresent(builder::role); return builder; } diff --git a/config-model/src/test/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerClusterTest.java b/config-model/src/test/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerClusterTest.java index bf0b2087fbc..932995414cc 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerClusterTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerClusterTest.java @@ -307,7 +307,7 @@ public class MetricsProxyContainerClusterTest { } private void assertNodeConfig(MetricsNodesConfig.Node node) { - assertTrue(node.nodeId().startsWith("container/foo/0/")); + assertTrue(node.role().startsWith("container/foo/0/")); assertTrue(node.hostname().startsWith("node-1-3-9-")); assertEquals(MetricsProxyContainer.BASEPORT, node.metricsPort()); assertEquals(MetricsHandler.VALUES_PATH, node.metricsPath()); diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/application/Node.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/application/Node.java index 07070027f96..c8a8e65be5d 100644 --- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/application/Node.java +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/application/Node.java @@ -13,31 +13,27 @@ import java.util.Objects; */ public class Node { - final String nodeId; - final String host; - final int port; - final String path; + public final String role; + public final String hostname; + private final int port; + private final String path; private final String metricsUriBase; public Node(MetricsNodesConfig.Node nodeConfig) { - this(nodeConfig.nodeId(), nodeConfig.hostname(), nodeConfig.metricsPort() , nodeConfig.metricsPath()); + this(nodeConfig.role(), nodeConfig.hostname(), nodeConfig.metricsPort() , nodeConfig.metricsPath()); } - public Node(String nodeId, String host, int port, String path) { - Objects.requireNonNull(nodeId, "Null configId is not allowed"); - Objects.requireNonNull(host, "Null host is not allowed"); + public Node(String role, String hostname, int port, String path) { + Objects.requireNonNull(role, "Null configId is not allowed"); + Objects.requireNonNull(hostname, "Null hostname is not allowed"); Objects.requireNonNull(path, "Null path is not allowed"); - this.nodeId = nodeId; - this.host = host; + this.role = role; + this.hostname = hostname; this.port = port; this.path = path; - metricsUriBase = "http://" + host + ":" + port + path; - } - - public String getName() { - return nodeId; + metricsUriBase = "http://" + hostname + ":" + port + path; } URI metricsUri(ConsumerId consumer) { @@ -50,12 +46,13 @@ public class Node { if (o == null || getClass() != o.getClass()) return false; Node node = (Node) o; return port == node.port && - nodeId.equals(node.nodeId) && - host.equals(node.host); + path.equals(node.path) && + role.equals(node.role) && + hostname.equals(node.hostname); } @Override public int hashCode() { - return Objects.hash(nodeId, host, port); + return Objects.hash(role, hostname, port, path); } } diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/json/GenericJsonModel.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/json/GenericJsonModel.java index bd17a238607..83a69a38a87 100644 --- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/json/GenericJsonModel.java +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/json/GenericJsonModel.java @@ -19,11 +19,11 @@ import static com.fasterxml.jackson.annotation.JsonInclude.Include.NON_ABSENT; */ @JsonIgnoreProperties(ignoreUnknown = true) @JsonInclude(NON_ABSENT) -@JsonPropertyOrder({ "name", "node", "services" }) +@JsonPropertyOrder({ "hostname", "node", "services" }) public class GenericJsonModel { - @JsonProperty("name") - public String name; + @JsonProperty("hostname") + public String hostname; @JsonProperty("node") public GenericNode node; diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/json/GenericJsonUtil.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/json/GenericJsonUtil.java index 437d6d300ef..8ff86d8a821 100644 --- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/json/GenericJsonUtil.java +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/json/GenericJsonUtil.java @@ -12,7 +12,6 @@ import java.util.ArrayList; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; -import java.util.Optional; import java.util.logging.Logger; import java.util.stream.Collectors; @@ -72,7 +71,7 @@ public class GenericJsonUtil { .get(); if (VESPA_NODE_SERVICE_ID.equals(serviceId)) { jsonModel.node = new GenericNode(genericService.timestamp, genericService.metrics); - jsonModel.name = node == null ? null : node.getName(); + jsonModel.hostname = node == null ? null : node.hostname; } else { genericServices.add(genericService); diff --git a/metrics-proxy/src/main/resources/configdefinitions/metrics-nodes.def b/metrics-proxy/src/main/resources/configdefinitions/metrics-nodes.def index 0de2f21d972..4feceb338b3 100644 --- a/metrics-proxy/src/main/resources/configdefinitions/metrics-nodes.def +++ b/metrics-proxy/src/main/resources/configdefinitions/metrics-nodes.def @@ -1,7 +1,7 @@ # Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package=ai.vespa.metricsproxy.http.application -node[].nodeId string node[].hostname string +node[].role string node[].metricsPort int node[].metricsPath string diff --git a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/http/application/ApplicationMetricsHandlerTest.java b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/http/application/ApplicationMetricsHandlerTest.java index d1224e79e45..959b73d5682 100644 --- a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/http/application/ApplicationMetricsHandlerTest.java +++ b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/http/application/ApplicationMetricsHandlerTest.java @@ -43,7 +43,8 @@ import static org.junit.Assert.fail; @SuppressWarnings("UnstableApiUsage") public class ApplicationMetricsHandlerTest { - private static final String URI_BASE = "http://localhost"; + private static final String HOST = "localhost"; + private static final String URI_BASE = "http://" + HOST; private static final String APP_METRICS_V1_URI = URI_BASE + V1_PATH; private static final String APP_METRICS_VALUES_URI = URI_BASE + VALUES_PATH; @@ -117,7 +118,7 @@ public class ApplicationMetricsHandlerTest { assertEquals(1, jsonModel.nodes.size()); GenericJsonModel nodeModel = jsonModel.nodes.get(0); - assertEquals(MOCK_METRICS_PATH, nodeModel.name); + assertEquals(HOST, nodeModel.hostname); assertEquals(2, nodeModel.node.metrics.size()); assertEquals(16.222, nodeModel.node.metrics.get(0).values.get(CPU_METRIC), 0.0001d); } @@ -171,7 +172,7 @@ public class ApplicationMetricsHandlerTest { private MetricsNodesConfig.Node.Builder nodeConfig(String path) { return new MetricsNodesConfig.Node.Builder() - .nodeId(path) + .role(path) .hostname("localhost") .metricsPath(path) .metricsPort(port); diff --git a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/http/application/ApplicationMetricsRetrieverTest.java b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/http/application/ApplicationMetricsRetrieverTest.java index 5ff6b580988..1f2852e3526 100644 --- a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/http/application/ApplicationMetricsRetrieverTest.java +++ b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/http/application/ApplicationMetricsRetrieverTest.java @@ -157,7 +157,7 @@ public class ApplicationMetricsRetrieverTest { private MetricsNodesConfig.Node.Builder nodeConfig(String path) { return new MetricsNodesConfig.Node.Builder() - .nodeId(path) + .role(path) .hostname(HOST) .metricsPath(path) .metricsPort(port); diff --git a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/metric/model/json/GenericApplicationModelTest.java b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/metric/model/json/GenericApplicationModelTest.java index c0abc3efb86..73b6c014b92 100644 --- a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/metric/model/json/GenericApplicationModelTest.java +++ b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/metric/model/json/GenericApplicationModelTest.java @@ -33,7 +33,7 @@ public class GenericApplicationModelTest { // Do some sanity checking assertEquals(2, model.nodes.size()); GenericJsonModel node0Model = model.nodes.get(0); - assertEquals("node0", node0Model.name); + assertEquals("node0", node0Model.hostname); assertEquals(1, node0Model.services.size()); GenericService service = node0Model.services.get(0); assertEquals(1, service.metrics.size()); @@ -41,7 +41,7 @@ public class GenericApplicationModelTest { GenericJsonModel node1Model = model.nodes.get(1); GenericNode node1 = node1Model.node; - assertEquals("node1", node1Model.name); + assertEquals("node1", node1Model.hostname); assertEquals(32.444, node1.metrics.get(0).values.get("cpu.util"), 0.001d); assertThatSerializedModelEqualsTestFile(model); @@ -63,13 +63,14 @@ public class GenericApplicationModelTest { .build(); - var metricsByNode = Map.of(toNode("node0"), List.of(nodePacket, servicePacket)); + var metricsByNode = Map.of(new Node("my-role", "hostname", 0, "path"), + List.of(nodePacket, servicePacket)); GenericApplicationModel model = GenericJsonUtil.toGenericApplicationModel(metricsByNode); GenericJsonModel nodeModel = model.nodes.get(0); assertNotNull(nodeModel.node); - assertEquals("node0", nodeModel.name); + assertEquals("hostname", nodeModel.hostname); assertEquals(1, nodeModel.node.metrics.size()); GenericMetrics nodeMetrics = nodeModel.node.metrics.get(0); assertEquals(1.234, nodeMetrics.values.get("node-metric"), 0.001d); @@ -112,7 +113,4 @@ public class GenericApplicationModelTest { return mapper.readValue(getFileContents(TEST_FILE), GenericApplicationModel.class); } - private static Node toNode(String name) { - return new Node(name, "host", 0, "path"); - } } diff --git a/metrics-proxy/src/test/resources/generic-application.json b/metrics-proxy/src/test/resources/generic-application.json index 5ddd11962be..377293a1d3b 100644 --- a/metrics-proxy/src/test/resources/generic-application.json +++ b/metrics-proxy/src/test/resources/generic-application.json @@ -1,7 +1,7 @@ { "nodes": [ { - "name": "node0", + "hostname": "node0", "node": { "timestamp": 1234, "metrics": [ @@ -36,7 +36,7 @@ ] }, { - "name": "node1", + "hostname": "node1", "node": { "timestamp": 1234, "metrics": [ -- cgit v1.2.3 From d6f1aad857954ab8d4b65b630b12f03ac53f963b Mon Sep 17 00:00:00 2001 From: gjoranv Date: Fri, 17 Jan 2020 15:31:37 +0100 Subject: Add node's role to json output. --- .../ai/vespa/metricsproxy/metric/model/json/GenericJsonModel.java | 5 ++++- .../ai/vespa/metricsproxy/metric/model/json/GenericJsonUtil.java | 6 +++++- .../http/application/ApplicationMetricsHandlerTest.java | 1 + .../metricsproxy/metric/model/json/GenericApplicationModelTest.java | 3 +++ metrics-proxy/src/test/resources/generic-application.json | 2 ++ 5 files changed, 15 insertions(+), 2 deletions(-) (limited to 'metrics-proxy/src') diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/json/GenericJsonModel.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/json/GenericJsonModel.java index 83a69a38a87..b7abb2c0349 100644 --- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/json/GenericJsonModel.java +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/json/GenericJsonModel.java @@ -19,12 +19,15 @@ import static com.fasterxml.jackson.annotation.JsonInclude.Include.NON_ABSENT; */ @JsonIgnoreProperties(ignoreUnknown = true) @JsonInclude(NON_ABSENT) -@JsonPropertyOrder({ "hostname", "node", "services" }) +@JsonPropertyOrder({ "hostname", "role", "node", "services" }) public class GenericJsonModel { @JsonProperty("hostname") public String hostname; + @JsonProperty("role") + public String role; + @JsonProperty("node") public GenericNode node; diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/json/GenericJsonUtil.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/json/GenericJsonUtil.java index 8ff86d8a821..e249338c318 100644 --- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/json/GenericJsonUtil.java +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/json/GenericJsonUtil.java @@ -56,6 +56,11 @@ public class GenericJsonUtil { .collect(Collectors.groupingBy(packet -> packet.service, LinkedHashMap::new, toList())); var jsonModel = new GenericJsonModel(); + if (node != null) { + jsonModel.hostname = node.hostname; + jsonModel.role = node.role; + } + var genericServices = new ArrayList(); packetsByService.forEach((serviceId, packets) -> { var genericMetricsList = packets.stream() @@ -71,7 +76,6 @@ public class GenericJsonUtil { .get(); if (VESPA_NODE_SERVICE_ID.equals(serviceId)) { jsonModel.node = new GenericNode(genericService.timestamp, genericService.metrics); - jsonModel.hostname = node == null ? null : node.hostname; } else { genericServices.add(genericService); diff --git a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/http/application/ApplicationMetricsHandlerTest.java b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/http/application/ApplicationMetricsHandlerTest.java index 959b73d5682..074b7877430 100644 --- a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/http/application/ApplicationMetricsHandlerTest.java +++ b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/http/application/ApplicationMetricsHandlerTest.java @@ -119,6 +119,7 @@ public class ApplicationMetricsHandlerTest { assertEquals(1, jsonModel.nodes.size()); GenericJsonModel nodeModel = jsonModel.nodes.get(0); assertEquals(HOST, nodeModel.hostname); + assertEquals(MOCK_METRICS_PATH, nodeModel.role); assertEquals(2, nodeModel.node.metrics.size()); assertEquals(16.222, nodeModel.node.metrics.get(0).values.get(CPU_METRIC), 0.0001d); } diff --git a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/metric/model/json/GenericApplicationModelTest.java b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/metric/model/json/GenericApplicationModelTest.java index 73b6c014b92..5a2d374af2e 100644 --- a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/metric/model/json/GenericApplicationModelTest.java +++ b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/metric/model/json/GenericApplicationModelTest.java @@ -34,6 +34,7 @@ public class GenericApplicationModelTest { assertEquals(2, model.nodes.size()); GenericJsonModel node0Model = model.nodes.get(0); assertEquals("node0", node0Model.hostname); + assertEquals("role0", node0Model.role); assertEquals(1, node0Model.services.size()); GenericService service = node0Model.services.get(0); assertEquals(1, service.metrics.size()); @@ -42,6 +43,7 @@ public class GenericApplicationModelTest { GenericJsonModel node1Model = model.nodes.get(1); GenericNode node1 = node1Model.node; assertEquals("node1", node1Model.hostname); + assertEquals("role1", node1Model.role); assertEquals(32.444, node1.metrics.get(0).values.get("cpu.util"), 0.001d); assertThatSerializedModelEqualsTestFile(model); @@ -71,6 +73,7 @@ public class GenericApplicationModelTest { GenericJsonModel nodeModel = model.nodes.get(0); assertNotNull(nodeModel.node); assertEquals("hostname", nodeModel.hostname); + assertEquals("my-role", nodeModel.role); assertEquals(1, nodeModel.node.metrics.size()); GenericMetrics nodeMetrics = nodeModel.node.metrics.get(0); assertEquals(1.234, nodeMetrics.values.get("node-metric"), 0.001d); diff --git a/metrics-proxy/src/test/resources/generic-application.json b/metrics-proxy/src/test/resources/generic-application.json index 377293a1d3b..ff529a240db 100644 --- a/metrics-proxy/src/test/resources/generic-application.json +++ b/metrics-proxy/src/test/resources/generic-application.json @@ -2,6 +2,7 @@ "nodes": [ { "hostname": "node0", + "role": "role0", "node": { "timestamp": 1234, "metrics": [ @@ -37,6 +38,7 @@ }, { "hostname": "node1", + "role": "role1", "node": { "timestamp": 1234, "metrics": [ -- cgit v1.2.3 From 2b80ddb639d6740099107896548e24f694539ef0 Mon Sep 17 00:00:00 2001 From: gjoranv Date: Mon, 20 Jan 2020 11:18:00 +0100 Subject: Add back nodeId to config def - Add a default value because the model does not produce the value anymore. - This should not be necessary. The field can safely be removed because the config model of older versions always adds a value for this field. --- metrics-proxy/src/main/resources/configdefinitions/metrics-nodes.def | 3 +++ 1 file changed, 3 insertions(+) (limited to 'metrics-proxy/src') diff --git a/metrics-proxy/src/main/resources/configdefinitions/metrics-nodes.def b/metrics-proxy/src/main/resources/configdefinitions/metrics-nodes.def index 4feceb338b3..ca06148638f 100644 --- a/metrics-proxy/src/main/resources/configdefinitions/metrics-nodes.def +++ b/metrics-proxy/src/main/resources/configdefinitions/metrics-nodes.def @@ -1,6 +1,9 @@ # Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package=ai.vespa.metricsproxy.http.application +# TODO: remove, unused +node[].nodeId string default="" + node[].hostname string node[].role string node[].metricsPort int -- cgit v1.2.3