Skip to content

Commit 75901a1

Browse files
felixbarnyfelixbarny
authored
Don't set index.dimensions if dimensions are added via dynamic templates (elastic#135212)
1 parent bf486c1 commit 75901a1

File tree

3 files changed

+99
-43
lines changed

3 files changed

+99
-43
lines changed

‎modules/data-streams/src/javaRestTest/java/org/elasticsearch/datastreams/TsdbDataStreamRestIT.java‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -436,10 +436,8 @@ public void testSimulateTsdbDataStreamTemplate() throws Exception {
436436
ObjectPath.evaluate(responseBody, "template.settings.index.routing_path"),
437437
containsInAnyOrder("metricset", "k8s.pod.uid", "pod.labels.*")
438438
);
439-
assertThat(
440-
ObjectPath.evaluate(responseBody, "template.settings.index.dimensions"),
441-
containsInAnyOrder("metricset", "k8s.pod.uid", "pod.labels.*")
442-
);
439+
// not supported when using a dynamic template for time_series_dimension
440+
assertThat(ObjectPath.evaluate(responseBody, "template.settings.index.dimensions"), nullValue());
443441
assertThat(ObjectPath.evaluate(responseBody, "overlapping"), empty());
444442
}
445443

‎modules/data-streams/src/main/java/org/elasticsearch/datastreams/DataStreamIndexSettingsProvider.java‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -254,13 +254,17 @@ private static boolean findDimensionFields(List<String> dimensions, DocumentMapp
254254
if (template.isTimeSeriesDimension() == false) {
255255
continue;
256256
}
257-
if (template.isSimplePathMatch() == false) {
258-
// If the template is not using a simple path match, the dimensions list can't match all potential dimensions.
259-
// For example, if the dynamic template matches by mapping type (all strings are mapped as dimensions),
260-
// the coordinating node can't rely on the dimensions list to match all dimensions.
261-
// In this case, the index.routing_path setting will be used instead.
262-
matchesAllDimensions = false;
263-
}
257+
// At this point, we don't support index.dimensions when dimensions are mapped via a dynamic template.
258+
// This is because more specific matches with a higher priority can exist that exclude certain fields from being mapped as a
259+
// dimension. For example:
260+
// - path_match: "labels.host_ip", time_series_dimension: false
261+
// - path_match: "labels.*", time_series_dimension: true
262+
// In this case, "labels.host_ip" is not a dimension,
263+
// and adding labels.* to index.dimensions would lead to non-dimension fields being included in the tsid.
264+
// Therefore, we fall back to using index.routing_path.
265+
// While this also may include non-dimension fields in the routing path,
266+
// it at least guarantees that the tsid only includes dimension fields and includes all dimension fields.
267+
matchesAllDimensions = false;
264268
if (template.pathMatch().isEmpty() == false) {
265269
dimensions.addAll(template.pathMatch());
266270
}

‎modules/data-streams/src/test/java/org/elasticsearch/datastreams/DataStreamIndexSettingsProviderTests.java‎

Lines changed: 86 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -525,16 +525,12 @@ public void testGenerateRoutingPathFromDynamicTemplate() throws Exception {
525525
}
526526
""";
527527
Settings result = generateTsdbSettings(mapping, now);
528-
assertThat(result.size(), equalTo(INDEX_DIMENSIONS_TSID_OPTIMIZATION_FEATURE_FLAG ? 5 : 4));
528+
assertThat(result.size(), equalTo(4));
529529
assertThat(IndexSettings.MODE.get(result), equalTo(IndexMode.TIME_SERIES));
530530
assertThat(IndexSettings.TIME_SERIES_START_TIME.get(result), equalTo(now.minusMillis(DEFAULT_LOOK_BACK_TIME.getMillis())));
531531
assertThat(IndexSettings.TIME_SERIES_END_TIME.get(result), equalTo(now.plusMillis(DEFAULT_LOOK_AHEAD_TIME.getMillis())));
532532
assertThat(IndexMetadata.INDEX_ROUTING_PATH.get(result), containsInAnyOrder("host.id", "prometheus.labels.*"));
533-
if (INDEX_DIMENSIONS_TSID_OPTIMIZATION_FEATURE_FLAG) {
534-
assertThat(IndexMetadata.INDEX_DIMENSIONS.get(result), containsInAnyOrder("host.id", "prometheus.labels.*"));
535-
} else {
536-
assertThat(IndexMetadata.INDEX_DIMENSIONS.get(result), empty());
537-
}
533+
assertThat(IndexMetadata.INDEX_DIMENSIONS.get(result), empty());
538534
}
539535

540536
public void testGenerateRoutingPathFromDynamicTemplateWithMultiplePathMatchEntries() throws Exception {
@@ -570,22 +566,15 @@ public void testGenerateRoutingPathFromDynamicTemplateWithMultiplePathMatchEntri
570566
}
571567
""";
572568
Settings result = generateTsdbSettings(mapping, now);
573-
assertThat(result.size(), equalTo(INDEX_DIMENSIONS_TSID_OPTIMIZATION_FEATURE_FLAG ? 5 : 4));
569+
assertThat(result.size(), equalTo(4));
574570
assertThat(IndexSettings.MODE.get(result), equalTo(IndexMode.TIME_SERIES));
575571
assertThat(IndexSettings.TIME_SERIES_START_TIME.get(result), equalTo(now.minusMillis(DEFAULT_LOOK_BACK_TIME.getMillis())));
576572
assertThat(IndexSettings.TIME_SERIES_END_TIME.get(result), equalTo(now.plusMillis(DEFAULT_LOOK_AHEAD_TIME.getMillis())));
577573
assertThat(
578574
IndexMetadata.INDEX_ROUTING_PATH.get(result),
579575
containsInAnyOrder("host.id", "xprometheus.labels.*", "yprometheus.labels.*")
580576
);
581-
if (INDEX_DIMENSIONS_TSID_OPTIMIZATION_FEATURE_FLAG) {
582-
assertThat(
583-
IndexMetadata.INDEX_DIMENSIONS.get(result),
584-
containsInAnyOrder("host.id", "xprometheus.labels.*", "yprometheus.labels.*")
585-
);
586-
} else {
587-
assertThat(IndexMetadata.INDEX_DIMENSIONS.get(result), empty());
588-
}
577+
assertThat(IndexMetadata.INDEX_DIMENSIONS.get(result), empty());
589578
}
590579

