Composable templates: add a default mapping for @timestamp by andreidan · Pull Request #59244 · elastic/elasticsearch

andreidan · 2020-07-08T17:55:57Z

This adds a low precedence mapping for the @timestamp field with
type date.
This will aid with the bootstrapping of data streams as a timestamp
mapping can be omitted when nanos precision is not needed.

Relates to #53100

This adds a low precendece mapping for the `@timestamp` field with type `date`. This will aid with the bootstrapping of data streams as a timestamp mapping can be omitted when nanos precision is not needed.

elasticmachine · 2020-07-08T17:55:58Z

Pinging @elastic/es-docs (>docs)

elasticmachine · 2020-07-08T17:56:00Z

Pinging @elastic/es-core-features (:Core/Features/Data streams)

andreidan · 2020-07-08T17:56:36Z

rest-api-spec/src/main/resources/rest-api-spec/test/indices.data_stream/10_basic.yml

              properties:
                '@timestamp':
-                  type: date
+                  type: date_nanos


This is changed to exemplify a custom @timestamp mapping

Maybe in the Create data stream yaml test, also check that mapping for the backing index of each data streams to have the correct mapping?

server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateService.java

…ping

jrodewig

Docs LGTM. I made some minor adjustments to fix a merge conflict introduced with #59225.

andreidan · 2020-07-09T07:17:39Z

@elasticmachine update branch

…ping

martijnvg

Thanks! I left a few comments.

martijnvg · 2020-07-09T08:53:06Z

server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateService.java

            .map(Template::mappings)
            .filter(Objects::nonNull)
-            .collect(Collectors.toList());
+            .collect(Collectors.toCollection(LinkedList::new));


why linked list?

because we insert a mapping on the first position below (adding the default @timestamp). That takes linear time with ArrayList as opposed to constant time with the LinkedList. We also don't random access the mappings list but always iterate it on, so it seems like the LinkedList is a better fit for the current use cases.

martijnvg · 2020-07-09T08:56:26Z

server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateService.java

        Optional.ofNullable(template.template())
            .map(Template::mappings)
            .ifPresent(mappings::add);
