diff options
author | Ola Aunronning <olaa@yahooinc.com> | 2023-02-03 18:40:26 +0100 |
---|---|---|
committer | Ola Aunronning <olaa@yahooinc.com> | 2023-02-03 18:40:26 +0100 |
commit | 4032e10a1ebae78f7bf4eff5ad3cc6438417bac1 (patch) | |
tree | d15a002b5a47dd95644d5c662e7d48e950060628 /metrics-proxy | |
parent | 97a9d25154b863baac946e11d95a8fa63d97a738 (diff) |
Replace dimension allowlisting with blocklist in /metrics/v2
Diffstat (limited to 'metrics-proxy')
7 files changed, 88 insertions, 68 deletions
diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/application/NodeMetricsClient.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/application/NodeMetricsClient.java index 3f93cdda61c..1d7f7000c5a 100644 --- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/application/NodeMetricsClient.java +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/application/NodeMetricsClient.java @@ -41,8 +41,6 @@ public class NodeMetricsClient { private static final Logger log = Logger.getLogger(NodeMetricsClient.class.getName()); - private static final int MAX_DIMENSIONS = 10; - final Node node; private final CloseableHttpAsyncClient httpClient; private final Clock clock; @@ -89,7 +87,7 @@ public class NodeMetricsClient { var metrics = processAndBuild(GenericJsonUtil.toMetricsPackets(respons), new ServiceIdDimensionProcessor(), new ClusterIdDimensionProcessor(), - new PublicDimensionsProcessor(MAX_DIMENSIONS)); + new PublicDimensionsProcessor()); snapshotsRetrieved.incrementAndGet(); log.log(FINE, () -> "Successfully retrieved " + metrics.size() + " metrics packets from " + metricsUri); snapshots.put(consumer, new Snapshot(Instant.now(clock), metrics)); diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/application/PublicDimensionsProcessor.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/application/PublicDimensionsProcessor.java index 50eb74e8693..639f642778e 100644 --- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/application/PublicDimensionsProcessor.java +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/application/PublicDimensionsProcessor.java @@ -1,6 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package ai.vespa.metricsproxy.http.application; +import ai.vespa.metricsproxy.metric.dimensions.BlocklistDimensions; import ai.vespa.metricsproxy.metric.dimensions.PublicDimensions; import ai.vespa.metricsproxy.metric.model.DimensionId; import ai.vespa.metricsproxy.metric.model.MetricsPacket; @@ -12,53 +13,23 @@ import java.util.Set; import java.util.stream.Collectors; /** - * Ensures that only whitelisted dimensions are retained in the given metrics packet, and that - * there are no more dimensions than the given maximum number. + * Ensures that blocklisted dimensions are removed from the metric set * * @author gjoranv */ public class PublicDimensionsProcessor implements MetricsProcessor { - private final int maxDimensions; - private Set<DimensionId> publicDimensions = getPublicDimensions(); - - public PublicDimensionsProcessor(int maxDimensions) { - int numCommonDimensions = PublicDimensions.commonDimensions.size(); - if (numCommonDimensions > maxDimensions) { - throw new IllegalArgumentException(String.format( - ("The maximum number of dimensions (%d) cannot be smaller than the number of " + - "common metrics dimensions (%d)."), maxDimensions, numCommonDimensions)); - } - this.maxDimensions = maxDimensions; - } + private final Set<DimensionId> blocklistDimensions = getBlocklistDimensions(); @Override public void process(MetricsPacket.Builder builder) { Set<DimensionId> dimensionsToRetain = builder.getDimensionIds(); - dimensionsToRetain.retainAll(publicDimensions); - - if (dimensionsToRetain.size() > maxDimensions) { - for (var metricDim : getMetricDimensions()) { - dimensionsToRetain.remove(metricDim); - if (dimensionsToRetain.size() <= maxDimensions) break; - } - } - + dimensionsToRetain.removeAll(blocklistDimensions); builder.retainDimensions(dimensionsToRetain); - - // Extra safeguard, to make sure we don't exceed the limit of some metric systems. - if (builder.getDimensionIds().size() > maxDimensions) { - throw new IllegalStateException(String.format( - "Metrics packet is only allowed to have %d dimensions, but has: %s", maxDimensions, builder.getDimensionIds())); - } - } - - static Set<DimensionId> getPublicDimensions() { - return toDimensionIds(PublicDimensions.publicDimensions); } - static Set<DimensionId> getMetricDimensions() { - return toDimensionIds(PublicDimensions.metricDimensions); + static Set<DimensionId> getBlocklistDimensions() { + return toDimensionIds(BlocklistDimensions.blocklistDimensions); } static Set<DimensionId> toDimensionIds(Collection<String> dimensionNames) { diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/metrics/MetricsV2Handler.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/metrics/MetricsV2Handler.java index 339f06c67b8..1aa7244363b 100644 --- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/metrics/MetricsV2Handler.java +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/metrics/MetricsV2Handler.java @@ -40,7 +40,6 @@ public class MetricsV2Handler extends HttpHandlerBase { public static final String V2_PATH = "/metrics/v2"; public static final String VALUES_PATH = V2_PATH + "/values"; - private static final int MAX_DIMENSIONS = 10; private final ValuesFetcher valuesFetcher; private final NodeInfoConfig nodeInfoConfig; @@ -68,7 +67,7 @@ public class MetricsV2Handler extends HttpHandlerBase { List<MetricsPacket> metrics = processAndBuild(valuesFetcher.fetchMetricsAsBuilders(consumer), new ServiceIdDimensionProcessor(), new ClusterIdDimensionProcessor(), - new PublicDimensionsProcessor(MAX_DIMENSIONS)); + new PublicDimensionsProcessor()); Node localNode = new Node(nodeInfoConfig.role(), nodeInfoConfig.hostname(), 0, ""); Map<Node, List<MetricsPacket>> metricsByNode = Map.of(localNode, metrics); diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/dimensions/BlocklistDimensions.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/dimensions/BlocklistDimensions.java new file mode 100644 index 00000000000..5e72594759a --- /dev/null +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/dimensions/BlocklistDimensions.java @@ -0,0 +1,73 @@ +package ai.vespa.metricsproxy.metric.dimensions; + +import java.util.ArrayList; +import java.util.List; + +/** + * @author olaa + */ +public class BlocklistDimensions { + + private BlocklistDimensions() {} + + /** + * Deployment related metrics - most of which are redundant + * E.g. app/applicationName/tenantName/instanceName is already included in applicationId + */ + public static final String APP = "app"; + public static final String APPLICATION_NAME = "applicationName"; + public static final String CLUSTER_NAME = "clustername"; + public static final String CLUSTER_ID = "clusterid"; + public static final String CLUSTER_TYPE = "clustertype"; + public static final String DEPLOYMENT_CLUSTER = "deploymentCluster"; + public static final String GROUP_ID = "groupId"; + public static final String INSTANCE = "instance"; + public static final String INSTANCE_NAME = "instanceName"; + public static final String TENANT_NAME = "tenantName"; + + /** + * State related dimensions - will always be the same value for a given snapshot + */ + public static final String METRIC_TYPE = "metrictype"; + public static final String ORCHESTRATOR_STATE = "orchestratorState"; + public static final String ROLE = "role"; + public static final String STATE = "state"; + public static final String SYSTEM = "system"; + public static final String VESPA_VERSION = "vespaVersion"; + + /** Metric specific dimensions **/ + public static final String ARCHITECTURE = "arch"; + public static final String AUTHZ_REQUIRED = "authz-required"; + public static final String HOME = "home"; + public static final String PORT = "port"; + public static final String SCHEME = "scheme"; + public static final String DRYRUN = "dryrun"; + public static final String VERSION = "version"; + + public static final List<String> blocklistDimensions = List.of( + APP, + APPLICATION_NAME, + CLUSTER_NAME, + CLUSTER_ID, + CLUSTER_TYPE, + DEPLOYMENT_CLUSTER, + GROUP_ID, + INSTANCE, + INSTANCE_NAME, + TENANT_NAME, + METRIC_TYPE, + ORCHESTRATOR_STATE, + ROLE, + STATE, + SYSTEM, + VESPA_VERSION, + ARCHITECTURE, + AUTHZ_REQUIRED, + HOME, + PORT, + SCHEME, + DRYRUN, + VERSION + ); + +} 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 d6b084acbb3..7cc51d7b46b 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 @@ -181,14 +181,14 @@ public class ApplicationMetricsHandlerTest { GenericService searchnode = jsonModel.nodes.get(0).services.get(0); Map<String, String> dimensions = searchnode.metrics.get(0).dimensions; - assertEquals(6, dimensions.size()); + assertEquals(7, dimensions.size()); assertEquals("music.default", dimensions.get(PublicDimensions.APPLICATION_ID)); assertEquals("container/default", dimensions.get(PublicDimensions.CLUSTER_ID)); assertEquals("us-west", dimensions.get(PublicDimensions.ZONE)); assertEquals("search/", dimensions.get(PublicDimensions.API)); assertEquals("music", dimensions.get(PublicDimensions.DOCUMENT_TYPE)); assertEquals("default0", dimensions.get(PublicDimensions.SERVICE_ID)); - assertFalse(dimensions.containsKey("non-public")); + assertFalse(dimensions.containsKey("clusterid")); } @Test diff --git a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/http/application/PublicDimensionsProcessorTest.java b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/http/application/PublicDimensionsProcessorTest.java index 68368d28fdf..e155b444580 100644 --- a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/http/application/PublicDimensionsProcessorTest.java +++ b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/http/application/PublicDimensionsProcessorTest.java @@ -4,11 +4,7 @@ package ai.vespa.metricsproxy.http.application; import ai.vespa.metricsproxy.metric.model.MetricsPacket; import org.junit.Test; -import static ai.vespa.metricsproxy.http.application.PublicDimensionsProcessor.getPublicDimensions; -import static ai.vespa.metricsproxy.http.application.PublicDimensionsProcessor.toDimensionIds; import static ai.vespa.metricsproxy.metric.dimensions.PublicDimensions.APPLICATION_ID; -import static ai.vespa.metricsproxy.metric.dimensions.PublicDimensions.commonDimensions; -import static ai.vespa.metricsproxy.metric.dimensions.PublicDimensions.publicDimensions; import static ai.vespa.metricsproxy.metric.model.DimensionId.toDimensionId; import static ai.vespa.metricsproxy.metric.model.ServiceId.toServiceId; import static org.junit.Assert.assertEquals; @@ -20,11 +16,11 @@ import static org.junit.Assert.assertTrue; public class PublicDimensionsProcessorTest { @Test - public void non_public_dimensions_are_removed() { + public void blocklisted_dimensions_are_removed() { var builder = new MetricsPacket.Builder(toServiceId("foo")) - .putDimension(toDimensionId("a"), ""); + .putDimension(toDimensionId("applicationName"), ""); - var processor = new PublicDimensionsProcessor(10); + var processor = new PublicDimensionsProcessor(); processor.process(builder); assertTrue(builder.getDimensionIds().isEmpty()); } @@ -34,27 +30,10 @@ public class PublicDimensionsProcessorTest { var builder = new MetricsPacket.Builder(toServiceId("foo")) .putDimension(toDimensionId(APPLICATION_ID), "app"); - var processor = new PublicDimensionsProcessor(10); + var processor = new PublicDimensionsProcessor(); processor.process(builder); assertEquals(1, builder.getDimensionIds().size()); assertEquals(toDimensionId(APPLICATION_ID), builder.getDimensionIds().iterator().next()); } - @Test - public void common_dimensions_have_priority_when_there_are_too_many() { - var builder = new MetricsPacket.Builder(toServiceId("foo")); - getPublicDimensions() - .forEach(dimensionId -> builder.putDimension(dimensionId, dimensionId.id)); - assertEquals(publicDimensions.size(), builder.getDimensionIds().size()); - - var processor = new PublicDimensionsProcessor(commonDimensions.size()); - processor.process(builder); - - var includedDimensions = builder.getDimensionIds(); - assertEquals(commonDimensions.size(), includedDimensions.size()); - - toDimensionIds(commonDimensions).forEach(commonDimension -> - assertTrue(includedDimensions.contains(commonDimension))); - } - } diff --git a/metrics-proxy/src/test/resources/generic-sample.json b/metrics-proxy/src/test/resources/generic-sample.json index 13302121507..b562ccb13d3 100644 --- a/metrics-proxy/src/test/resources/generic-sample.json +++ b/metrics-proxy/src/test/resources/generic-sample.json @@ -40,7 +40,7 @@ "zone": "us-west", "api": "search/", "documenttype": "music", - "non-public": "will-be-removed" + "custom-dimension": "will-not-be-removed" } } ] |