591580
public void testGenerateRoutingPathFromDynamicTemplateWithMultiplePathMatchEntriesMultiFields() throws Exception {
@@ -626,22 +615,15 @@ public void testGenerateRoutingPathFromDynamicTemplateWithMultiplePathMatchEntri
626615
}
627616
""";
628617
Settings result = generateTsdbSettings(mapping, now);
629-
assertThat(result.size(), equalTo(INDEX_DIMENSIONS_TSID_OPTIMIZATION_FEATURE_FLAG ? 5 : 4));
618+
assertThat(result.size(), equalTo(4));
630619
assertThat(IndexSettings.MODE.get(result), equalTo(IndexMode.TIME_SERIES));
631620
assertThat(IndexSettings.TIME_SERIES_START_TIME.get(result), equalTo(now.minusMillis(DEFAULT_LOOK_BACK_TIME.getMillis())));
632621
assertThat(IndexSettings.TIME_SERIES_END_TIME.get(result), equalTo(now.plusMillis(DEFAULT_LOOK_AHEAD_TIME.getMillis())));
633622
assertThat(
634623
IndexMetadata.INDEX_ROUTING_PATH.get(result),
635624
containsInAnyOrder("host.id", "xprometheus.labels.*", "yprometheus.labels.*")
636625
);
637-
if (INDEX_DIMENSIONS_TSID_OPTIMIZATION_FEATURE_FLAG) {
638-
assertThat(
639-
IndexMetadata.INDEX_DIMENSIONS.get(result),
640-
containsInAnyOrder("host.id", "xprometheus.labels.*", "yprometheus.labels.*")
641-
);
642-
} else {
643-
assertThat(IndexMetadata.INDEX_DIMENSIONS.get(result), empty());
644-
}
626+
assertThat(IndexMetadata.INDEX_DIMENSIONS.get(result), empty());
645627
}
646628

647629
public void testGenerateRoutingPathFromDynamicTemplate_templateWithNoPathMatch() throws Exception {
@@ -686,13 +668,51 @@ public void testGenerateRoutingPathFromDynamicTemplate_templateWithNoPathMatch()
686668
}
687669
""";
688670
Settings result = generateTsdbSettings(mapping, now);
689-
assertThat(result.size(), equalTo(INDEX_DIMENSIONS_TSID_OPTIMIZATION_FEATURE_FLAG ? 5 : 4));
671+
assertThat(result.size(), equalTo(4));
690672
assertThat(IndexSettings.MODE.get(result), equalTo(IndexMode.TIME_SERIES));
691673
assertThat(IndexSettings.TIME_SERIES_START_TIME.get(result), equalTo(now.minusMillis(DEFAULT_LOOK_BACK_TIME.getMillis())));
692674
assertThat(IndexSettings.TIME_SERIES_END_TIME.get(result), equalTo(now.plusMillis(DEFAULT_LOOK_AHEAD_TIME.getMillis())));
693675
assertThat(IndexMetadata.INDEX_ROUTING_PATH.get(result), containsInAnyOrder("host.id", "prometheus.labels.*"));
676+
assertThat(IndexMetadata.INDEX_DIMENSIONS.get(result), empty());
677+
}
678+
679+
public void testGenerateNonDimensionDynamicTemplate() throws Exception {
680+
Instant now = Instant.now().truncatedTo(ChronoUnit.SECONDS);
681+
String mapping = """
682+
{
683+
"_doc": {
684+
"dynamic_templates": [
685+
{
686+
"strings_as_keyword": {
687+
"match_mapping_type": "string",
688+
"mapping": {
689+
"type": "keyword",
690+
"ignore_above": 1024,
691+
"time_series_dimension": false
692+
}
693+
}
694+
}
695+
],
696+
"properties": {
697+
"host.id": {
698+
"type": "keyword",
699+
"time_series_dimension": true
700+
},
701+
"another_field": {
702+
"type": "keyword"
703+
}
704+
}
705+
}
706+
}
707+
""";
708+
Settings result = generateTsdbSettings(mapping, now);
709+
assertThat(result.size(), equalTo(INDEX_DIMENSIONS_TSID_OPTIMIZATION_FEATURE_FLAG ? 5 : 4));
710+
assertThat(IndexSettings.MODE.get(result), equalTo(IndexMode.TIME_SERIES));
711+
assertThat(IndexSettings.TIME_SERIES_START_TIME.get(result), equalTo(now.minusMillis(DEFAULT_LOOK_BACK_TIME.getMillis())));
712+
assertThat(IndexSettings.TIME_SERIES_END_TIME.get(result), equalTo(now.plusMillis(DEFAULT_LOOK_AHEAD_TIME.getMillis())));
713+
assertThat(IndexMetadata.INDEX_ROUTING_PATH.get(result), containsInAnyOrder("host.id"));
694714
if (INDEX_DIMENSIONS_TSID_OPTIMIZATION_FEATURE_FLAG) {
695-
assertThat(IndexMetadata.INDEX_DIMENSIONS.get(result), containsInAnyOrder("host.id", "prometheus.labels.*"));
715+
assertThat(IndexMetadata.INDEX_DIMENSIONS.get(result), containsInAnyOrder("host.id"));
696716
} else {
697717
assertThat(IndexMetadata.INDEX_DIMENSIONS.get(result), empty());
698718
}
@@ -742,11 +762,7 @@ public void testGenerateRoutingPathFromDynamicTemplate_nonKeywordTemplate() thro
742762
Settings result = generateTsdbSettings(mapping, now);
743763
assertThat(IndexSettings.TIME_SERIES_START_TIME.get(result), equalTo(now.minusMillis(DEFAULT_LOOK_BACK_TIME.getMillis())));
744764
assertThat(IndexSettings.TIME_SERIES_END_TIME.get(result), equalTo(now.plusMillis(DEFAULT_LOOK_AHEAD_TIME.getMillis())));
745-
if (INDEX_DIMENSIONS_TSID_OPTIMIZATION_FEATURE_FLAG) {
746-
assertThat(IndexMetadata.INDEX_DIMENSIONS.get(result), containsInAnyOrder("host.id", "prometheus.labels.*"));
747-
} else {
748-
assertThat(IndexMetadata.INDEX_DIMENSIONS.get(result), empty());
749-
}
765+
assertThat(IndexMetadata.INDEX_DIMENSIONS.get(result), empty());
750766
}
751767

