diff options
author | gjoranv <gv@verizonmedia.com> | 2020-01-17 17:12:18 +0100 |
---|---|---|
committer | gjoranv <gv@verizonmedia.com> | 2020-01-22 10:56:22 +0100 |
commit | a90738cffb70b958809090220fc291813a7a1182 (patch) | |
tree | 089e3d54783367915a7a78df2f0420923da6d2f1 | |
parent | 9ba0df95ef5e96c38ba53aff3b465ff2238948f8 (diff) |
Implement metrics dimensions processing.
- retain only public dimensions
- clusterId
- serviceId
+ Move constants for public dimensions from config-model to new
class in metrics-proxy
17 files changed, 473 insertions, 37 deletions
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainer.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainer.java index 08f782a0c54..fd924eb2a0f 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainer.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainer.java @@ -4,6 +4,7 @@ package com.yahoo.vespa.model.admin.metricsproxy; import ai.vespa.metricsproxy.metric.dimensions.NodeDimensions; import ai.vespa.metricsproxy.metric.dimensions.NodeDimensionsConfig; +import ai.vespa.metricsproxy.metric.dimensions.PublicDimensions; import ai.vespa.metricsproxy.rpc.RpcConnector; import ai.vespa.metricsproxy.rpc.RpcConnectorConfig; import ai.vespa.metricsproxy.service.VespaServices; @@ -18,8 +19,6 @@ import java.util.LinkedHashMap; import java.util.Map; import static com.yahoo.config.model.api.container.ContainerServiceType.METRICS_PROXY_CONTAINER; -import static com.yahoo.vespa.model.admin.metricsproxy.MetricsProxyContainer.NodeDimensionNames.CLUSTER_ID; -import static com.yahoo.vespa.model.admin.metricsproxy.MetricsProxyContainer.NodeDimensionNames.CLUSTER_TYPE; import static com.yahoo.vespa.model.admin.metricsproxy.MetricsProxyContainerCluster.METRICS_PROXY_BUNDLE_NAME; /** @@ -33,11 +32,6 @@ public class MetricsProxyContainer extends Container implements VespaServicesConfig.Producer { - static final class NodeDimensionNames { - static final String CLUSTER_TYPE = "clustertype"; - static final String CLUSTER_ID = "clusterid"; - } - final boolean isHostedVespa; public MetricsProxyContainer(AbstractConfigProducer parent, String hostname, int index, boolean isHostedVespa) { @@ -119,8 +113,8 @@ public class MetricsProxyContainer extends Container implements Map<String, String> dimensions = new LinkedHashMap<>(); if (isHostedVespa) { getHostResource().spec().membership().map(ClusterMembership::cluster).ifPresent(cluster -> { - dimensions.put(CLUSTER_TYPE, cluster.type().name()); - dimensions.put(CLUSTER_ID, cluster.id().value()); + dimensions.put(PublicDimensions.INTERNAL_CLUSTER_TYPE, cluster.type().name()); + dimensions.put(PublicDimensions.INTERNAL_CLUSTER_ID, cluster.id().value()); }); builder.dimensions(dimensions); diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerCluster.java index d7d178f13fc..fcc6c3279de 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerCluster.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerCluster.java @@ -16,6 +16,7 @@ import ai.vespa.metricsproxy.http.prometheus.PrometheusHandler; import ai.vespa.metricsproxy.metric.ExternalMetrics; import ai.vespa.metricsproxy.metric.dimensions.ApplicationDimensions; import ai.vespa.metricsproxy.metric.dimensions.ApplicationDimensionsConfig; +import ai.vespa.metricsproxy.metric.dimensions.PublicDimensions; import ai.vespa.metricsproxy.rpc.RpcServer; import ai.vespa.metricsproxy.service.ConfigSentinelClient; import ai.vespa.metricsproxy.service.SystemPollerProvider; @@ -47,12 +48,10 @@ import static com.yahoo.vespa.model.admin.metricsproxy.ConsumersConfigGenerator. import static com.yahoo.vespa.model.admin.metricsproxy.ConsumersConfigGenerator.generateConsumers; import static com.yahoo.vespa.model.admin.metricsproxy.ConsumersConfigGenerator.toConsumerBuilder; import static com.yahoo.vespa.model.admin.metricsproxy.MetricsProxyContainerCluster.AppDimensionNames.APPLICATION; -import static com.yahoo.vespa.model.admin.metricsproxy.MetricsProxyContainerCluster.AppDimensionNames.APPLICATION_ID; import static com.yahoo.vespa.model.admin.metricsproxy.MetricsProxyContainerCluster.AppDimensionNames.INSTANCE; import static com.yahoo.vespa.model.admin.metricsproxy.MetricsProxyContainerCluster.AppDimensionNames.LEGACY_APPLICATION; -import static com.yahoo.vespa.model.admin.metricsproxy.MetricsProxyContainerCluster.AppDimensionNames.TENANT; -import static com.yahoo.vespa.model.admin.metricsproxy.MetricsProxyContainerCluster.AppDimensionNames.ZONE; import static com.yahoo.vespa.model.admin.metricsproxy.MetricsProxyContainerCluster.AppDimensionNames.SYSTEM; +import static com.yahoo.vespa.model.admin.metricsproxy.MetricsProxyContainerCluster.AppDimensionNames.TENANT; import static com.yahoo.vespa.model.admin.monitoring.DefaultPublicConsumer.getDefaultPublicConsumer; import static com.yahoo.vespa.model.admin.monitoring.MetricSet.emptyMetricSet; import static com.yahoo.vespa.model.admin.monitoring.VespaMetricsConsumer.getVespaMetricsConsumer; @@ -79,8 +78,6 @@ public class MetricsProxyContainerCluster extends ContainerCluster<MetricsProxyC static final class AppDimensionNames { static final String SYSTEM = "system"; - static final String ZONE = "zone"; - static final String APPLICATION_ID = "applicationId"; // tenant.app.instance static final String TENANT = "tenantName"; static final String APPLICATION = "applicationName"; static final String INSTANCE = "instanceName"; @@ -208,8 +205,8 @@ public class MetricsProxyContainerCluster extends ContainerCluster<MetricsProxyC private Map<String, String> applicationDimensions() { Map<String, String> dimensions = new LinkedHashMap<>(); dimensions.put(SYSTEM, getZone().system().value()); - dimensions.put(ZONE, zoneString(getZone())); - dimensions.put(APPLICATION_ID, serializeWithDots(applicationId)); + dimensions.put(PublicDimensions.ZONE, zoneString(getZone())); + dimensions.put(PublicDimensions.APPLICATION_ID, serializeWithDots(applicationId)); dimensions.put(TENANT, applicationId.tenant().value()); dimensions.put(APPLICATION, applicationId.application().value()); dimensions.put(INSTANCE, applicationId.instance().value()); 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 6fe69ac5c64..de40e557265 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 @@ -12,6 +12,7 @@ import ai.vespa.metricsproxy.http.application.MetricsNodesConfig; import ai.vespa.metricsproxy.http.prometheus.PrometheusHandler; import ai.vespa.metricsproxy.http.yamas.YamasHandler; import ai.vespa.metricsproxy.metric.dimensions.ApplicationDimensionsConfig; +import ai.vespa.metricsproxy.metric.dimensions.PublicDimensions; import com.yahoo.component.ComponentSpecification; import com.yahoo.config.model.test.MockApplicationPackage; import com.yahoo.config.provision.Zone; @@ -289,11 +290,11 @@ public class MetricsProxyContainerClusterTest { ApplicationDimensionsConfig config = getApplicationDimensionsConfig(hostedModel); assertEquals(Zone.defaultZone().system().value(), config.dimensions(AppDimensionNames.SYSTEM)); - assertEquals(zoneString(Zone.defaultZone()), config.dimensions(AppDimensionNames.ZONE)); + assertEquals(zoneString(Zone.defaultZone()), config.dimensions(PublicDimensions.ZONE)); assertEquals(MY_TENANT, config.dimensions(AppDimensionNames.TENANT)); assertEquals(MY_APPLICATION, config.dimensions(AppDimensionNames.APPLICATION)); assertEquals(MY_INSTANCE, config.dimensions(AppDimensionNames.INSTANCE)); - assertEquals(MY_TENANT + "." + MY_APPLICATION + "." + MY_INSTANCE, config.dimensions(AppDimensionNames.APPLICATION_ID)); + assertEquals(MY_TENANT + "." + MY_APPLICATION + "." + MY_INSTANCE, config.dimensions(PublicDimensions.APPLICATION_ID)); assertEquals(MY_APPLICATION + "." + MY_INSTANCE, config.dimensions(AppDimensionNames.LEGACY_APPLICATION)); } diff --git a/config-model/src/test/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerTest.java b/config-model/src/test/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerTest.java index 89248d2469d..621cebd6246 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerTest.java @@ -2,10 +2,10 @@ package com.yahoo.vespa.model.admin.metricsproxy; import ai.vespa.metricsproxy.metric.dimensions.NodeDimensionsConfig; +import ai.vespa.metricsproxy.metric.dimensions.PublicDimensions; import ai.vespa.metricsproxy.rpc.RpcConnectorConfig; import ai.vespa.metricsproxy.service.VespaServicesConfig; import com.yahoo.vespa.model.VespaModel; -import com.yahoo.vespa.model.admin.metricsproxy.MetricsProxyContainer.NodeDimensionNames; import com.yahoo.vespa.model.test.VespaModelTester; import org.junit.Test; @@ -93,8 +93,8 @@ public class MetricsProxyContainerTest { String configId = CLUSTER_CONFIG_ID + "/" + hostedModel.getHosts().iterator().next().getHostname(); NodeDimensionsConfig config = getNodeDimensionsConfig(hostedModel, configId); - assertEquals("content", config.dimensions(NodeDimensionNames.CLUSTER_TYPE)); - assertEquals("my-content", config.dimensions(NodeDimensionNames.CLUSTER_ID)); + assertEquals("content", config.dimensions(PublicDimensions.INTERNAL_CLUSTER_TYPE)); + assertEquals("my-content", config.dimensions(PublicDimensions.INTERNAL_CLUSTER_ID)); } diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/core/VespaMetrics.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/core/VespaMetrics.java index 247a5ddeeef..c9d7618b9d7 100644 --- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/core/VespaMetrics.java +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/core/VespaMetrics.java @@ -24,6 +24,7 @@ import java.util.Set; import java.util.logging.Logger; import java.util.stream.Collectors; +import static ai.vespa.metricsproxy.metric.dimensions.PublicDimensions.INTERNAL_SERVICE_ID; import static ai.vespa.metricsproxy.metric.model.ConsumerId.toConsumerId; import static ai.vespa.metricsproxy.metric.model.DimensionId.toDimensionId; import static ai.vespa.metricsproxy.metric.model.ServiceId.toServiceId; @@ -39,7 +40,7 @@ public class VespaMetrics { public static final ConsumerId VESPA_CONSUMER_ID = toConsumerId("Vespa"); public static final DimensionId METRIC_TYPE_DIMENSION_ID = toDimensionId("metrictype"); - public static final DimensionId INSTANCE_DIMENSION_ID = toDimensionId("instance"); + public static final DimensionId INSTANCE_DIMENSION_ID = toDimensionId(INTERNAL_SERVICE_ID); private final MetricsConsumers metricsConsumers; diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/application/ApplicationMetricsHandler.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/application/ApplicationMetricsHandler.java index ce2e383f0d2..ff6eb4d80b3 100644 --- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/application/ApplicationMetricsHandler.java +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/application/ApplicationMetricsHandler.java @@ -8,12 +8,12 @@ import ai.vespa.metricsproxy.http.HttpHandlerBase; import ai.vespa.metricsproxy.http.JsonResponse; import ai.vespa.metricsproxy.metric.model.ConsumerId; import ai.vespa.metricsproxy.metric.model.MetricsPacket; +import ai.vespa.metricsproxy.metric.model.processing.MetricsProcessor; import com.google.inject.Inject; import com.yahoo.container.jdisc.HttpResponse; import com.yahoo.restapi.Path; import java.net.URI; -import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -23,6 +23,7 @@ import java.util.logging.Level; import static ai.vespa.metricsproxy.http.ValuesFetcher.getConsumerOrDefault; import static ai.vespa.metricsproxy.metric.model.json.GenericJsonUtil.toGenericApplicationModel; +import static ai.vespa.metricsproxy.metric.model.processing.MetricsProcessor.applyProcessors; import static com.yahoo.jdisc.Response.Status.INTERNAL_SERVER_ERROR; import static com.yahoo.jdisc.Response.Status.OK; import static java.util.stream.Collectors.toList; @@ -37,6 +38,8 @@ public class ApplicationMetricsHandler extends HttpHandlerBase { public static final String V1_PATH = "/applicationmetrics/v1"; static final String VALUES_PATH = V1_PATH + "/values"; + private static final int MAX_DIMENSIONS = 10; + private final ApplicationMetricsRetriever metricsRetriever; private final MetricsConsumers metricsConsumers; @@ -60,7 +63,10 @@ public class ApplicationMetricsHandler extends HttpHandlerBase { try { ConsumerId consumer = getConsumerOrDefault(requestedConsumer, metricsConsumers); var buildersByNode = metricsRetriever.getMetrics(consumer); - var metricsByNode = processAndBuild(buildersByNode); + var metricsByNode = processAndBuild(buildersByNode, + new ServiceIdDimensionProcessor(), + new ClusterIdDimensionProcessor(), + new PublicDimensionsProcessor(MAX_DIMENSIONS)); return new JsonResponse(OK, toGenericApplicationModel(metricsByNode).serialize()); } catch (Exception e) { @@ -69,8 +75,8 @@ public class ApplicationMetricsHandler extends HttpHandlerBase { } } - private Map<Node, List<MetricsPacket>> processAndBuild(Map<Node, List<MetricsPacket.Builder>> buildersByNode, - MetricsProcessor... processors) { + private static Map<Node, List<MetricsPacket>> processAndBuild(Map<Node, List<MetricsPacket.Builder>> buildersByNode, + MetricsProcessor... processors) { var metricsByNode = new HashMap<Node, List<MetricsPacket>>(); buildersByNode.forEach((node, builders) -> { @@ -84,14 +90,4 @@ public class ApplicationMetricsHandler extends HttpHandlerBase { return metricsByNode; } - private MetricsPacket.Builder applyProcessors(MetricsPacket.Builder builder, MetricsProcessor... processors) { - Arrays.stream(processors).forEach(processor -> processor.process(builder)); - return builder; - } - - interface MetricsProcessor { - // Processes the metrics packet builder in-place. - void process(MetricsPacket.Builder builder); - } - } diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/application/ClusterIdDimensionProcessor.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/application/ClusterIdDimensionProcessor.java new file mode 100644 index 00000000000..292c6da3de2 --- /dev/null +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/application/ClusterIdDimensionProcessor.java @@ -0,0 +1,39 @@ +package ai.vespa.metricsproxy.http.application; + +import ai.vespa.metricsproxy.metric.model.MetricsPacket; +import ai.vespa.metricsproxy.metric.model.processing.MetricsProcessor; + +import static ai.vespa.metricsproxy.metric.dimensions.PublicDimensions.CLUSTER_ID; +import static ai.vespa.metricsproxy.metric.dimensions.PublicDimensions.INTERNAL_CLUSTER_ID; +import static ai.vespa.metricsproxy.metric.dimensions.PublicDimensions.INTERNAL_CLUSTER_TYPE; +import static ai.vespa.metricsproxy.metric.model.DimensionId.toDimensionId; + +/** + * Replaces the current cluster ID dimension value with "clustertype/clusterid". + * + * @author gjoranv + */ +public class ClusterIdDimensionProcessor implements MetricsProcessor { + + @Override + public void process(MetricsPacket.Builder builder) { + String clusterType = emptyIfNull(builder.getDimensionValue(toDimensionId(INTERNAL_CLUSTER_TYPE))); + String clusterId = emptyIfNull(builder.getDimensionValue(toDimensionId(INTERNAL_CLUSTER_ID))); + + String newClusterId; + if (! clusterType.isEmpty() && ! clusterId.isEmpty()) + newClusterId = clusterType + "/" + clusterId; + else if (! clusterType.isEmpty()) + newClusterId = clusterType; + else if (! clusterId.isEmpty()) + newClusterId = clusterId; + else + return; // Both type and id were null or empty + + builder.putDimension(toDimensionId(CLUSTER_ID), newClusterId); + } + + private String emptyIfNull(String s) { + return s == null ? "" : s; + } +} 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 new file mode 100644 index 00000000000..395ec0bea4f --- /dev/null +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/application/PublicDimensionsProcessor.java @@ -0,0 +1,69 @@ +package ai.vespa.metricsproxy.http.application; + +import ai.vespa.metricsproxy.metric.dimensions.PublicDimensions; +import ai.vespa.metricsproxy.metric.model.DimensionId; +import ai.vespa.metricsproxy.metric.model.MetricsPacket; +import ai.vespa.metricsproxy.metric.model.processing.MetricsProcessor; + +import java.util.Collection; +import java.util.LinkedHashSet; +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. + * + * @author gjoranv + */ +public class PublicDimensionsProcessor implements MetricsProcessor { + + private final int maxDimensions; + private Set<DimensionId> publicDimensions = getPublicDimensions(); + + 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; + } + + @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; + } + } + + 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> toDimensionIds(Collection<String> dimensionNames) { + return dimensionNames.stream() + .map(DimensionId::toDimensionId) + .collect(Collectors.toCollection(LinkedHashSet::new)); + + } +} diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/application/ServiceIdDimensionProcessor.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/application/ServiceIdDimensionProcessor.java new file mode 100644 index 00000000000..45f211a5704 --- /dev/null +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/application/ServiceIdDimensionProcessor.java @@ -0,0 +1,24 @@ +package ai.vespa.metricsproxy.http.application; + +import ai.vespa.metricsproxy.metric.model.MetricsPacket; +import ai.vespa.metricsproxy.metric.model.processing.MetricsProcessor; + +import static ai.vespa.metricsproxy.metric.dimensions.PublicDimensions.INTERNAL_SERVICE_ID; +import static ai.vespa.metricsproxy.metric.dimensions.PublicDimensions.SERVICE_ID; +import static ai.vespa.metricsproxy.metric.model.DimensionId.toDimensionId; + +/** + * Copies the value of the internally used 'instance' dimension to the more aptly named 'serviceId'. + * + * @author gjoranv + */ +public class ServiceIdDimensionProcessor implements MetricsProcessor { + + @Override + public void process(MetricsPacket.Builder builder) { + String serviceIdValue = builder.getDimensionValue(toDimensionId(INTERNAL_SERVICE_ID)); + if (serviceIdValue != null) + builder.putDimension(toDimensionId(SERVICE_ID), serviceIdValue); + } + +} diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/dimensions/PublicDimensions.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/dimensions/PublicDimensions.java new file mode 100644 index 00000000000..465b419be09 --- /dev/null +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/dimensions/PublicDimensions.java @@ -0,0 +1,77 @@ +package ai.vespa.metricsproxy.metric.dimensions; + +import java.util.List; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +/** + * The names of all dimensions that are publicly available, in addition to some dimensions that + * are used in the process of composing these public dimensions. + * + * 'INTERNAL' in this context means non-public. + * + * @author gjoranv + */ +public final class PublicDimensions { + private PublicDimensions() { } + + public static final String APPLICATION_ID = "applicationId"; // <tenant.app.instance> + public static final String ZONE = "zone"; + + // The public CLUSTER_ID dimension value is composed from the two non-public dimensions. + // Node-specific. + public static final String INTERNAL_CLUSTER_TYPE = "clustertype"; + public static final String INTERNAL_CLUSTER_ID = "clusterid"; + public static final String CLUSTER_ID = INTERNAL_CLUSTER_ID; + + // Internal name (instance) is confusing, so renamed to 'serviceId' for public use. + // This is added by the metrics-proxy. + public static final String INTERNAL_SERVICE_ID = "instance"; + public static final String SERVICE_ID = "serviceId"; + + // From host-admin, currently (Jan 2020) only included for 'vespa.node' metrics + public static final String HOSTNAME = "host"; + + + /** Metric specific dimensions **/ + public static final String API = "api"; // feed + public static final String CHAIN = "chain"; // query + public static final String DOCUMENT_TYPE = "documenttype"; // content + public static final String ENDPOINT = "endpoint"; // query + public static final String GC_NAME = "gcName"; // container + public static final String HTTP_METHOD = "httpMethod"; // container + public static final String OPERATION = "operation"; // feed + public static final String RANK_PROFILE = "rankProfile"; // content + public static final String REASON = "reason"; // query (degraded etc.) + public static final String STATUS = "status"; // feed + + + // Dimensions that are valid (but not necessarily used) for all metrics. + public static List<String> commonDimensions = + List.of(APPLICATION_ID, + CLUSTER_ID, + HOSTNAME, + SERVICE_ID, + ZONE); + + // Dimensions that are only used for a subset of metrics. + public static List<String> metricDimensions = + List.of(API, + CHAIN, + DOCUMENT_TYPE, + ENDPOINT, + GC_NAME, + HTTP_METHOD, + OPERATION, + RANK_PROFILE, + REASON, + STATUS); + + + /** + * All public dimensions, common dimensions first, then dimensions for individual metrics + */ + public static final List<String> publicDimensions = Stream.concat(commonDimensions.stream(), metricDimensions.stream()) + .collect(Collectors.toList()); + +} diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/MetricsPacket.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/MetricsPacket.java index f9cc17f94ea..8ecf57237ef 100644 --- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/MetricsPacket.java +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/MetricsPacket.java @@ -158,6 +158,22 @@ public class MetricsPacket { return this; } + /** + * Returns a modifiable copy of the dimension IDs of this builder, usually for use with {@link #retainDimensions(Collection)}. + */ + public Set<DimensionId> getDimensionIds() { + return new LinkedHashSet<>(dimensions.keySet()); + } + + public String getDimensionValue(DimensionId id) { + return dimensions.get(id); + } + + public Builder retainDimensions(Collection<DimensionId> idsToRetain) { + dimensions.keySet().retainAll(idsToRetain); + return this; + } + public Builder addConsumers(Set<ConsumerId> extraConsumers) { if (extraConsumers != null) consumers.addAll(extraConsumers); return this; diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/processing/MetricsProcessor.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/processing/MetricsProcessor.java new file mode 100644 index 00000000000..7860825e075 --- /dev/null +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/processing/MetricsProcessor.java @@ -0,0 +1,31 @@ +package ai.vespa.metricsproxy.metric.model.processing; + +import ai.vespa.metricsproxy.metric.model.MetricsPacket; + +import java.util.Arrays; + +/** + * Interface for classes that make amendments to a metrics packet builder. + * Includes a utility method to apply a list of processors to a metrics packet. + * + * @author gjoranv + */ +public interface MetricsProcessor { + + /** + * Processes the metrics packet builder in-place. + */ + void process(MetricsPacket.Builder builder); + + + /** + * Helper method to apply a list of processors to a metrics packet builder. + * Returns the metrics packet builder (which has been processed in-place) for + * convenient use in stream processing. + */ + static MetricsPacket.Builder applyProcessors(MetricsPacket.Builder builder, MetricsProcessor... processors) { + Arrays.stream(processors).forEach(processor -> processor.process(builder)); + return builder; + } + +} 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 074b7877430..155bbf094a1 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 @@ -3,6 +3,7 @@ package ai.vespa.metricsproxy.http.application; import ai.vespa.metricsproxy.core.ConsumersConfig; import ai.vespa.metricsproxy.core.MetricsConsumers; +import ai.vespa.metricsproxy.metric.dimensions.PublicDimensions; import ai.vespa.metricsproxy.metric.model.json.GenericApplicationModel; import ai.vespa.metricsproxy.metric.model.json.GenericJsonModel; import ai.vespa.metricsproxy.metric.model.json.GenericMetrics; @@ -19,6 +20,7 @@ import org.junit.Test; import java.io.IOException; import java.util.Arrays; +import java.util.Map; import java.util.concurrent.Executors; import static ai.vespa.metricsproxy.TestUtil.getFileContents; @@ -34,6 +36,7 @@ import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.options import static com.yahoo.collections.CollectionUtil.first; import static java.util.stream.Collectors.toList; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -138,6 +141,22 @@ public class ApplicationMetricsHandlerTest { } @Test + public void metrics_processors_are_applied() { + GenericApplicationModel jsonModel = getResponseAsJsonModel(DEFAULT_PUBLIC_CONSUMER_ID.id); + + GenericService searchnode = jsonModel.nodes.get(0).services.get(0); + Map<String, String> dimensions = searchnode.metrics.get(0).dimensions; + assertEquals(6, 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("qrserver0", dimensions.get(PublicDimensions.SERVICE_ID)); + assertFalse(dimensions.containsKey("non-public")); + } + + @Test public void consumer_is_propagated_in_uri_to_retriever() { GenericApplicationModel jsonModel = getResponseAsJsonModel(CUSTOM_CONSUMER); GenericJsonModel nodeModel = jsonModel.nodes.get(0); diff --git a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/http/application/ClusterIdDimensionProcessorTest.java b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/http/application/ClusterIdDimensionProcessorTest.java new file mode 100644 index 00000000000..0f4c6a885ec --- /dev/null +++ b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/http/application/ClusterIdDimensionProcessorTest.java @@ -0,0 +1,63 @@ +package ai.vespa.metricsproxy.http.application; + +import ai.vespa.metricsproxy.metric.model.DimensionId; +import ai.vespa.metricsproxy.metric.model.MetricsPacket; +import org.junit.Test; + +import static ai.vespa.metricsproxy.metric.dimensions.PublicDimensions.CLUSTER_ID; +import static ai.vespa.metricsproxy.metric.dimensions.PublicDimensions.INTERNAL_CLUSTER_ID; +import static ai.vespa.metricsproxy.metric.dimensions.PublicDimensions.INTERNAL_CLUSTER_TYPE; +import static ai.vespa.metricsproxy.metric.model.DimensionId.toDimensionId; +import static ai.vespa.metricsproxy.metric.model.ServiceId.toServiceId; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; + +/** + * @author gjoranv + */ +public class ClusterIdDimensionProcessorTest { + private static final DimensionId NEW_ID_DIMENSION = toDimensionId(CLUSTER_ID); + + @Test + public void cluster_id_is_replaced_with_type_and_id() { + var builder = new MetricsPacket.Builder(toServiceId("foo")); + builder.putDimension(toDimensionId(INTERNAL_CLUSTER_TYPE), "type") ; + builder.putDimension(toDimensionId(INTERNAL_CLUSTER_ID), "id") ; + + assertEquals("type/id", newClusterId(builder)); + } + + @Test + public void cluster_id_is_type_when_id_is_null() { + var builder = new MetricsPacket.Builder(toServiceId("foo")); + builder.putDimension(toDimensionId(INTERNAL_CLUSTER_TYPE), "type") ; + + assertEquals(newClusterId(builder), "type"); + } + + @Test + public void cluster_id_is_id_when_type_is_null() { + var builder = new MetricsPacket.Builder(toServiceId("foo")); + builder.putDimension(toDimensionId(INTERNAL_CLUSTER_ID), "id") ; + + assertEquals(newClusterId(builder), "id"); + } + + @Test + public void cluster_id_is_not_added_when_both_type_and_id_are_null() { + var builder = new MetricsPacket.Builder(toServiceId("foo")); + + var processor = new ClusterIdDimensionProcessor(); + processor.process(builder); + + assertFalse(builder.getDimensionIds().contains(NEW_ID_DIMENSION)); + } + + private String newClusterId(MetricsPacket.Builder builder) { + var processor = new ClusterIdDimensionProcessor(); + processor.process(builder); + + return builder.getDimensionValue(NEW_ID_DIMENSION); + } + +} 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 new file mode 100644 index 00000000000..c6f47d70a1f --- /dev/null +++ b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/http/application/PublicDimensionsProcessorTest.java @@ -0,0 +1,59 @@ +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; +import static org.junit.Assert.assertTrue; + +/** + * @author gjoranv + */ +public class PublicDimensionsProcessorTest { + + @Test + public void non_public_dimensions_are_removed() { + var builder = new MetricsPacket.Builder(toServiceId("foo")) + .putDimension(toDimensionId("a"), ""); + + var processor = new PublicDimensionsProcessor(10); + processor.process(builder); + assertTrue(builder.getDimensionIds().isEmpty()); + } + + @Test + public void public_dimensions_are_retained() { + var builder = new MetricsPacket.Builder(toServiceId("foo")) + .putDimension(toDimensionId(APPLICATION_ID), "app"); + + var processor = new PublicDimensionsProcessor(10); + 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/java/ai/vespa/metricsproxy/http/application/ServiceIdDimensionProcessorTest.java b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/http/application/ServiceIdDimensionProcessorTest.java new file mode 100644 index 00000000000..3237abdbfa3 --- /dev/null +++ b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/http/application/ServiceIdDimensionProcessorTest.java @@ -0,0 +1,43 @@ +package ai.vespa.metricsproxy.http.application; + +import ai.vespa.metricsproxy.metric.model.DimensionId; +import ai.vespa.metricsproxy.metric.model.MetricsPacket; +import org.junit.Test; + +import static ai.vespa.metricsproxy.metric.dimensions.PublicDimensions.INTERNAL_SERVICE_ID; +import static ai.vespa.metricsproxy.metric.dimensions.PublicDimensions.SERVICE_ID; +import static ai.vespa.metricsproxy.metric.model.DimensionId.toDimensionId; +import static ai.vespa.metricsproxy.metric.model.ServiceId.toServiceId; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +/** + * @author gjoranv + */ +public class ServiceIdDimensionProcessorTest { + private static final DimensionId NEW_ID_DIMENSION = toDimensionId(SERVICE_ID); + + @Test + public void new_service_id_is_added_when_internal_service_id_exists() { + var builder = new MetricsPacket.Builder(toServiceId("foo")); + builder.putDimension(toDimensionId(INTERNAL_SERVICE_ID), "service"); + + var processor = new ServiceIdDimensionProcessor(); + processor.process(builder); + + assertTrue(builder.getDimensionIds().contains(NEW_ID_DIMENSION)); + assertEquals("service", builder.getDimensionValue(NEW_ID_DIMENSION)); + } + + @Test + public void new_service_id_is_not_added_when_internal_service_id_is_null() { + var builder = new MetricsPacket.Builder(toServiceId("foo")); + + var processor = new ServiceIdDimensionProcessor(); + processor.process(builder); + + assertFalse(builder.getDimensionIds().contains(NEW_ID_DIMENSION)); + } + +} diff --git a/metrics-proxy/src/test/resources/generic-sample.json b/metrics-proxy/src/test/resources/generic-sample.json index de617895f86..c9b02696e69 100644 --- a/metrics-proxy/src/test/resources/generic-sample.json +++ b/metrics-proxy/src/test/resources/generic-sample.json @@ -33,7 +33,14 @@ "queries.count": 4 }, "dimensions": { - "documentType": "music" + "applicationId": "music.default", + "clustertype": "container", + "clusterid": "default", + "instance": "qrserver0", + "zone": "us-west", + "api": "search/", + "documenttype": "music", + "non-public": "will-be-removed" } } ] |