+        if (template.getDataStreamTemplate() != null) {


I think we need to move this side the body of if (indexName.startsWith(DataStream.BACKING_INDEX_PREFIX)) {,
because this should only be applied when a backing index is created (and otherwise a regular create index call and if a template matches with a 'data_stream' definition would get this mapping applied as well).

martijnvg · 2020-07-09T09:09:32Z

server/src/test/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateServiceTests.java

+            equalTo(Map.of("_doc", Map.of("properties", Map.of("field2", Map.of("type", "integer"))))));
+    }
+
+    public void testUserDefinedMappingTakesPrecedenceOverDefault() throws Exception {


These are good tests. Maybe also add another test that tests that a user adds a mapping for @timestamp in a composable index template?

martijnvg · 2020-07-09T09:12:06Z

docs/reference/data-streams/set-up-a-data-stream.asciidoc

+
+* An optional field mapping for the `@timestamp` field. Both the <<date,`date`>> and 
+<<date_nanos,`date_nanos`>> field data types are supported. If no mapping is specified,
+the <<date,`date`>> field data type is used by default.


maybe include the mapping snippet or that a date field with default options is then used?

martijnvg · 2020-07-09T09:14:52Z

rest-api-spec/src/main/resources/rest-api-spec/test/indices.data_stream/10_basic.yml

              properties:
                '@timestamp':
-                  type: date
+                  type: date_nanos


Maybe in the Create data stream yaml test, also check that mapping for the backing index of each data streams to have the correct mapping?

andreidan · 2020-07-09T12:29:10Z

@elasticmachine run elasticsearch-ci/1

martijnvg

LGTM

…ping

andreidan · 2020-07-13T18:56:00Z

@elasticmachine update branch

…ping

…9244) This adds a low precendece mapping for the `@timestamp` field with type `date`. This will aid with the bootstrapping of data streams as a timestamp mapping can be omitted when nanos precision is not needed. (cherry picked from commit 4e72f43) Signed-off-by: Andrei Dan <andrei.dan@elastic.co>

) (#59510) This adds a low precendece mapping for the `@timestamp` field with type `date`. This will aid with the bootstrapping of data streams as a timestamp mapping can be omitted when nanos precision is not needed. (cherry picked from commit 4e72f43) Signed-off-by: Andrei Dan <andrei.dan@elastic.co>

Composable templates: add a default mapping for @timestamp

c5e73b7

This adds a low precendece mapping for the `@timestamp` field with type `date`. This will aid with the bootstrapping of data streams as a timestamp mapping can be omitted when nanos precision is not needed.

andreidan added >docs General docs changes v8.0.0 :StorageEngine/Data streams Data streams and their lifecycles v7.9.0 labels Jul 8, 2020

elasticmachine added the Team:Docs Meta label for docs team label Jul 8, 2020

elasticmachine added the Team:Data Management (obsolete) DO NOT USE. This team no longer exists. label Jul 8, 2020

andreidan requested review from jrodewig and martijnvg July 8, 2020 17:56

andreidan commented Jul 8, 2020

View reviewed changes

andreidan added 2 commits July 8, 2020 19:02

Add precedence test

b414ed4

Fix line length

13ab97b

dakrone reviewed Jul 8, 2020

View reviewed changes

server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateService.java Outdated Show resolved Hide resolved

Merge branch 'master' into composable-templates-default-timestamp-map…

6fc65c7

…ping

jrodewig approved these changes Jul 8, 2020

View reviewed changes

Add default timestamp only when datastream is defined in template

ed62220

elasticmachine and others added 2 commits July 9, 2020 01:17

Merge branch 'master' into composable-templates-default-timestamp-map…

c4ea34c

…ping

Disable tests in 7.9 until after we backport

f6de98d

martijnvg requested a review from jtibshirani July 9, 2020 08:47

martijnvg reviewed Jul 9, 2020

View reviewed changes

andreidan added 3 commits July 9, 2020 11:58

Add @timestamp mapping only for backing indices

ae6dab8

DOCS: mention the default date field used has the default options

57403c3

YAML test: assert timestamp field mapping type

d24c886

andreidan requested a review from martijnvg July 9, 2020 11:53

martijnvg mentioned this pull request Jul 9, 2020

Make timestamp field mapping part of data stream definition. #58642

Closed

martijnvg approved these changes Jul 13, 2020

View reviewed changes

andreidan added 4 commits July 13, 2020 17:36

Merge branch 'master' into composable-templates-default-timestamp-map…

009dc35

…ping

Fix merge

5b3c066

Fix test after merge

20343be

Spotless

d254ea6

elasticmachine and others added 2 commits July 13, 2020 12:56

Merge branch 'master' into composable-templates-default-timestamp-map…

97a2a59

…ping

Disable bwc tests

9bf89f8

andreidan merged commit 4e72f43 into elastic:master Jul 14, 2020

andreidan mentioned this pull request Jul 14, 2020

[7.x] Composable templates: add a default mapping for @timestamp (#59244) #59510

Merged

This was referenced Jul 14, 2020

Data Streams: fix bwc test #59528

Merged

Default to @timestamp in composable template datastream definition #59317

Merged

jakelandis added v8.0.0-alpha1 and removed v8.0.0 labels Jul 26, 2021

Conversation

andreidan commented Jul 8, 2020

Uh oh!

elasticmachine commented Jul 8, 2020

Uh oh!

elasticmachine commented Jul 8, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jrodewig left a comment

Choose a reason for hiding this comment

Uh oh!

andreidan commented Jul 9, 2020

Uh oh!

martijnvg left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andreidan commented Jul 9, 2020

Uh oh!

martijnvg left a comment

Choose a reason for hiding this comment

Uh oh!

andreidan commented Jul 13, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants