diff --git a/modules/data-streams/src/javaRestTest/java/org/elasticsearch/datastreams/TsdbDataStreamRestIT.java b/modules/data-streams/src/javaRestTest/java/org/elasticsearch/datastreams/TsdbDataStreamRestIT.java index d847666ba28b8..3e06c457bf74e 100644 --- a/modules/data-streams/src/javaRestTest/java/org/elasticsearch/datastreams/TsdbDataStreamRestIT.java +++ b/modules/data-streams/src/javaRestTest/java/org/elasticsearch/datastreams/TsdbDataStreamRestIT.java @@ -436,10 +436,8 @@ public void testSimulateTsdbDataStreamTemplate() throws Exception { ObjectPath.evaluate(responseBody, "template.settings.index.routing_path"), containsInAnyOrder("metricset", "k8s.pod.uid", "pod.labels.*") ); - assertThat( - ObjectPath.evaluate(responseBody, "template.settings.index.dimensions"), - containsInAnyOrder("metricset", "k8s.pod.uid", "pod.labels.*") - ); + // not supported when using a dynamic template for time_series_dimension + assertThat(ObjectPath.evaluate(responseBody, "template.settings.index.dimensions"), nullValue()); assertThat(ObjectPath.evaluate(responseBody, "overlapping"), empty()); } diff --git a/modules/data-streams/src/main/java/org/elasticsearch/datastreams/DataStreamIndexSettingsProvider.java b/modules/data-streams/src/main/java/org/elasticsearch/datastreams/DataStreamIndexSettingsProvider.java index 20c36bb4188c0..2fa6656b7fc82 100644 --- a/modules/data-streams/src/main/java/org/elasticsearch/datastreams/DataStreamIndexSettingsProvider.java +++ b/modules/data-streams/src/main/java/org/elasticsearch/datastreams/DataStreamIndexSettingsProvider.java @@ -254,13 +254,17 @@ private static boolean findDimensionFields(List dimensions, DocumentMapp if (template.isTimeSeriesDimension() == false) { continue; } - if (template.isSimplePathMatch() == false) { - // If the template is not using a simple path match, the dimensions list can't match all potential dimensions. - // For example, if the dynamic template matches by mapping type (all strings are mapped as dimensions), - // the coordinating node can't rely on the dimensions list to match all dimensions. - // In this case, the index.routing_path setting will be used instead. - matchesAllDimensions = false; - } + // At this point, we don't support index.dimensions when dimensions are mapped via a dynamic template. + // This is because more specific matches with a higher priority can exist that exclude certain fields from being mapped as a + // dimension. For example: + // - path_match: "labels.host_ip", time_series_dimension: false + // - path_match: "labels.*", time_series_dimension: true + // In this case, "labels.host_ip" is not a dimension, + // and adding labels.* to index.dimensions would lead to non-dimension fields being included in the tsid. + // Therefore, we fall back to using index.routing_path. + // While this also may include non-dimension fields in the routing path, + // it at least guarantees that the tsid only includes dimension fields and includes all dimension fields. + matchesAllDimensions = false; if (template.pathMatch().isEmpty() == false) { dimensions.addAll(template.pathMatch()); } diff --git a/modules/data-streams/src/test/java/org/elasticsearch/datastreams/DataStreamIndexSettingsProviderTests.java b/modules/data-streams/src/test/java/org/elasticsearch/datastreams/DataStreamIndexSettingsProviderTests.java index ac398c67130f6..ceae1f960fcdd 100644 --- a/modules/data-streams/src/test/java/org/elasticsearch/datastreams/DataStreamIndexSettingsProviderTests.java +++ b/modules/data-streams/src/test/java/org/elasticsearch/datastreams/DataStreamIndexSettingsProviderTests.java @@ -525,16 +525,12 @@ public void testGenerateRoutingPathFromDynamicTemplate() throws Exception { } """; Settings result = generateTsdbSettings(mapping, now); - assertThat(result.size(), equalTo(INDEX_DIMENSIONS_TSID_OPTIMIZATION_FEATURE_FLAG ? 5 : 4)); + assertThat(result.size(), equalTo(4)); assertThat(IndexSettings.MODE.get(result), equalTo(IndexMode.TIME_SERIES)); assertThat(IndexSettings.TIME_SERIES_START_TIME.get(result), equalTo(now.minusMillis(DEFAULT_LOOK_BACK_TIME.getMillis()))); assertThat(IndexSettings.TIME_SERIES_END_TIME.get(result), equalTo(now.plusMillis(DEFAULT_LOOK_AHEAD_TIME.getMillis()))); assertThat(IndexMetadata.INDEX_ROUTING_PATH.get(result), containsInAnyOrder("host.id", "prometheus.labels.*")); - if (INDEX_DIMENSIONS_TSID_OPTIMIZATION_FEATURE_FLAG) { - assertThat(IndexMetadata.INDEX_DIMENSIONS.get(result), containsInAnyOrder("host.id", "prometheus.labels.*")); - } else { - assertThat(IndexMetadata.INDEX_DIMENSIONS.get(result), empty()); - } + assertThat(IndexMetadata.INDEX_DIMENSIONS.get(result), empty()); } public void testGenerateRoutingPathFromDynamicTemplateWithMultiplePathMatchEntries() throws Exception { @@ -570,7 +566,7 @@ public void testGenerateRoutingPathFromDynamicTemplateWithMultiplePathMatchEntri } """; Settings result = generateTsdbSettings(mapping, now); - assertThat(result.size(), equalTo(INDEX_DIMENSIONS_TSID_OPTIMIZATION_FEATURE_FLAG ? 5 : 4)); + assertThat(result.size(), equalTo(4)); assertThat(IndexSettings.MODE.get(result), equalTo(IndexMode.TIME_SERIES)); assertThat(IndexSettings.TIME_SERIES_START_TIME.get(result), equalTo(now.minusMillis(DEFAULT_LOOK_BACK_TIME.getMillis()))); assertThat(IndexSettings.TIME_SERIES_END_TIME.get(result), equalTo(now.plusMillis(DEFAULT_LOOK_AHEAD_TIME.getMillis()))); @@ -578,14 +574,7 @@ public void testGenerateRoutingPathFromDynamicTemplateWithMultiplePathMatchEntri IndexMetadata.INDEX_ROUTING_PATH.get(result), containsInAnyOrder("host.id", "xprometheus.labels.*", "yprometheus.labels.*") ); - if (INDEX_DIMENSIONS_TSID_OPTIMIZATION_FEATURE_FLAG) { - assertThat( - IndexMetadata.INDEX_DIMENSIONS.get(result), - containsInAnyOrder("host.id", "xprometheus.labels.*", "yprometheus.labels.*") - ); - } else { - assertThat(IndexMetadata.INDEX_DIMENSIONS.get(result), empty()); - } + assertThat(IndexMetadata.INDEX_DIMENSIONS.get(result), empty()); } public void testGenerateRoutingPathFromDynamicTemplateWithMultiplePathMatchEntriesMultiFields() throws Exception { @@ -626,7 +615,7 @@ public void testGenerateRoutingPathFromDynamicTemplateWithMultiplePathMatchEntri } """; Settings result = generateTsdbSettings(mapping, now); - assertThat(result.size(), equalTo(INDEX_DIMENSIONS_TSID_OPTIMIZATION_FEATURE_FLAG ? 5 : 4)); + assertThat(result.size(), equalTo(4)); assertThat(IndexSettings.MODE.get(result), equalTo(IndexMode.TIME_SERIES)); assertThat(IndexSettings.TIME_SERIES_START_TIME.get(result), equalTo(now.minusMillis(DEFAULT_LOOK_BACK_TIME.getMillis()))); assertThat(IndexSettings.TIME_SERIES_END_TIME.get(result), equalTo(now.plusMillis(DEFAULT_LOOK_AHEAD_TIME.getMillis()))); @@ -634,14 +623,7 @@ public void testGenerateRoutingPathFromDynamicTemplateWithMultiplePathMatchEntri IndexMetadata.INDEX_ROUTING_PATH.get(result), containsInAnyOrder("host.id", "xprometheus.labels.*", "yprometheus.labels.*") ); - if (INDEX_DIMENSIONS_TSID_OPTIMIZATION_FEATURE_FLAG) { - assertThat( - IndexMetadata.INDEX_DIMENSIONS.get(result), - containsInAnyOrder("host.id", "xprometheus.labels.*", "yprometheus.labels.*") - ); - } else { - assertThat(IndexMetadata.INDEX_DIMENSIONS.get(result), empty()); - } + assertThat(IndexMetadata.INDEX_DIMENSIONS.get(result), empty()); } public void testGenerateRoutingPathFromDynamicTemplate_templateWithNoPathMatch() throws Exception { @@ -686,13 +668,51 @@ public void testGenerateRoutingPathFromDynamicTemplate_templateWithNoPathMatch() } """; Settings result = generateTsdbSettings(mapping, now); - assertThat(result.size(), equalTo(INDEX_DIMENSIONS_TSID_OPTIMIZATION_FEATURE_FLAG ? 5 : 4)); + assertThat(result.size(), equalTo(4)); assertThat(IndexSettings.MODE.get(result), equalTo(IndexMode.TIME_SERIES)); assertThat(IndexSettings.TIME_SERIES_START_TIME.get(result), equalTo(now.minusMillis(DEFAULT_LOOK_BACK_TIME.getMillis()))); assertThat(IndexSettings.TIME_SERIES_END_TIME.get(result), equalTo(now.plusMillis(DEFAULT_LOOK_AHEAD_TIME.getMillis()))); assertThat(IndexMetadata.INDEX_ROUTING_PATH.get(result), containsInAnyOrder("host.id", "prometheus.labels.*")); + assertThat(IndexMetadata.INDEX_DIMENSIONS.get(result), empty()); + } + + public void testGenerateNonDimensionDynamicTemplate() throws Exception { + Instant now = Instant.now().truncatedTo(ChronoUnit.SECONDS); + String mapping = """ + { + "_doc": { + "dynamic_templates": [ + { + "strings_as_keyword": { + "match_mapping_type": "string", + "mapping": { + "type": "keyword", + "ignore_above": 1024, + "time_series_dimension": false + } + } + } + ], + "properties": { + "host.id": { + "type": "keyword", + "time_series_dimension": true + }, + "another_field": { + "type": "keyword" + } + } + } + } + """; + Settings result = generateTsdbSettings(mapping, now); + assertThat(result.size(), equalTo(INDEX_DIMENSIONS_TSID_OPTIMIZATION_FEATURE_FLAG ? 5 : 4)); + assertThat(IndexSettings.MODE.get(result), equalTo(IndexMode.TIME_SERIES)); + assertThat(IndexSettings.TIME_SERIES_START_TIME.get(result), equalTo(now.minusMillis(DEFAULT_LOOK_BACK_TIME.getMillis()))); + assertThat(IndexSettings.TIME_SERIES_END_TIME.get(result), equalTo(now.plusMillis(DEFAULT_LOOK_AHEAD_TIME.getMillis()))); + assertThat(IndexMetadata.INDEX_ROUTING_PATH.get(result), containsInAnyOrder("host.id")); if (INDEX_DIMENSIONS_TSID_OPTIMIZATION_FEATURE_FLAG) { - assertThat(IndexMetadata.INDEX_DIMENSIONS.get(result), containsInAnyOrder("host.id", "prometheus.labels.*")); + assertThat(IndexMetadata.INDEX_DIMENSIONS.get(result), containsInAnyOrder("host.id")); } else { assertThat(IndexMetadata.INDEX_DIMENSIONS.get(result), empty()); } @@ -742,11 +762,7 @@ public void testGenerateRoutingPathFromDynamicTemplate_nonKeywordTemplate() thro Settings result = generateTsdbSettings(mapping, now); assertThat(IndexSettings.TIME_SERIES_START_TIME.get(result), equalTo(now.minusMillis(DEFAULT_LOOK_BACK_TIME.getMillis()))); assertThat(IndexSettings.TIME_SERIES_END_TIME.get(result), equalTo(now.plusMillis(DEFAULT_LOOK_AHEAD_TIME.getMillis()))); - if (INDEX_DIMENSIONS_TSID_OPTIMIZATION_FEATURE_FLAG) { - assertThat(IndexMetadata.INDEX_DIMENSIONS.get(result), containsInAnyOrder("host.id", "prometheus.labels.*")); - } else { - assertThat(IndexMetadata.INDEX_DIMENSIONS.get(result), empty()); - } + assertThat(IndexMetadata.INDEX_DIMENSIONS.get(result), empty()); } public void testGenerateRoutingPathFromPassThroughObject() throws Exception { @@ -789,6 +805,44 @@ public void testGenerateRoutingPathFromPassThroughObject() throws Exception { } } + public void testDynamicTemplatePrecedence() throws Exception { + Instant now = Instant.now().truncatedTo(ChronoUnit.SECONDS); + String mapping = """ + { + "_doc": { + "dynamic_templates": [ + { + "no_dimension_labels": { + "path_match": "labels.host_ip", + "mapping": { + "type": "keyword" + } + } + }, + { + "labels": { + "path_match": "labels.*", + "mapping": { + "type": "keyword", + "time_series_dimension": true + } + } + } + ] + } + } + """; + Settings result = generateTsdbSettings(mapping, now); + assertThat(result.size(), equalTo(4)); + assertThat(IndexSettings.MODE.get(result), equalTo(IndexMode.TIME_SERIES)); + assertThat(IndexSettings.TIME_SERIES_START_TIME.get(result), equalTo(now.minusMillis(DEFAULT_LOOK_BACK_TIME.getMillis()))); + assertThat(IndexSettings.TIME_SERIES_END_TIME.get(result), equalTo(now.plusMillis(DEFAULT_LOOK_AHEAD_TIME.getMillis()))); + // labels.host_ip is not a dimension because it matches the first template which does not have time_series_dimension:true + // we can't use index.dimensions as it would add non-dimension fields to the tsid + assertThat(IndexMetadata.INDEX_DIMENSIONS.get(result), empty()); + assertThat(IndexMetadata.INDEX_ROUTING_PATH.get(result), containsInAnyOrder("labels.*")); + } + public void testAddNewDimension() throws Exception { String newMapping = """ { @@ -888,7 +942,7 @@ public void testAddDynamicTemplate() throws Exception { } } """; - // the new labels.label1 field already matches labels.*, so no change + // we don't support index.dimensions with dynamic templates so we'll unset index.dimensions Settings result = onUpdateMappings("labels.*", "labels.*", mapping); assertThat(result.size(), equalTo(1)); assertThat(IndexMetadata.INDEX_DIMENSIONS.get(result), empty());