diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedConfig.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedConfig.java index 0525ca28b1b..4ebb12555e2 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedConfig.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedConfig.java @@ -10,6 +10,7 @@ import org.elasticsearch.Version; import org.elasticsearch.cluster.AbstractDiffable; import org.elasticsearch.common.ParseField; import org.elasticsearch.common.Strings; +import org.elasticsearch.common.TriFunction; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.unit.TimeValue; @@ -46,7 +47,6 @@ import java.util.Map; import java.util.Objects; import java.util.Random; import java.util.concurrent.TimeUnit; -import java.util.function.BiFunction; /** * Datafeed configuration options. Describes where to proactively pull input @@ -65,9 +65,10 @@ public class DatafeedConfig extends AbstractDiffable implements private static final int TWENTY_MINS_SECONDS = 20 * SECONDS_IN_MINUTE; private static final int HALF_DAY_SECONDS = 12 * 60 * SECONDS_IN_MINUTE; static final XContentObjectTransformer QUERY_TRANSFORMER = XContentObjectTransformer.queryBuilderTransformer(); - private static final BiFunction, String, QueryBuilder> lazyQueryParser = (objectMap, id) -> { + static final TriFunction, String, List, QueryBuilder> lazyQueryParser = + (objectMap, id, warnings) -> { try { - return QUERY_TRANSFORMER.fromMap(objectMap); + return QUERY_TRANSFORMER.fromMap(objectMap, warnings); } catch (IOException | XContentParseException exception) { // Certain thrown exceptions wrap up the real Illegal argument making it hard to determine cause for the user if (exception.getCause() instanceof IllegalArgumentException) { @@ -85,9 +86,10 @@ public class DatafeedConfig extends AbstractDiffable implements }; static final XContentObjectTransformer AGG_TRANSFORMER = XContentObjectTransformer.aggregatorTransformer(); - private static final BiFunction, String, AggregatorFactories.Builder> lazyAggParser = (objectMap, id) -> { + static final TriFunction, String, List, AggregatorFactories.Builder> lazyAggParser = + (objectMap, id, warnings) -> { try { - return AGG_TRANSFORMER.fromMap(objectMap); + return AGG_TRANSFORMER.fromMap(objectMap, warnings); } catch (IOException | XContentParseException exception) { // Certain thrown exceptions wrap up the real Illegal argument making it hard to determine cause for the user if (exception.getCause() instanceof IllegalArgumentException) { @@ -237,8 +239,8 @@ public class DatafeedConfig extends AbstractDiffable implements this.chunkingConfig = chunkingConfig; this.headers = Collections.unmodifiableMap(headers); this.delayedDataCheckConfig = delayedDataCheckConfig; - this.querySupplier = new CachedSupplier<>(() -> lazyQueryParser.apply(query, id)); - this.aggSupplier = new CachedSupplier<>(() -> lazyAggParser.apply(aggregations, id)); + this.querySupplier = new CachedSupplier<>(() -> lazyQueryParser.apply(query, id, new ArrayList<>())); + this.aggSupplier = new CachedSupplier<>(() -> lazyAggParser.apply(aggregations, id, new ArrayList<>())); } public DatafeedConfig(StreamInput in) throws IOException { @@ -284,8 +286,8 @@ public class DatafeedConfig extends AbstractDiffable implements } else { delayedDataCheckConfig = DelayedDataCheckConfig.defaultDelayedDataCheckConfig(); } - this.querySupplier = new CachedSupplier<>(() -> lazyQueryParser.apply(query, id)); - this.aggSupplier = new CachedSupplier<>(() -> lazyAggParser.apply(aggregations, id)); + this.querySupplier = new CachedSupplier<>(() -> lazyQueryParser.apply(query, id, new ArrayList<>())); + this.aggSupplier = new CachedSupplier<>(() -> lazyAggParser.apply(aggregations, id, new ArrayList<>())); } public String getId() { @@ -320,6 +322,20 @@ public class DatafeedConfig extends AbstractDiffable implements return querySupplier.get(); } + /** + * Calls the lazy parser and returns any gathered deprecations + * @return The deprecations from parsing the query + */ + public List getQueryDeprecations() { + return getQueryDeprecations(lazyQueryParser); + } + + List getQueryDeprecations(TriFunction, String, List, QueryBuilder> parser) { + List deprecations = new ArrayList<>(); + parser.apply(query, id, deprecations); + return deprecations; + } + public Map getQuery() { return query; } @@ -328,6 +344,20 @@ public class DatafeedConfig extends AbstractDiffable implements return aggSupplier.get(); } + /** + * Calls the lazy parser and returns any gathered deprecations + * @return The deprecations from parsing the aggregations + */ + public List getAggDeprecations() { + return getAggDeprecations(lazyAggParser); + } + + List getAggDeprecations(TriFunction, String, List, AggregatorFactories.Builder> parser) { + List deprecations = new ArrayList<>(); + parser.apply(aggregations, id, deprecations); + return deprecations; + } + public Map getAggregations() { return aggregations; } @@ -758,7 +788,8 @@ public class DatafeedConfig extends AbstractDiffable implements if (aggregations == null) { chunkingConfig = ChunkingConfig.newAuto(); } else { - long histogramIntervalMillis = ExtractorUtils.getHistogramIntervalMillis(lazyAggParser.apply(aggregations, id)); + long histogramIntervalMillis = + ExtractorUtils.getHistogramIntervalMillis(lazyAggParser.apply(aggregations, id, new ArrayList<>())); chunkingConfig = ChunkingConfig.newManual(TimeValue.timeValueMillis( DEFAULT_AGGREGATION_CHUNKING_BUCKETS * histogramIntervalMillis)); } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/utils/LoggingDeprecationAccumulationHandler.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/utils/LoggingDeprecationAccumulationHandler.java index 1ab443b12de..2322b3d9026 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/utils/LoggingDeprecationAccumulationHandler.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/utils/LoggingDeprecationAccumulationHandler.java @@ -29,16 +29,14 @@ public class LoggingDeprecationAccumulationHandler implements DeprecationHandler public void usedDeprecatedName(String usedName, String modernName) { LoggingDeprecationHandler.INSTANCE.usedDeprecatedName(usedName, modernName); deprecations.add(LoggerMessageFormat.format("Deprecated field [{}] used, expected [{}] instead", - usedName, - modernName)); + new Object[] {usedName, modernName})); } @Override public void usedDeprecatedField(String usedName, String replacedWith) { LoggingDeprecationHandler.INSTANCE.usedDeprecatedField(usedName, replacedWith); deprecations.add(LoggerMessageFormat.format("Deprecated field [{}] used, replaced by [{}]", - usedName, - replacedWith)); + new Object[] {usedName, replacedWith})); } /** diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/utils/XContentObjectTransformer.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/utils/XContentObjectTransformer.java index 9749f565660..bbea1014183 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/utils/XContentObjectTransformer.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/utils/XContentObjectTransformer.java @@ -22,7 +22,9 @@ import org.elasticsearch.search.SearchModule; import org.elasticsearch.search.aggregations.AggregatorFactories; import java.io.IOException; +import java.util.ArrayList; import java.util.Collections; +import java.util.List; import java.util.Map; /** @@ -61,7 +63,27 @@ public class XContentObjectTransformer { this.registry = registry; } + /** + * Parses the map into the type T with the previously supplied parserFunction + * All deprecation warnings are ignored + * @param stringObjectMap The Map to parse into the Object + * @return parsed object T + * @throws IOException When there is an unforeseen parsing issue + */ public T fromMap(Map stringObjectMap) throws IOException { + return fromMap(stringObjectMap, new ArrayList<>()); + } + + /** + * Parses the map into the type T with the previously supplied parserFunction + * All deprecation warnings are added to the passed deprecationWarnings list. + * + * @param stringObjectMap The Map to parse into the Object + * @param deprecationWarnings The list to which to add all deprecation warnings + * @return parsed object T + * @throws IOException When there is an unforeseen parsing issue + */ + public T fromMap(Map stringObjectMap, List deprecationWarnings) throws IOException { if (stringObjectMap == null) { return null; } @@ -72,8 +94,9 @@ public class XContentObjectTransformer { .createParser(registry, deprecationLogger, BytesReference.bytes(xContentBuilder).streamInput())) { - //TODO do something with the accumulated deprecation warnings - return parserFunction.apply(parser); + T retVal = parserFunction.apply(parser); + deprecationWarnings.addAll(deprecationLogger.getDeprecations()); + return retVal; } } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedConfigTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedConfigTests.java index 554fd131c1e..d39fd35dd9c 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedConfigTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedConfigTests.java @@ -57,9 +57,12 @@ import java.util.TimeZone; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThanOrEqualTo; +import static org.hamcrest.Matchers.hasItem; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.lessThan; import static org.hamcrest.Matchers.not; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verify; public class DatafeedConfigTests extends AbstractSerializingTestCase { @@ -577,6 +580,34 @@ public class DatafeedConfigTests extends AbstractSerializingTestCase deprecations = datafeed.getAggDeprecations((map, id, deprecationlist) -> { + deprecationlist.add(deprecationWarning); + return new AggregatorFactories.Builder().addAggregator(new MaxAggregationBuilder("field").field("field")); + }); + assertThat(deprecations, hasItem(deprecationWarning)); + + DatafeedConfig spiedConfig = spy(datafeed); + spiedConfig.getAggDeprecations(); + verify(spiedConfig).getAggDeprecations(DatafeedConfig.lazyAggParser); + } + + public void testGetQueryDeprecations() { + DatafeedConfig datafeed = createDatafeedWithDateHistogram("1h"); + String deprecationWarning = "Warning"; + List deprecations = datafeed.getQueryDeprecations((map, id, deprecationlist) -> { + deprecationlist.add(deprecationWarning); + return new BoolQueryBuilder(); + }); + assertThat(deprecations, hasItem(deprecationWarning)); + + DatafeedConfig spiedConfig = spy(datafeed); + spiedConfig.getQueryDeprecations(); + verify(spiedConfig).getQueryDeprecations(DatafeedConfig.lazyQueryParser); + } + public void testSerializationOfComplexAggs() throws IOException { MaxAggregationBuilder maxTime = AggregationBuilders.max("timestamp").field("timestamp"); AvgAggregationBuilder avgAggregationBuilder = AggregationBuilders.avg("bytes_in_avg").field("system.network.in.bytes"); diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/utils/XContentObjectTransformerTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/utils/XContentObjectTransformerTests.java index ea125f216ba..1f61168c420 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/utils/XContentObjectTransformerTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/utils/XContentObjectTransformerTests.java @@ -7,11 +7,13 @@ package org.elasticsearch.xpack.core.ml.utils; import org.elasticsearch.common.ParsingException; import org.elasticsearch.common.bytes.BytesReference; +import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.common.xcontent.ToXContentObject; import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentHelper; import org.elasticsearch.common.xcontent.XContentParseException; import org.elasticsearch.common.xcontent.XContentType; +import org.elasticsearch.index.query.BoolQueryBuilder; import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.index.query.QueryBuilders; import org.elasticsearch.search.aggregations.AggregationBuilders; @@ -20,14 +22,18 @@ import org.elasticsearch.search.aggregations.metrics.MaxAggregationBuilder; import org.elasticsearch.test.ESTestCase; import java.io.IOException; +import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; +import java.util.List; import java.util.Map; import static org.elasticsearch.common.xcontent.ToXContent.EMPTY_PARAMS; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertToXContentEquivalent; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.hasItem; +import static org.hamcrest.Matchers.hasSize; public class XContentObjectTransformerTests extends ESTestCase { @@ -104,6 +110,26 @@ public class XContentObjectTransformerTests extends ESTestCase { queryBuilderTransformer.toMap(queryBuilder)); } + public void testDeprecationWarnings() throws IOException { + XContentObjectTransformer queryBuilderTransformer = new XContentObjectTransformer<>(NamedXContentRegistry.EMPTY, + (p)-> { + p.getDeprecationHandler().usedDeprecatedField("oldField", "newField"); + p.getDeprecationHandler().usedDeprecatedName("oldName", "modernName"); + return new BoolQueryBuilder(); + }); + List deprecations = new ArrayList<>(); + queryBuilderTransformer.fromMap(Collections.singletonMap("bool", "match"), deprecations); + + assertThat(deprecations, hasSize(2)); + assertThat(deprecations, hasItem("Deprecated field [oldField] used, replaced by [newField]")); + assertThat(deprecations, hasItem("Deprecated field [oldName] used, expected [modernName] instead")); + } + + @Override + protected boolean enableWarningsCheck() { + return false; + } + private void assertXContentAreEqual(ToXContentObject object, Map map) throws IOException { XContentType xContentType = XContentType.JSON; BytesReference objectReference = XContentHelper.toXContent(object, xContentType, EMPTY_PARAMS, false); diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportStartDatafeedAction.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportStartDatafeedAction.java index de0caee778e..9e749ddf8b3 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportStartDatafeedAction.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportStartDatafeedAction.java @@ -5,6 +5,7 @@ */ package org.elasticsearch.xpack.ml.action; +import org.elasticsearch.common.Strings; import org.elasticsearch.ElasticsearchException; import org.elasticsearch.ElasticsearchStatusException; import org.elasticsearch.ResourceAlreadyExistsException; @@ -47,7 +48,9 @@ import org.elasticsearch.xpack.ml.MachineLearning; import org.elasticsearch.xpack.ml.datafeed.DatafeedManager; import org.elasticsearch.xpack.ml.datafeed.DatafeedNodeSelector; import org.elasticsearch.xpack.ml.datafeed.extractor.DataExtractorFactory; +import org.elasticsearch.xpack.ml.notifications.Auditor; +import java.util.ArrayList; import java.util.List; import java.util.Locale; import java.util.Map; @@ -66,18 +69,20 @@ public class TransportStartDatafeedAction extends TransportMasterNodeAction deprecationWarnings = new ArrayList<>(); + deprecationWarnings.addAll(datafeed.getAggDeprecations()); + deprecationWarnings.addAll(datafeed.getQueryDeprecations()); + if (deprecationWarnings.isEmpty() == false) { + String msg = "datafeed [" + datafeed.getId() +"] configuration has deprecations. [" + + Strings.collectionToDelimitedString(deprecationWarnings, ", ") + "]"; + auditor.warning(job.getId(), msg); + } + + } + @Override protected String executor() { // This api doesn't do heavy or blocking operations (just delegates PersistentTasksService), @@ -142,6 +160,8 @@ public class TransportStartDatafeedAction extends TransportMasterNodeAction