diff --git a/docs/changelog/138715.yaml b/docs/changelog/138715.yaml new file mode 100644 index 0000000000000..627fcd3b5b88f --- /dev/null +++ b/docs/changelog/138715.yaml @@ -0,0 +1,5 @@ +pr: 138715 +summary: Fix downsampling with disabled subobjects +area: Downsampling +type: bug +issues: [] diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/stats/MappingVisitor.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/stats/MappingVisitor.java index a4b7411867188..ea568b23ead64 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/stats/MappingVisitor.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/stats/MappingVisitor.java @@ -9,10 +9,16 @@ package org.elasticsearch.action.admin.cluster.stats; +import org.elasticsearch.common.TriConsumer; + +import java.util.HashMap; import java.util.Map; import java.util.function.BiConsumer; public final class MappingVisitor { + public static final String PROPERTIES = "properties"; + public static final String FIELD_TYPE = "type"; + public static final String MULTI_FIELDS = "fields"; private MappingVisitor() {} @@ -25,7 +31,7 @@ private static void visitMapping( final String path, final BiConsumer> fieldMappingConsumer ) { - Object properties = mapping.get("properties"); + Object properties = mapping.get(PROPERTIES); if (properties instanceof Map) { @SuppressWarnings("unchecked") Map propertiesAsMap = (Map) properties; @@ -40,7 +46,7 @@ private static void visitMapping( visitMapping(fieldMapping, prefix + ".", fieldMappingConsumer); // Multi fields - Object fieldsO = fieldMapping.get("fields"); + Object fieldsO = fieldMapping.get(MULTI_FIELDS); if (fieldsO instanceof Map) { @SuppressWarnings("unchecked") Map fields = (Map) fieldsO; @@ -75,4 +81,67 @@ public static void visitRuntimeMapping(Map mapping, BiConsumer sourceMapping, + final Map destMapping, + final TriConsumer, Map> fieldMappingConsumer + ) { + Map sourceProperties = getMapOrNull(sourceMapping.get(PROPERTIES)); + if (sourceProperties == null) { + return; + } + Map destProperties = new HashMap<>(sourceProperties.size()); + destMapping.put(PROPERTIES, destProperties); + + for (Map.Entry entry : sourceProperties.entrySet()) { + Map sourceFieldMapping = getMapOrNull(entry.getValue()); + if (sourceFieldMapping == null) { + return; + } + var destFieldMapping = processAndCopy(entry.getKey(), sourceFieldMapping, destProperties, fieldMappingConsumer); + visitAndCopyMapping(sourceFieldMapping, destFieldMapping, fieldMappingConsumer); + + // Multi fields + Map sourceMultiFields = getMapOrNull(sourceFieldMapping.get(MULTI_FIELDS)); + if (sourceMultiFields == null) { + continue; + } + Map destFields = new HashMap<>(sourceMultiFields.size()); + destFieldMapping.put(MULTI_FIELDS, destFields); + for (Map.Entry multiFieldEntry : sourceMultiFields.entrySet()) { + String multiFieldName = multiFieldEntry.getKey(); + Map sourceMultiFieldMapping = getMapOrNull(multiFieldEntry.getValue()); + if (sourceMultiFieldMapping == null) { + continue; + } + processAndCopy(multiFieldName, sourceMultiFieldMapping, destFields, fieldMappingConsumer); + } + } + } + + private static Map getMapOrNull(Object object) { + if (object instanceof Map) { + @SuppressWarnings("unchecked") + Map map = (Map) object; + return map; + } + return null; + } + + private static Map processAndCopy( + String fieldName, + Map sourceFieldMapping, + Map destParentMap, + TriConsumer, Map> fieldMappingConsumer + ) { + Map destFieldMapping = new HashMap<>(sourceFieldMapping.size()); + destParentMap.put(fieldName, destFieldMapping); + fieldMappingConsumer.apply(fieldName, sourceFieldMapping, destFieldMapping); + return destFieldMapping; + } } diff --git a/server/src/test/java/org/elasticsearch/action/admin/cluster/stats/MappingVisitorTests.java b/server/src/test/java/org/elasticsearch/action/admin/cluster/stats/MappingVisitorTests.java index fe8b6a12f70d9..f4278a464a558 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/cluster/stats/MappingVisitorTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/cluster/stats/MappingVisitorTests.java @@ -136,4 +136,61 @@ public void testCountRuntimeFields() { private static void collectRuntimeTypes(Map mapping, Set types) { MappingVisitor.visitRuntimeMapping(mapping, (f, m) -> types.add(m.get("type").toString())); } + + @SuppressWarnings("unchecked") + public void testConvertLongToKeyword() { + Map longType = Map.of("type", "long"); + Map textType = Map.of("type", "text"); + Map floatType = Map.of("type", "float", "scaling_factor", 1000); + Map multiField = Map.of("type", "keyword", "fields", Map.of("my-long", longType, "my-float", floatType)); + Map objectField = Map.of("type", "object", "properties", Map.of("my-text", textType, "my-long", longType)); + Map expectedProperties = Map.of( + "properties", + Map.of("my-long", longType, "my-float", floatType, "my-multi-field", multiField, "my-object", objectField) + ); + + HashMap result = new HashMap<>(); + MappingVisitor.visitAndCopyMapping(expectedProperties, result, (ignored, source, dest) -> { + for (String key : source.keySet()) { + if (key.equals("type") && source.get(key).equals("long")) { + dest.put(key, "keyword"); + } else { + dest.put(key, source.get(key)); + } + } + }); + + assertTrue(result.containsKey("properties")); + Map properties = (Map) result.get("properties"); + + assertTrue(properties.containsKey("my-long")); + Map myLong = (Map) properties.get("my-long"); + assertEquals("keyword", myLong.get("type")); + + assertTrue(properties.containsKey("my-float")); + Map myFloat = (Map) properties.get("my-float"); + assertEquals("float", myFloat.get("type")); + assertEquals(1000, myFloat.get("scaling_factor")); + + assertTrue(properties.containsKey("my-multi-field")); + Map myMultiField = (Map) properties.get("my-multi-field"); + assertEquals("keyword", myMultiField.get("type")); + assertTrue(myMultiField.containsKey("fields")); + Map foundFields = (Map) myMultiField.get("fields"); + assertTrue(foundFields.containsKey("my-long")); + assertEquals("keyword", ((Map) foundFields.get("my-long")).get("type")); + assertTrue(foundFields.containsKey("my-float")); + assertEquals("float", ((Map) foundFields.get("my-float")).get("type")); + assertEquals(1000, ((Map) foundFields.get("my-float")).get("scaling_factor")); + + assertTrue(properties.containsKey("my-object")); + Map myObject = (Map) properties.get("my-object"); + assertEquals("object", myObject.get("type")); + assertTrue(myObject.containsKey("properties")); + Map foundSubObjects = (Map) myObject.get("properties"); + assertTrue(foundSubObjects.containsKey("my-long")); + assertEquals("keyword", ((Map) foundSubObjects.get("my-long")).get("type")); + assertTrue(foundSubObjects.containsKey("my-text")); + assertEquals("text", ((Map) foundSubObjects.get("my-text")).get("type")); + } } diff --git a/x-pack/plugin/downsample/src/internalClusterTest/java/org/elasticsearch/xpack/downsample/DownsampleIT.java b/x-pack/plugin/downsample/src/internalClusterTest/java/org/elasticsearch/xpack/downsample/DownsampleIT.java index 8e2bce4251251..68a41be9dfff9 100644 --- a/x-pack/plugin/downsample/src/internalClusterTest/java/org/elasticsearch/xpack/downsample/DownsampleIT.java +++ b/x-pack/plugin/downsample/src/internalClusterTest/java/org/elasticsearch/xpack/downsample/DownsampleIT.java @@ -102,6 +102,14 @@ public void testDownsamplingPassthroughMetrics() throws Exception { "cpu_usage": { "type": "double", "time_series_metric": "counter" + }, + "memory_usage": { + "type": "double", + "time_series_metric": "counter" + }, + "memory_usage.free": { + "type": "double", + "time_series_metric": "counter" } } } @@ -120,6 +128,8 @@ public void testDownsamplingPassthroughMetrics() throws Exception { .field("attributes.os.name", randomFrom("linux", "windows", "macos")) .field("metrics.cpu_usage", randomDouble()) .field("metrics.memory_usage", randomDouble()) + .field("metrics.memory_usage.free", randomDouble()) + .field("metrics.load", randomDouble()) .endObject(); } catch (IOException e) { throw new RuntimeException(e); diff --git a/x-pack/plugin/downsample/src/main/java/org/elasticsearch/xpack/downsample/FieldValueFetcher.java b/x-pack/plugin/downsample/src/main/java/org/elasticsearch/xpack/downsample/FieldValueFetcher.java index c9913302372bd..e6d30de4392a7 100644 --- a/x-pack/plugin/downsample/src/main/java/org/elasticsearch/xpack/downsample/FieldValueFetcher.java +++ b/x-pack/plugin/downsample/src/main/java/org/elasticsearch/xpack/downsample/FieldValueFetcher.java @@ -103,10 +103,7 @@ static List create(SearchExecutionContext context, String[] f } else { fieldData = context.getForField(fieldType, MappedFieldType.FielddataOperation.SEARCH); } - final String fieldName = context.isMultiField(field) - ? fieldType.name().substring(0, fieldType.name().lastIndexOf('.')) - : fieldType.name(); - fetchers.add(new FieldValueFetcher(fieldName, fieldType, fieldData, samplingMethod)); + fetchers.add(new FieldValueFetcher(fieldType.name(), fieldType, fieldData, samplingMethod)); } } } diff --git a/x-pack/plugin/downsample/src/main/java/org/elasticsearch/xpack/downsample/TimeseriesFieldTypeHelper.java b/x-pack/plugin/downsample/src/main/java/org/elasticsearch/xpack/downsample/TimeseriesFieldTypeHelper.java index 93425be0b7d40..68b46a86f3284 100644 --- a/x-pack/plugin/downsample/src/main/java/org/elasticsearch/xpack/downsample/TimeseriesFieldTypeHelper.java +++ b/x-pack/plugin/downsample/src/main/java/org/elasticsearch/xpack/downsample/TimeseriesFieldTypeHelper.java @@ -52,7 +52,7 @@ public boolean isTimeSeriesDimension(final String unused, final Map f return Boolean.TRUE.equals(fieldMapping.get(TIME_SERIES_DIMENSION_PARAM)) && isPassthroughField(fieldMapping) == false; } - public static boolean isPassthroughField(final Map fieldMapping) { + public boolean isPassthroughField(final Map fieldMapping) { return PassThroughObjectMapper.CONTENT_TYPE.equals(fieldMapping.get(ContextMapping.FIELD_TYPE)); } diff --git a/x-pack/plugin/downsample/src/main/java/org/elasticsearch/xpack/downsample/TransportDownsampleAction.java b/x-pack/plugin/downsample/src/main/java/org/elasticsearch/xpack/downsample/TransportDownsampleAction.java index ea6f33c903009..51f2b6f6ff116 100644 --- a/x-pack/plugin/downsample/src/main/java/org/elasticsearch/xpack/downsample/TransportDownsampleAction.java +++ b/x-pack/plugin/downsample/src/main/java/org/elasticsearch/xpack/downsample/TransportDownsampleAction.java @@ -45,13 +45,11 @@ import org.elasticsearch.cluster.service.MasterServiceTaskQueue; import org.elasticsearch.common.Priority; import org.elasticsearch.common.Strings; -import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.compress.CompressedXContent; import org.elasticsearch.common.settings.IndexScopedSettings; import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.concurrent.EsExecutors; -import org.elasticsearch.common.xcontent.XContentHelper; import org.elasticsearch.core.TimeValue; import org.elasticsearch.core.Tuple; import org.elasticsearch.index.Index; @@ -75,8 +73,6 @@ import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.transport.TransportService; import org.elasticsearch.xcontent.XContentBuilder; -import org.elasticsearch.xcontent.XContentFactory; -import org.elasticsearch.xcontent.XContentType; import org.elasticsearch.xpack.aggregatemetric.mapper.AggregateMetricDoubleFieldMapper; import org.elasticsearch.xpack.core.ClientHelper; import org.elasticsearch.xpack.core.downsample.DownsampleShardPersistentTaskState; @@ -90,6 +86,7 @@ import java.time.OffsetDateTime; import java.time.format.DateTimeFormatter; import java.util.ArrayList; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Objects; @@ -99,6 +96,10 @@ import java.util.function.Predicate; import java.util.function.Supplier; +import static org.elasticsearch.action.admin.cluster.stats.MappingVisitor.FIELD_TYPE; +import static org.elasticsearch.action.admin.cluster.stats.MappingVisitor.MULTI_FIELDS; +import static org.elasticsearch.action.admin.cluster.stats.MappingVisitor.PROPERTIES; +import static org.elasticsearch.action.admin.cluster.stats.MappingVisitor.visitAndCopyMapping; import static org.elasticsearch.index.mapper.TimeSeriesParams.TIME_SERIES_METRIC_PARAM; import static org.elasticsearch.xpack.core.ilm.DownsampleAction.DOWNSAMPLED_INDEX_PREFIX; @@ -378,7 +379,7 @@ protected void masterOperation( final String mapping; try { - mapping = createDownsampleIndexMapping(helper, request.getDownsampleConfig(), mapperService, sourceIndexMappings); + mapping = createDownsampleIndexMapping(helper, request.getDownsampleConfig(), sourceIndexMappings); } catch (IOException e) { recordFailureMetrics(startTime); delegate.onFailure(e); @@ -716,88 +717,81 @@ protected ClusterBlockException checkBlock(DownsampleAction.Request request, Clu public static String createDownsampleIndexMapping( final TimeseriesFieldTypeHelper helper, final DownsampleConfig config, - final MapperService mapperService, final Map sourceIndexMappings ) throws IOException { - final XContentBuilder builder = XContentFactory.jsonBuilder().startObject(); - addDynamicTemplates(builder); - - builder.startObject("properties"); - - addTimestampField(config, sourceIndexMappings, builder); - addMetricFieldOverwrites(config, helper, sourceIndexMappings, builder); - - builder.endObject(); // match initial startObject - builder.endObject(); // match startObject("properties") - - final CompressedXContent mappingDiffXContent = CompressedXContent.fromJSON( - XContentHelper.convertToJson(BytesReference.bytes(builder), false, XContentType.JSON) - ); - return mapperService.merge(MapperService.SINGLE_MAPPING_NAME, mappingDiffXContent, MapperService.MergeReason.INDEX_TEMPLATE) - .mappingSource() - .uncompressed() - .utf8ToString(); + final String timestampField = config.getTimestampField(); + final String dateIntervalType = config.getIntervalType(); + final String dateInterval = config.getInterval().toString(); + final String timezone = config.getTimeZone(); + Map downsampledMapping = new HashMap<>(); + for (Map.Entry entry : sourceIndexMappings.entrySet()) { + if (entry.getKey().equals(PROPERTIES) == false) { + downsampledMapping.put(entry.getKey(), entry.getValue()); + } + } + visitAndCopyMapping(sourceIndexMappings, downsampledMapping, (fieldName, sourceMapping, updatedMapping) -> { + if (timestampField.equals(fieldName)) { + updateTimestampField(sourceMapping, updatedMapping, dateIntervalType, dateInterval, timezone); + } else if (helper.isTimeSeriesMetric(fieldName, sourceMapping)) { + processMetricField(sourceMapping, updatedMapping, config.getSamplingMethodOrDefault()); + } else { + copyMapping(sourceMapping, updatedMapping); + } + }); + return new CompressedXContent(downsampledMapping).uncompressed().utf8ToString(); } /** * Adds metric mapping overwrites. When downsampling certain metrics change their mapping type. For example, * when we are using the aggregate sampling method, the mapping of a gauge metric becomes an aggregate_metric_double. */ - private static void addMetricFieldOverwrites( - final DownsampleConfig config, - final TimeseriesFieldTypeHelper helper, - final Map sourceIndexMappings, - final XContentBuilder builder + private static void processMetricField( + Map sourceMapping, + Map updatedMapping, + DownsampleConfig.SamplingMethod samplingMethod ) { - // The last value sampling method preserves the source mapping. - if (config.getSamplingMethodOrDefault() == DownsampleConfig.SamplingMethod.LAST_VALUE) { - return; + final TimeSeriesParams.MetricType metricType = TimeSeriesParams.MetricType.fromString( + sourceMapping.get(TIME_SERIES_METRIC_PARAM).toString() + ); + if (samplingMethod == DownsampleConfig.SamplingMethod.AGGREGATE + && metricType == TimeSeriesParams.MetricType.GAUGE + && AggregateMetricDoubleFieldMapper.CONTENT_TYPE.equals(sourceMapping.get(FIELD_TYPE)) == false) { + var supportedMetrics = getSupportedMetrics(metricType, sourceMapping); + + updatedMapping.put(TIME_SERIES_METRIC_PARAM, metricType.toString()); + updatedMapping.put(FIELD_TYPE, AggregateMetricDoubleFieldMapper.CONTENT_TYPE); + updatedMapping.put(AggregateMetricDoubleFieldMapper.Names.METRICS, supportedMetrics.supportedMetrics); + updatedMapping.put(AggregateMetricDoubleFieldMapper.Names.DEFAULT_METRIC, supportedMetrics.defaultMetric); + } else { + copyMapping(sourceMapping, updatedMapping); } - MappingVisitor.visitMapping(sourceIndexMappings, (field, mapping) -> { - if (helper.isTimeSeriesMetric(field, mapping)) { - try { - addMetricFieldMapping(builder, field, mapping); - } catch (IOException e) { - throw new ElasticsearchException("Error while adding metric for field [" + field + "]"); - } - } - }); } - private static void addTimestampField( - final DownsampleConfig config, - Map sourceIndexMappings, - final XContentBuilder builder - ) throws IOException { - final String timestampField = config.getTimestampField(); - final String dateIntervalType = config.getIntervalType(); - final String dateInterval = config.getInterval().toString(); - final String timezone = config.getTimeZone(); - builder.startObject(timestampField); - - MappingVisitor.visitMapping(sourceIndexMappings, (field, mapping) -> { - try { - if (timestampField.equals(field)) { - final String timestampType = String.valueOf(mapping.get("type")); - builder.field("type", timestampType != null ? timestampType : DateFieldMapper.CONTENT_TYPE); - if (mapping.get("format") != null) { - builder.field("format", mapping.get("format")); - } - if (mapping.get("ignore_malformed") != null) { - builder.field("ignore_malformed", mapping.get("ignore_malformed")); - } - } - } catch (IOException e) { - throw new ElasticsearchException("Unable to create timestamp field mapping for field [" + timestampField + "]", e); + private static void copyMapping(Map sourceMapping, Map updatedMapping) { + for (String f : sourceMapping.keySet()) { + if (f.equals(PROPERTIES) == false && f.equals(MULTI_FIELDS) == false) { + updatedMapping.put(f, sourceMapping.get(f)); } - }); + } + } - builder.startObject("meta") - .field(dateIntervalType, dateInterval) - .field(DownsampleConfig.TIME_ZONE, timezone) - .endObject() - .endObject(); + private static void updateTimestampField( + Map sourceMapping, + Map updatedMapping, + String dateIntervalType, + String dateInterval, + String timezone + ) { + final String timestampType = String.valueOf(sourceMapping.get(FIELD_TYPE)); + updatedMapping.put(FIELD_TYPE, timestampType != null ? timestampType : DateFieldMapper.CONTENT_TYPE); + if (sourceMapping.get("format") != null) { + updatedMapping.put("format", sourceMapping.get("format")); + } + if (sourceMapping.get("ignore_malformed") != null) { + updatedMapping.put("ignore_malformed", sourceMapping.get("ignore_malformed")); + } + updatedMapping.put("meta", Map.of(dateIntervalType, dateInterval, DownsampleConfig.TIME_ZONE, timezone)); } // public for testing @@ -833,29 +827,6 @@ public static AggregateMetricDoubleFieldSupportedMetrics getSupportedMetrics( return new AggregateMetricDoubleFieldSupportedMetrics(defaultMetric, supportedAggs); } - private static void addMetricFieldMapping(final XContentBuilder builder, final String field, final Map fieldProperties) - throws IOException { - final TimeSeriesParams.MetricType metricType = TimeSeriesParams.MetricType.fromString( - fieldProperties.get(TIME_SERIES_METRIC_PARAM).toString() - ); - builder.startObject(field); - if (metricType == TimeSeriesParams.MetricType.COUNTER) { - // For counters, we keep the same field type, because they store - // only one value (the last value of the counter) - for (String fieldProperty : fieldProperties.keySet()) { - builder.field(fieldProperty, fieldProperties.get(fieldProperty)); - } - } else { - var supported = getSupportedMetrics(metricType, fieldProperties); - - builder.field("type", AggregateMetricDoubleFieldMapper.CONTENT_TYPE) - .stringListField(AggregateMetricDoubleFieldMapper.Names.METRICS, supported.supportedMetrics) - .field(AggregateMetricDoubleFieldMapper.Names.DEFAULT_METRIC, supported.defaultMetric) - .field(TIME_SERIES_METRIC_PARAM, metricType); - } - builder.endObject(); - } - private static void validateDownsamplingConfiguration( MapperService mapperService, DownsampleConfig config,