752768
public void testGenerateRoutingPathFromPassThroughObject() throws Exception {
@@ -789,6 +805,44 @@ public void testGenerateRoutingPathFromPassThroughObject() throws Exception {
789805
}
790806
}
791807

808+
public void testDynamicTemplatePrecedence() throws Exception {
809+
Instant now = Instant.now().truncatedTo(ChronoUnit.SECONDS);
810+
String mapping = """
811+
{
812+
"_doc": {
813+
"dynamic_templates": [
814+
{
815+
"no_dimension_labels": {
816+
"path_match": "labels.host_ip",
817+
"mapping": {
818+
"type": "keyword"
819+
}
820+
}
821+
},
822+
{
823+
"labels": {
824+
"path_match": "labels.*",
825+
"mapping": {
826+
"type": "keyword",
827+
"time_series_dimension": true
828+
}
829+
}
830+
}
831+
]
832+
}
833+
}
834+
""";
835+
Settings result = generateTsdbSettings(mapping, now);
836+
assertThat(result.size(), equalTo(4));
837+
assertThat(IndexSettings.MODE.get(result), equalTo(IndexMode.TIME_SERIES));
838+
assertThat(IndexSettings.TIME_SERIES_START_TIME.get(result), equalTo(now.minusMillis(DEFAULT_LOOK_BACK_TIME.getMillis())));
839+
assertThat(IndexSettings.TIME_SERIES_END_TIME.get(result), equalTo(now.plusMillis(DEFAULT_LOOK_AHEAD_TIME.getMillis())));
840+
// labels.host_ip is not a dimension because it matches the first template which does not have time_series_dimension:true
841+
// we can't use index.dimensions as it would add non-dimension fields to the tsid
842+
assertThat(IndexMetadata.INDEX_DIMENSIONS.get(result), empty());
843+
assertThat(IndexMetadata.INDEX_ROUTING_PATH.get(result), containsInAnyOrder("labels.*"));
844+
}
845+
792846
public void testAddNewDimension() throws Exception {
793847
String newMapping = """
794848
{
@@ -888,7 +942,7 @@ public void testAddDynamicTemplate() throws Exception {
888942
}
889943
}
890944
""";
891-
// the new labels.label1 field already matches labels.*, so no change
945+
// we don't support index.dimensions with dynamic templates so we'll unset index.dimensions
892946
Settings result = onUpdateMappings("labels.*", "labels.*", mapping);
893947
assertThat(result.size(), equalTo(1));
894948
assertThat(IndexMetadata.INDEX_DIMENSIONS.get(result), empty());

0 commit comments

Comments
 (0)