From 609c61f75c71a230637e53f2418e7f5ff3d2f5a3 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Wed, 4 Mar 2020 10:29:11 -0500 Subject: [PATCH] Formalize usage stats for analytics (backport of #52966) (#53077) This moves the usage statistics gathering from the `AnalyticsPlugin` into an `AnalyicsUsage`, removing the static state. It also checks the license level when parsing all analytics aggregations. This is how we were checking them before but we did it in an easy to forget way. This way is slightly simpler, I think. --- .../xpack/analytics/AnalyticsPlugin.java | 49 +++++++++------ .../xpack/analytics/AnalyticsUsage.java | 59 +++++++++++++++++++ .../action/TransportAnalyticsStatsAction.java | 14 ++--- ...CardinalityPipelineAggregationBuilder.java | 11 ---- .../TransportAnalyticsStatsActionTests.java | 54 +++++++++++------ .../BoxplotAggregationBuilderTests.java | 13 ++-- .../action/AnalyticsStatsAction.java | 56 ++++++++++++++---- 7 files changed, 183 insertions(+), 73 deletions(-) create mode 100644 x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/AnalyticsUsage.java diff --git a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/AnalyticsPlugin.java b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/AnalyticsPlugin.java index 9a9a333ce33..cee78007dbd 100644 --- a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/AnalyticsPlugin.java +++ b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/AnalyticsPlugin.java @@ -7,16 +7,27 @@ package org.elasticsearch.xpack.analytics; import org.elasticsearch.action.ActionRequest; import org.elasticsearch.action.ActionResponse; +import org.elasticsearch.client.Client; +import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; +import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.inject.Module; +import org.elasticsearch.common.io.stream.NamedWriteableRegistry; import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.ContextParser; +import org.elasticsearch.common.xcontent.NamedXContentRegistry; +import org.elasticsearch.env.Environment; +import org.elasticsearch.env.NodeEnvironment; import org.elasticsearch.index.mapper.Mapper; +import org.elasticsearch.license.LicenseUtils; import org.elasticsearch.license.XPackLicenseState; import org.elasticsearch.plugins.ActionPlugin; import org.elasticsearch.plugins.MapperPlugin; import org.elasticsearch.plugins.Plugin; import org.elasticsearch.plugins.SearchPlugin; +import org.elasticsearch.script.ScriptService; +import org.elasticsearch.threadpool.ThreadPool; +import org.elasticsearch.watcher.ResourceWatcherService; import org.elasticsearch.xpack.analytics.action.TransportAnalyticsStatsAction; import org.elasticsearch.xpack.analytics.boxplot.BoxplotAggregationBuilder; import org.elasticsearch.xpack.analytics.boxplot.InternalBoxplot; @@ -28,6 +39,7 @@ import org.elasticsearch.xpack.analytics.stringstats.StringStatsAggregationBuild import org.elasticsearch.xpack.analytics.topmetrics.InternalTopMetrics; import org.elasticsearch.xpack.analytics.topmetrics.TopMetricsAggregationBuilder; import org.elasticsearch.xpack.analytics.topmetrics.TopMetricsAggregatorFactory; +import org.elasticsearch.xpack.core.XPackField; import org.elasticsearch.xpack.core.XPackPlugin; import org.elasticsearch.xpack.core.analytics.action.AnalyticsStatsAction; @@ -37,15 +49,11 @@ import java.util.Collection; import java.util.Collections; import java.util.List; import java.util.Map; -import java.util.concurrent.atomic.AtomicLong; import static java.util.Collections.singletonList; public class AnalyticsPlugin extends Plugin implements SearchPlugin, ActionPlugin, MapperPlugin { - - // TODO this should probably become more structured - public static AtomicLong cumulativeCardUsage = new AtomicLong(0); - public static AtomicLong topMetricsUsage = new AtomicLong(0); + private final AnalyticsUsage usage = new AnalyticsUsage(); private final boolean transportClientMode; public AnalyticsPlugin(Settings settings) { @@ -61,7 +69,8 @@ public class AnalyticsPlugin extends Plugin implements SearchPlugin, ActionPlugi CumulativeCardinalityPipelineAggregationBuilder.NAME, CumulativeCardinalityPipelineAggregationBuilder::new, CumulativeCardinalityPipelineAggregator::new, - CumulativeCardinalityPipelineAggregationBuilder.PARSER) + usage.track(AnalyticsUsage.Item.CUMULATIVE_CARDINALITY, + checkLicense(CumulativeCardinalityPipelineAggregationBuilder.PARSER))) ); } @@ -71,16 +80,17 @@ public class AnalyticsPlugin extends Plugin implements SearchPlugin, ActionPlugi new AggregationSpec( StringStatsAggregationBuilder.NAME, StringStatsAggregationBuilder::new, - StringStatsAggregationBuilder.PARSER).addResultReader(InternalStringStats::new), + usage.track(AnalyticsUsage.Item.STRING_STATS, checkLicense(StringStatsAggregationBuilder.PARSER))) + .addResultReader(InternalStringStats::new), new AggregationSpec( BoxplotAggregationBuilder.NAME, BoxplotAggregationBuilder::new, - BoxplotAggregationBuilder.PARSER) + usage.track(AnalyticsUsage.Item.BOXPLOT, checkLicense(BoxplotAggregationBuilder.PARSER))) .addResultReader(InternalBoxplot::new), new AggregationSpec( TopMetricsAggregationBuilder.NAME, TopMetricsAggregationBuilder::new, - track(TopMetricsAggregationBuilder.PARSER, topMetricsUsage)) + usage.track(AnalyticsUsage.Item.TOP_METRICS, checkLicense(TopMetricsAggregationBuilder.PARSER))) .addResultReader(InternalTopMetrics::new) ); } @@ -113,15 +123,20 @@ public class AnalyticsPlugin extends Plugin implements SearchPlugin, ActionPlugi return Collections.singletonMap(HistogramFieldMapper.CONTENT_TYPE, new HistogramFieldMapper.TypeParser()); } - /** - * Track successful parsing. - */ - private static ContextParser track(ContextParser realParser, AtomicLong usage) { + @Override + public Collection createComponents(Client client, ClusterService clusterService, ThreadPool threadPool, + ResourceWatcherService resourceWatcherService, ScriptService scriptService, NamedXContentRegistry xContentRegistry, + Environment environment, NodeEnvironment nodeEnvironment, NamedWriteableRegistry namedWriteableRegistry, + IndexNameExpressionResolver indexNameExpressionResolver) { + return singletonList(new AnalyticsUsage()); + } + + private static ContextParser checkLicense(ContextParser realParser) { return (parser, name) -> { - T value = realParser.parse(parser, name); - // Intentionally doesn't count unless the parser returns cleanly. - usage.addAndGet(1); - return value; + if (getLicenseState().isAnalyticsAllowed() == false) { + throw LicenseUtils.newComplianceException(XPackField.ANALYTICS); + } + return realParser.parse(parser, name); }; } } diff --git a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/AnalyticsUsage.java b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/AnalyticsUsage.java new file mode 100644 index 00000000000..508966301c7 --- /dev/null +++ b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/AnalyticsUsage.java @@ -0,0 +1,59 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +package org.elasticsearch.xpack.analytics; + +import org.elasticsearch.cluster.node.DiscoveryNode; +import org.elasticsearch.common.xcontent.ContextParser; +import org.elasticsearch.xpack.core.analytics.action.AnalyticsStatsAction; + +import java.util.EnumMap; +import java.util.Map; +import java.util.concurrent.atomic.AtomicLong; + +/** + * Tracks usage of the Analytics aggregations. + */ +public class AnalyticsUsage { + /** + * Items to track. + */ + public enum Item { + BOXPLOT, + CUMULATIVE_CARDINALITY, + STRING_STATS, + TOP_METRICS; + } + + private final Map trackers = new EnumMap<>(Item.class); + + public AnalyticsUsage() { + for (Item item: Item.values()) { + trackers.put(item, new AtomicLong(0)); + } + } + + /** + * Track successful parsing. + */ + public ContextParser track(Item item, ContextParser realParser) { + AtomicLong usage = trackers.get(item); + return (parser, context) -> { + T value = realParser.parse(parser, context); + // Intentionally doesn't count unless the parser returns cleanly. + usage.incrementAndGet(); + return value; + }; + } + + public AnalyticsStatsAction.NodeResponse stats(DiscoveryNode node) { + return new AnalyticsStatsAction.NodeResponse(node, + trackers.get(Item.BOXPLOT).get(), + trackers.get(Item.CUMULATIVE_CARDINALITY).get(), + trackers.get(Item.STRING_STATS).get(), + trackers.get(Item.TOP_METRICS).get()); + } +} diff --git a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/action/TransportAnalyticsStatsAction.java b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/action/TransportAnalyticsStatsAction.java index 52833d47974..24ba37afcd0 100644 --- a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/action/TransportAnalyticsStatsAction.java +++ b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/action/TransportAnalyticsStatsAction.java @@ -13,22 +13,23 @@ import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.transport.TransportService; +import org.elasticsearch.xpack.analytics.AnalyticsUsage; import org.elasticsearch.xpack.core.analytics.action.AnalyticsStatsAction; -import org.elasticsearch.xpack.analytics.AnalyticsPlugin; import java.io.IOException; import java.util.List; public class TransportAnalyticsStatsAction extends TransportNodesAction { - + AnalyticsStatsAction.NodeRequest, AnalyticsStatsAction.NodeResponse> { + private final AnalyticsUsage usage; @Inject public TransportAnalyticsStatsAction(TransportService transportService, ClusterService clusterService, - ThreadPool threadPool, ActionFilters actionFilters) { + ThreadPool threadPool, ActionFilters actionFilters, AnalyticsUsage usage) { super(AnalyticsStatsAction.NAME, threadPool, clusterService, transportService, actionFilters, AnalyticsStatsAction.Request::new, AnalyticsStatsAction.NodeRequest::new, ThreadPool.Names.MANAGEMENT, AnalyticsStatsAction.NodeResponse.class); + this.usage = usage; } @Override @@ -50,10 +51,7 @@ public class TransportAnalyticsStatsAction extends TransportNodesAction PARSER = new ConstructingObjectParser<>(NAME, false, (args, name) -> { - if (AnalyticsPlugin.getLicenseState().isAnalyticsAllowed() == false) { - throw LicenseUtils.newComplianceException(XPackField.ANALYTICS); - } - - // Increment usage here since it is a good boundary between internal and external, and should correlate 1:1 with - // usage and not internal instantiations - AnalyticsPlugin.cumulativeCardUsage.incrementAndGet(); - return new CumulativeCardinalityPipelineAggregationBuilder(name, (String) args[0]); }); static { diff --git a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/action/TransportAnalyticsStatsActionTests.java b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/action/TransportAnalyticsStatsActionTests.java index 3b85e6ccc68..a13fbaa2270 100644 --- a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/action/TransportAnalyticsStatsActionTests.java +++ b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/action/TransportAnalyticsStatsActionTests.java @@ -13,6 +13,7 @@ import org.elasticsearch.cluster.metadata.MetaData; import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.bytes.BytesReference; +import org.elasticsearch.common.xcontent.ContextParser; import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.json.JsonXContent; @@ -20,58 +21,73 @@ import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.rest.yaml.ObjectPath; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.transport.TransportService; +import org.elasticsearch.xpack.analytics.AnalyticsUsage; import org.elasticsearch.xpack.core.analytics.action.AnalyticsStatsAction; -import org.junit.Before; +import java.io.IOException; import java.util.Arrays; import java.util.Collections; +import java.util.List; +import java.util.Locale; +import static java.util.Collections.emptyList; +import static java.util.stream.Collectors.toList; import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; import static org.hamcrest.Matchers.equalTo; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; public class TransportAnalyticsStatsActionTests extends ESTestCase { - - private TransportAnalyticsStatsAction action; - - @Before - public void setupTransportAction() { + public TransportAnalyticsStatsAction action(AnalyticsUsage usage) { TransportService transportService = mock(TransportService.class); ThreadPool threadPool = mock(ThreadPool.class); ClusterService clusterService = mock(ClusterService.class); DiscoveryNode discoveryNode = new DiscoveryNode("nodeId", buildNewFakeTransportAddress(), Version.CURRENT); when(clusterService.localNode()).thenReturn(discoveryNode); - ClusterName clusterName = new ClusterName("cluster_name"); when(clusterService.getClusterName()).thenReturn(clusterName); + ClusterState clusterState = mock(ClusterState.class); when(clusterState.getMetaData()).thenReturn(MetaData.EMPTY_META_DATA); when(clusterService.state()).thenReturn(clusterState); - - action = new TransportAnalyticsStatsAction(transportService, clusterService, threadPool, new - ActionFilters(Collections.emptySet())); + return new TransportAnalyticsStatsAction(transportService, clusterService, threadPool, + new ActionFilters(Collections.emptySet()), usage); } - public void testCumulativeCardStats() throws Exception { - AnalyticsStatsAction.Request request = new AnalyticsStatsAction.Request(); - AnalyticsStatsAction.NodeResponse nodeResponse1 = action.nodeOperation(new AnalyticsStatsAction.NodeRequest(request)); - AnalyticsStatsAction.NodeResponse nodeResponse2 = action.nodeOperation(new AnalyticsStatsAction.NodeRequest(request)); + public void test() throws IOException { + AnalyticsUsage.Item item = randomFrom(AnalyticsUsage.Item.values()); + AnalyticsUsage realUsage = new AnalyticsUsage(); + AnalyticsUsage emptyUsage = new AnalyticsUsage(); + ContextParser parser = realUsage.track(item, (p, c) -> c); + ObjectPath unused = run(realUsage, emptyUsage); + assertThat(unused.evaluate("stats.0." + item.name().toLowerCase(Locale.ROOT) + "_usage"), equalTo(0)); + assertThat(unused.evaluate("stats.1." + item.name().toLowerCase(Locale.ROOT) + "_usage"), equalTo(0)); + int count = between(1, 10000); + for (int i = 0; i < count; i++) { + assertNull(parser.parse(null, null)); + } + ObjectPath used = run(realUsage, emptyUsage); + assertThat(used.evaluate("stats.0." + item.name().toLowerCase(Locale.ROOT) + "_usage"), equalTo(count)); + assertThat(used.evaluate("stats.1." + item.name().toLowerCase(Locale.ROOT) + "_usage"), equalTo(0)); + } - AnalyticsStatsAction.Response response = action.newResponse(request, - Arrays.asList(nodeResponse1, nodeResponse2), Collections.emptyList()); + private ObjectPath run(AnalyticsUsage... nodeUsages) throws IOException { + AnalyticsStatsAction.Request request = new AnalyticsStatsAction.Request(); + List nodeResponses = Arrays.stream(nodeUsages) + .map(usage -> action(usage).nodeOperation(new AnalyticsStatsAction.NodeRequest(request))) + .collect(toList()); + AnalyticsStatsAction.Response response = new AnalyticsStatsAction.Response( + new ClusterName("cluster_name"), nodeResponses, emptyList()); try (XContentBuilder builder = jsonBuilder()) { builder.startObject(); response.toXContent(builder, ToXContent.EMPTY_PARAMS); builder.endObject(); - ObjectPath objectPath = ObjectPath.createFromXContent(JsonXContent.jsonXContent, BytesReference.bytes(builder)); - assertThat(objectPath.evaluate("stats.0.cumulative_cardinality_usage"), equalTo(0)); - assertThat(objectPath.evaluate("stats.1.cumulative_cardinality_usage"), equalTo(0)); + return ObjectPath.createFromXContent(JsonXContent.jsonXContent, BytesReference.bytes(builder)); } } } diff --git a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/boxplot/BoxplotAggregationBuilderTests.java b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/boxplot/BoxplotAggregationBuilderTests.java index 5d9b4a76310..e20649e57aa 100644 --- a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/boxplot/BoxplotAggregationBuilderTests.java +++ b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/boxplot/BoxplotAggregationBuilderTests.java @@ -6,19 +6,18 @@ package org.elasticsearch.xpack.analytics.boxplot; +import org.elasticsearch.common.ParseField; import org.elasticsearch.common.io.stream.Writeable; -import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.common.xcontent.XContentParser; -import org.elasticsearch.search.SearchModule; import org.elasticsearch.search.aggregations.AggregatorFactories; +import org.elasticsearch.search.aggregations.BaseAggregationBuilder; import org.elasticsearch.test.AbstractSerializingTestCase; -import org.elasticsearch.xpack.analytics.AnalyticsPlugin; import org.junit.Before; import java.io.IOException; -import java.util.Collections; +import static java.util.Collections.singletonList; import static org.hamcrest.Matchers.hasSize; public class BoxplotAggregationBuilderTests extends AbstractSerializingTestCase { @@ -31,8 +30,10 @@ public class BoxplotAggregationBuilderTests extends AbstractSerializingTestCase< @Override protected NamedXContentRegistry xContentRegistry() { - SearchModule searchModule = new SearchModule(Settings.EMPTY, false, Collections.singletonList(new AnalyticsPlugin(Settings.EMPTY))); - return new NamedXContentRegistry(searchModule.getNamedXContents()); + return new NamedXContentRegistry(singletonList(new NamedXContentRegistry.Entry( + BaseAggregationBuilder.class, + new ParseField(BoxplotAggregationBuilder.NAME), + (p, n) -> BoxplotAggregationBuilder.PARSER.apply(p, (String) n)))); } @Override diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/analytics/action/AnalyticsStatsAction.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/analytics/action/AnalyticsStatsAction.java index cf66f831208..136eeee6b3e 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/analytics/action/AnalyticsStatsAction.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/analytics/action/AnalyticsStatsAction.java @@ -110,48 +110,80 @@ public class AnalyticsStatsAction extends ActionType