From cd09cb7ede2cbe1c4373640e696facab010478d9 Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Thu, 29 Aug 2024 14:12:43 +0200 Subject: [PATCH] Allow dimension fields to have multiple values in standard and logsdb index mode Fixes https://github.com/elastic/elasticsearch/issues/112232 Fixes https://github.com/elastic/elasticsearch/issues/112239 --- .../org/elasticsearch/index/IndexMode.java | 4 +- .../index/mapper/DocumentDimensions.java | 43 +++++++++++++++ .../index/mapper/BooleanFieldMapperTests.java | 19 +++++-- .../index/mapper/IpFieldMapperTests.java | 26 ++++++--- .../index/mapper/KeywordFieldMapperTests.java | 19 +++++-- .../flattened/FlattenedFieldMapperTests.java | 23 +++++--- .../index/mapper/MapperServiceTestCase.java | 8 +++ .../mapper/WholeNumberFieldMapperTests.java | 27 +++++++--- .../UnsignedLongFieldMapperTests.java | 25 ++++++--- .../rest-api-spec/test/20_logs.tests.yml | 22 -------- .../rest-api-spec/test/20_logs_tests.yml | 53 +++++++++++++++++++ 11 files changed, 206 insertions(+), 63 deletions(-) delete mode 100644 x-pack/plugin/otel-data/src/yamlRestTest/resources/rest-api-spec/test/20_logs.tests.yml create mode 100644 x-pack/plugin/otel-data/src/yamlRestTest/resources/rest-api-spec/test/20_logs_tests.yml diff --git a/server/src/main/java/org/elasticsearch/index/IndexMode.java b/server/src/main/java/org/elasticsearch/index/IndexMode.java index b137cfe27a514..d05ba7c5956a6 100644 --- a/server/src/main/java/org/elasticsearch/index/IndexMode.java +++ b/server/src/main/java/org/elasticsearch/index/IndexMode.java @@ -105,7 +105,7 @@ public IdFieldMapper buildIdFieldMapper(BooleanSupplier fieldDataEnabled) { @Override public DocumentDimensions buildDocumentDimensions(IndexSettings settings) { - return new DocumentDimensions.OnlySingleValueAllowed(); + return DocumentDimensions.Noop.INSTANCE; } @Override @@ -279,7 +279,7 @@ public MetadataFieldMapper timeSeriesRoutingHashFieldMapper() { @Override public DocumentDimensions buildDocumentDimensions(IndexSettings settings) { - return new DocumentDimensions.OnlySingleValueAllowed(); + return DocumentDimensions.Noop.INSTANCE; } @Override diff --git a/server/src/main/java/org/elasticsearch/index/mapper/DocumentDimensions.java b/server/src/main/java/org/elasticsearch/index/mapper/DocumentDimensions.java index aa69e4db50e76..68d8d229d66c7 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/DocumentDimensions.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/DocumentDimensions.java @@ -104,4 +104,47 @@ private void add(String fieldName) { } } }; + + /** + * Noop implementation that doesn't perform validations on dimension fields + */ + enum Noop implements DocumentDimensions { + + INSTANCE; + + @Override + public DocumentDimensions addString(String fieldName, BytesRef utf8Value) { + return this; + } + + @Override + public DocumentDimensions addString(String fieldName, String value) { + return this; + } + + @Override + public DocumentDimensions addIp(String fieldName, InetAddress value) { + return this; + } + + @Override + public DocumentDimensions addLong(String fieldName, long value) { + return this; + } + + @Override + public DocumentDimensions addUnsignedLong(String fieldName, long value) { + return this; + } + + @Override + public DocumentDimensions addBoolean(String fieldName, boolean value) { + return this; + } + + @Override + public DocumentDimensions validate(IndexSettings settings) { + return this; + } + } } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/BooleanFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/BooleanFieldMapperTests.java index e08a443bd74cb..de17f362caade 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/BooleanFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/BooleanFieldMapperTests.java @@ -17,6 +17,7 @@ import org.elasticsearch.common.Strings; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.core.Tuple; +import org.elasticsearch.index.IndexMode; import org.elasticsearch.index.IndexSettings; import org.elasticsearch.index.IndexVersion; import org.elasticsearch.script.BooleanFieldScript; @@ -25,6 +26,7 @@ import org.elasticsearch.xcontent.XContentFactory; import java.io.IOException; +import java.time.Instant; import java.util.List; import java.util.function.Function; @@ -257,15 +259,24 @@ public void testDimensionIndexedAndDocvalues() { } } - public void testDimensionMultiValuedField() throws IOException { + public void testDimensionMultiValuedField() throws Throwable { XContentBuilder mapping = fieldMapping(b -> { minimalMapping(b); b.field("time_series_dimension", true); }); - DocumentMapper mapper = randomBoolean() ? createDocumentMapper(mapping) : createTimeSeriesModeDocumentMapper(mapping); + IndexMode indexMode = randomFrom(IndexMode.values()); + DocumentMapper mapper = createDocumentMapper(mapping, indexMode); - Exception e = expectThrows(DocumentParsingException.class, () -> mapper.parse(source(b -> b.array("field", true, false)))); - assertThat(e.getCause().getMessage(), containsString("Dimension field [field] cannot be a multi-valued field")); + ThrowingRunnable parseArray = () -> mapper.parse(source(b -> { + b.array("field", true, false); + b.field("@timestamp", Instant.now()); + })); + if (indexMode == IndexMode.TIME_SERIES) { + Exception e = expectThrows(DocumentParsingException.class, parseArray); + assertThat(e.getCause().getMessage(), containsString("Dimension field [field] cannot be a multi-valued field")); + } else { + parseArray.run(); + } } public void testDimensionInRoutingPath() throws IOException { diff --git a/server/src/test/java/org/elasticsearch/index/mapper/IpFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/IpFieldMapperTests.java index ba9c2e6c4a299..97149ac5726c7 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/IpFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/IpFieldMapperTests.java @@ -18,6 +18,7 @@ import org.elasticsearch.common.network.InetAddresses; import org.elasticsearch.common.network.NetworkAddress; import org.elasticsearch.core.Tuple; +import org.elasticsearch.index.IndexMode; import org.elasticsearch.index.IndexVersion; import org.elasticsearch.index.IndexVersions; import org.elasticsearch.script.IpFieldScript; @@ -26,6 +27,7 @@ import java.io.IOException; import java.net.InetAddress; +import java.time.Instant; import java.util.ArrayList; import java.util.List; import java.util.function.Function; @@ -255,17 +257,25 @@ public void testDimensionIndexedAndDocvalues() { } } - public void testDimensionMultiValuedField() throws IOException { - DocumentMapper mapper = createDocumentMapper(fieldMapping(b -> { + public void testDimensionMultiValuedField() throws Throwable { + XContentBuilder mapping = fieldMapping(b -> { minimalMapping(b); b.field("time_series_dimension", true); - })); + }); - Exception e = expectThrows( - DocumentParsingException.class, - () -> mapper.parse(source(b -> b.array("field", "192.168.1.1", "192.168.1.1"))) - ); - assertThat(e.getCause().getMessage(), containsString("Dimension field [field] cannot be a multi-valued field")); + IndexMode indexMode = randomFrom(IndexMode.values()); + DocumentMapper mapper = createDocumentMapper(mapping, indexMode); + + ThrowingRunnable parseArray = () -> mapper.parse(source(b -> { + b.array("field", "192.168.1.1", "192.168.1.1"); + b.field("@timestamp", Instant.now()); + })); + if (indexMode == IndexMode.TIME_SERIES) { + Exception e = expectThrows(DocumentParsingException.class, parseArray); + assertThat(e.getCause().getMessage(), containsString("Dimension field [field] cannot be a multi-valued field")); + } else { + parseArray.run(); + } } @Override diff --git a/server/src/test/java/org/elasticsearch/index/mapper/KeywordFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/KeywordFieldMapperTests.java index 833b0a60827d0..70f05606d7bb9 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/KeywordFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/KeywordFieldMapperTests.java @@ -25,6 +25,7 @@ import org.elasticsearch.common.Strings; import org.elasticsearch.common.lucene.Lucene; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.index.IndexMode; import org.elasticsearch.index.IndexSettings; import org.elasticsearch.index.IndexVersion; import org.elasticsearch.index.analysis.AnalyzerScope; @@ -44,6 +45,7 @@ import org.elasticsearch.xcontent.XContentBuilder; import java.io.IOException; +import java.time.Instant; import java.util.Arrays; import java.util.Collection; import java.util.List; @@ -373,15 +375,24 @@ public void testDimensionIndexedAndDocvalues() { } } - public void testDimensionMultiValuedField() throws IOException { + public void testDimensionMultiValuedField() throws Throwable { XContentBuilder mapping = fieldMapping(b -> { minimalMapping(b); b.field("time_series_dimension", true); }); - DocumentMapper mapper = randomBoolean() ? createDocumentMapper(mapping) : createTimeSeriesModeDocumentMapper(mapping); + IndexMode indexMode = randomFrom(IndexMode.values()); + DocumentMapper mapper = createDocumentMapper(mapping, indexMode); - Exception e = expectThrows(DocumentParsingException.class, () -> mapper.parse(source(b -> b.array("field", "1234", "45678")))); - assertThat(e.getCause().getMessage(), containsString("Dimension field [field] cannot be a multi-valued field")); + ThrowingRunnable parseArray = () -> mapper.parse(source(b -> { + b.array("field", "1234", "45678"); + b.field("@timestamp", Instant.now()); + })); + if (indexMode == IndexMode.TIME_SERIES) { + Exception e = expectThrows(DocumentParsingException.class, parseArray); + assertThat(e.getCause().getMessage(), containsString("Dimension field [field] cannot be a multi-valued field")); + } else { + parseArray.run(); + } } public void testDimensionExtraLongKeyword() throws IOException { diff --git a/server/src/test/java/org/elasticsearch/index/mapper/flattened/FlattenedFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/flattened/FlattenedFieldMapperTests.java index aba20ec5d81c8..077c8b26867fc 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/flattened/FlattenedFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/flattened/FlattenedFieldMapperTests.java @@ -15,6 +15,7 @@ import org.elasticsearch.common.bytes.BytesArray; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.index.IndexMode; import org.elasticsearch.index.IndexSettings; import org.elasticsearch.index.IndexVersion; import org.elasticsearch.index.mapper.DocumentMapper; @@ -34,6 +35,7 @@ import org.junit.AssumptionViolatedException; import java.io.IOException; +import java.time.Instant; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -189,18 +191,25 @@ public void testDimensionIndexedAndDocvalues() { } } - public void testDimensionMultiValuedField() throws IOException { + public void testDimensionMultiValuedField() throws Throwable { XContentBuilder mapping = fieldMapping(b -> { minimalMapping(b); b.field("time_series_dimensions", List.of("key1", "key2", "field3.key3")); }); - DocumentMapper mapper = randomBoolean() ? createDocumentMapper(mapping) : createTimeSeriesModeDocumentMapper(mapping); - Exception e = expectThrows( - DocumentParsingException.class, - () -> mapper.parse(source(b -> b.array("field.key1", "value1", "value2"))) - ); - assertThat(e.getCause().getMessage(), containsString("Dimension field [field.key1] cannot be a multi-valued field")); + IndexMode indexMode = randomFrom(IndexMode.values()); + DocumentMapper mapper = createDocumentMapper(mapping, indexMode); + + ThrowingRunnable parseArray = () -> mapper.parse(source(b -> { + b.array("field.key1", "value1", "value2"); + b.field("@timestamp", Instant.now()); + })); + if (indexMode == IndexMode.TIME_SERIES) { + Exception e = expectThrows(DocumentParsingException.class, parseArray); + assertThat(e.getCause().getMessage(), containsString("Dimension field [field.key1] cannot be a multi-valued field")); + } else { + parseArray.run(); + } } public void testDisableIndex() throws Exception { diff --git a/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperServiceTestCase.java b/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperServiceTestCase.java index 7c11e7446e5c5..235bb7208fb08 100644 --- a/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperServiceTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperServiceTestCase.java @@ -139,6 +139,14 @@ protected static String randomIndexOptions() { return randomFrom("docs", "freqs", "positions", "offsets"); } + protected final DocumentMapper createDocumentMapper(XContentBuilder mappings, IndexMode indexMode) throws IOException { + return switch (indexMode) { + case STANDARD -> createDocumentMapper(mappings); + case TIME_SERIES -> createTimeSeriesModeDocumentMapper(mappings); + case LOGSDB -> createLogsModeDocumentMapper(mappings); + }; + } + protected final DocumentMapper createDocumentMapper(XContentBuilder mappings) throws IOException { return createMapperService(mappings).documentMapper(); } diff --git a/test/framework/src/main/java/org/elasticsearch/index/mapper/WholeNumberFieldMapperTests.java b/test/framework/src/main/java/org/elasticsearch/index/mapper/WholeNumberFieldMapperTests.java index 99c500639bdde..7ef0b9de6d030 100644 --- a/test/framework/src/main/java/org/elasticsearch/index/mapper/WholeNumberFieldMapperTests.java +++ b/test/framework/src/main/java/org/elasticsearch/index/mapper/WholeNumberFieldMapperTests.java @@ -9,8 +9,11 @@ package org.elasticsearch.index.mapper; import org.apache.lucene.index.IndexableField; +import org.elasticsearch.index.IndexMode; +import org.elasticsearch.xcontent.XContentBuilder; import java.io.IOException; +import java.time.Instant; import java.util.List; import static org.hamcrest.Matchers.containsString; @@ -69,17 +72,25 @@ public void testDimensionIndexedAndDocvalues() { } } - public void testDimensionMultiValuedField() throws IOException { - DocumentMapper mapper = createDocumentMapper(fieldMapping(b -> { + public void testDimensionMultiValuedField() throws Throwable { + XContentBuilder mapping = fieldMapping(b -> { minimalMapping(b); b.field("time_series_dimension", true); - })); + }); - Exception e = expectThrows( - DocumentParsingException.class, - () -> mapper.parse(source(b -> b.array("field", randomNumber(), randomNumber(), randomNumber()))) - ); - assertThat(e.getCause().getMessage(), containsString("Dimension field [field] cannot be a multi-valued field")); + IndexMode indexMode = randomFrom(IndexMode.values()); + DocumentMapper mapper = createDocumentMapper(mapping, indexMode); + + ThrowingRunnable parseArray = () -> mapper.parse(source(b -> { + b.array("field", randomNumber(), randomNumber(), randomNumber()); + b.field("@timestamp", Instant.now()); + })); + if (indexMode == IndexMode.TIME_SERIES) { + Exception e = expectThrows(DocumentParsingException.class, parseArray); + assertThat(e.getCause().getMessage(), containsString("Dimension field [field] cannot be a multi-valued field")); + } else { + parseArray.run(); + } } public void testMetricAndDimension() { diff --git a/x-pack/plugin/mapper-unsigned-long/src/test/java/org/elasticsearch/xpack/unsignedlong/UnsignedLongFieldMapperTests.java b/x-pack/plugin/mapper-unsigned-long/src/test/java/org/elasticsearch/xpack/unsignedlong/UnsignedLongFieldMapperTests.java index 753440cb0b789..f303a652dfeac 100644 --- a/x-pack/plugin/mapper-unsigned-long/src/test/java/org/elasticsearch/xpack/unsignedlong/UnsignedLongFieldMapperTests.java +++ b/x-pack/plugin/mapper-unsigned-long/src/test/java/org/elasticsearch/xpack/unsignedlong/UnsignedLongFieldMapperTests.java @@ -28,6 +28,7 @@ import java.io.IOException; import java.math.BigInteger; +import java.time.Instant; import java.util.Collection; import java.util.Collections; import java.util.List; @@ -260,17 +261,25 @@ public void testDimensionIndexedAndDocvalues() { } } - public void testDimensionMultiValuedField() throws IOException { - DocumentMapper mapper = createDocumentMapper(fieldMapping(b -> { + public void testDimensionMultiValuedField() throws Throwable { + XContentBuilder mapping = fieldMapping(b -> { minimalMapping(b); b.field("time_series_dimension", true); - })); + }); - Exception e = expectThrows( - DocumentParsingException.class, - () -> mapper.parse(source(b -> b.array("field", randomNonNegativeLong(), randomNonNegativeLong(), randomNonNegativeLong()))) - ); - assertThat(e.getCause().getMessage(), containsString("Dimension field [field] cannot be a multi-valued field")); + IndexMode indexMode = randomFrom(IndexMode.values()); + DocumentMapper mapper = createDocumentMapper(mapping, indexMode); + + ThrowingRunnable parseArray = () -> mapper.parse(source(b -> { + b.array("field", randomNonNegativeLong(), randomNonNegativeLong(), randomNonNegativeLong()); + b.field("@timestamp", Instant.now()); + })); + if (indexMode == IndexMode.TIME_SERIES) { + Exception e = expectThrows(DocumentParsingException.class, parseArray); + assertThat(e.getCause().getMessage(), containsString("Dimension field [field] cannot be a multi-valued field")); + } else { + parseArray.run(); + } } public void testMetricType() throws IOException { diff --git a/x-pack/plugin/otel-data/src/yamlRestTest/resources/rest-api-spec/test/20_logs.tests.yml b/x-pack/plugin/otel-data/src/yamlRestTest/resources/rest-api-spec/test/20_logs.tests.yml deleted file mode 100644 index d87c2a80deab8..0000000000000 --- a/x-pack/plugin/otel-data/src/yamlRestTest/resources/rest-api-spec/test/20_logs.tests.yml +++ /dev/null @@ -1,22 +0,0 @@ ---- -setup: - - do: - cluster.health: - wait_for_events: languid ---- -"Default data_stream.type must be logs": - - do: - bulk: - index: logs-generic.otel-default - refresh: true - body: - - create: {} - - '{"@timestamp":"2024-07-18T14:48:33.467654000Z","data_stream":{"dataset":"generic.otel","namespace":"default"}, "attributes": { "foo": "bar"}, "body_text":"Error: Unable to connect to the database.","severity_text":"ERROR","severity_number":3,"trace_id":"abc123xyz456def789ghi012jkl345"}' - - is_false: errors - - do: - search: - index: logs-generic.otel-default - body: - fields: ["data_stream.type"] - - length: { hits.hits: 1 } - - match: { hits.hits.0.fields.data_stream\.type: ["logs"] } diff --git a/x-pack/plugin/otel-data/src/yamlRestTest/resources/rest-api-spec/test/20_logs_tests.yml b/x-pack/plugin/otel-data/src/yamlRestTest/resources/rest-api-spec/test/20_logs_tests.yml new file mode 100644 index 0000000000000..b0cf92b87667c --- /dev/null +++ b/x-pack/plugin/otel-data/src/yamlRestTest/resources/rest-api-spec/test/20_logs_tests.yml @@ -0,0 +1,53 @@ +--- +setup: + - do: + cluster.health: + wait_for_events: languid +--- +"Default data_stream.type must be logs": + - do: + bulk: + index: logs-generic.otel-default + refresh: true + body: + - create: {} + - '{"@timestamp":"2024-07-18T14:48:33.467654000Z","data_stream":{"dataset":"generic.otel","namespace":"default"}, "attributes": { "foo": "bar"}, "body_text":"Error: Unable to connect to the database.","severity_text":"ERROR","severity_number":3,"trace_id":"abc123xyz456def789ghi012jkl345"}' + - is_false: errors + - do: + search: + index: logs-generic.otel-default + body: + fields: ["data_stream.type"] + - length: { hits.hits: 1 } + - match: { hits.hits.0.fields.data_stream\.type: ["logs"] } +--- +"Multi value fields": + - do: + bulk: + index: logs-generic.otel-default + refresh: true + body: + - create: {} + - "@timestamp": 2024-07-18T14:48:33.467654000Z + data_stream: + type: logs + dataset: generic.otel + namespace: default + resource: + attributes: + host.ip: ["127.0.0.1", "0.0.0.0"] + attributes: + foo: [3, 2, 1] + bar: [b, c, a] + body_text: "Error: Unable to connect to the database." + severity_text: ERROR + - is_false: errors + - do: + search: + index: logs-generic.otel-default + body: + fields: ["*"] + - length: { hits.hits: 1 } + - match: { hits.hits.0.fields.resource\.attributes\.host\.ip: ["0.0.0.0", "127.0.0.1"] } + - match: { hits.hits.0.fields.attributes\.foo: [1, 2, 3] } + - match: { hits.hits.0.fields.attributes\.bar: [a, b, c] }