Add support for dots in field names for metrics usecases #86166

javanna · 2022-04-25T20:54:10Z

This PR adds support for a new mapping parameter to the configuration of the object mapper (root as well as individual fields), that makes it possible to store metrics data where it's common to have fields with dots in their names in the following format:

{
  "metrics.time" : 10,
  "metrics.time.min" : 1,
  "metrics.time.max" : 500
}

Instead of expanding dotted paths the their corresponding object structure, objects can be configured to preserve dots in field names, in which case they can only hold leaf sub-fields and no further objects.

The mapping parameter is called subobjects and controls whether an object can hold other objects (defaults to true) or not. The following example shows how it can be configured in the mappings:

{
  "mappings" : {
    "properties" : {
      "metrics" : {
        "type" : "object", 
        "subobjects" : false
      }
    }
  }
}

Closes #63530

elasticsearchmachine · 2022-04-28T10:18:54Z

Hi @javanna, I've created a changelog YAML for you.

elasticsearchmachine · 2022-04-28T10:23:32Z

Hi @javanna, I've updated the changelog YAML for you.

romseygeek

This is great! I left a couple of nits and suggestions. I think it's probably worth adding the tests for objects as arrays as well, just because that's an area where we are pretty under-tested and I can see it would be very easy for bugs to slip in there.

romseygeek · 2022-04-27T10:06:47Z

server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java

            parseObjectOrField(context, dynamicObjectMapper);
+            context.path().setCollapsed(false);


Maybe we can merge setCollapsed(false) with remove(), as once you've gone back up a level you are by definition no longer on a collapsed object (if it was collapsed you couldn't have added a new level to it!)

we use add also for multi-fields, so I think resetting collapsed at remove may actually cause problems.

That though reminds me: can you please triple check that I am calling setCollapsed in all the right places? I am afraid I am missing some...

docs/reference/mapping/params/collapsed.asciidoc

server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java

server/src/main/java/org/elasticsearch/index/mapper/DotExpandingXContentParser.java

romseygeek · 2022-04-28T10:35:42Z

server/src/test/java/org/elasticsearch/index/mapper/DocumentParserTests.java

+              }
+            }
+            """)));
+        assertEquals("Object [_doc] is collapsed and does not support inner object [metrics]", err.getMessage());


I wonder if we can detect this case and replace Object [_doc] with root, or something else? It's a bit confusing otherwise, as there isn't a _doc anywhere in the mappings that the user sees.

It would be nice to get the full object path here as well, but from a quick look at the parsing code it doesn't look like it will be simple to do, so happy to leave both of these as a TODO.

yea I agree about the error message, I believe that's what we already do for enabled though. I would look at changing this as a follow-up.

Which full object path do you mean?

elasticsearchmachine · 2022-04-28T11:43:04Z

Hi @javanna, I've updated the changelog YAML for you. Note that since this PR is labelled release highlight, you need to update the changelog YAML to fill out the extended information sections.

elasticmachine · 2022-04-28T11:43:08Z

Pinging @elastic/es-search (Team:Search)

romseygeek

LGTM. Thanks for bringing this one over the line @javanna!

As part of elastic#86166 we added support for a new mapping parameter called `subobjects` that can be set to the `object` field type. Such parameter will affect not only how incoming documents will be dinamically mapped in the future, but also how field names are treated as part of the mappings themselves. Mappings get parsed into a map, where keys ordering is not guaranteed. In case a mappings call contains `subobjects` set to `false` and also sub-fields for that same object, we need to make sure that we read the parameter in advance in order to know how to treat the sub-field and decide whether we need to expand dots in field names or leave them as they are.

With elastic#86166 we have added support for leaf fields that have dots in their names, without needing to expand their path to intermediate objects. That means that for instance a host.name keyword field mapper can be held directly by the root object mapper if the root has subobjects set to false. This opens up for situations where a field name containing dots can't necessarily be split into a leaf name and a parent path made of intermediate object mappers. In the field mapper merge code, we build the full path of the merged field making that now incorrect assumption, which causes the result of merged leaf fields to have incorrect full path. This is a bug although it got unnoticed so far as the full path of a field mapper is not so widely used compared to its leaf name (returned by the simpleName method). One place where the full path is used is when sorting the child mappers in ObjectMapper#serializeMappers which was causing MapperService#assertRefreshIsNotNeeded to trip as the result of the merge has fields in a different order compared to the incoming mappings. With this fix, we add a MapperBuilderContext argument to the different merge methods, propagate the MapperBuilderContext in all the merge calls and create a child context when needed, meaning whenever merging object mappers, so that their children mappers are created with the proper full path.

With #86166 we have added support for leaf fields that have dots in their names, without needing to expand their path to intermediate objects. That means that for instance a host.name keyword field mapper can be held directly by the root object mapper if the root has subobjects set to false. This opens up for situations where a field name containing dots can't necessarily be split into a leaf name and a parent path made of intermediate object mappers. In the field mapper merge code, we build the full path of the merged field making that now incorrect assumption, which causes the result of merged leaf fields to have incorrect full path. This is a bug although it got unnoticed so far as the full path of a field mapper is not so widely used compared to its leaf name (returned by the simpleName method). One place where the full path is used is when sorting the child mappers in ObjectMapper#serializeMappers which was causing MapperService#assertRefreshIsNotNeeded to trip as the result of the merge has fields in a different order compared to the incoming mappings. With this fix, we add a MapperBuilderContext argument to the different merge methods, propagate the MapperBuilderContext in all the merge calls and create a child context when needed, meaning whenever merging object mappers, so that their children mappers are created with the proper full path.

As part of #86166 we added support for a new mapping parameter called `subobjects` that can be set to the `object` field type. Such parameter will affect not only how incoming documents will be dynamically mapped in the future, but also how field names are treated as part of the mappings themselves. Mappings get parsed into a map, where keys ordering is not guaranteed. In case a mappings call contains `subobjects` set to `false` and also sub-fields for that same object, we need to make sure that we read the parameter in advance in order to know how to treat the sub-field and decide whether we need to expand dots in field names or leave them as they are.

This adds some paranoid tests for synthetic source with disabling subobjects, as added by elastic#86166. It turns out that synthetic source does exactly what you'd expect with disabling subobjects - it creates fields with dots in their names. This adds tests for that.

This adds some paranoid tests for synthetic source with disabling subobjects, as added by #86166. It turns out that synthetic source does exactly what you'd expect with disabling subobjects - it creates fields with dots in their names. This adds tests for that.

This adds some paranoid tests for synthetic source with disabling subobjects, as added by elastic#86166. It turns out that synthetic source does exactly what you'd expect with disabling subobjects - it creates fields with dots in their names. This adds tests for that.

This adds some paranoid tests for synthetic source with disabling subobjects, as added by #86166. It turns out that synthetic source does exactly what you'd expect with disabling subobjects - it creates fields with dots in their names. This adds tests for that.

ruflin · 2022-06-02T08:49:13Z

I filed elastic/package-spec#349 to get support for this in package-spec.

Add support for dots in field names for metrics usecases

e4c9458

elasticsearchmachine added the v8.3.0 label Apr 25, 2022

javanna added 2 commits April 26, 2022 11:34

Merge branch 'main' into enhancement/collapsed_objects

5d34e0f

checkstyle

af1d3fa

javanna removed the v8.3.0 label Apr 26, 2022

javanna added 10 commits April 26, 2022 14:26

iter

64cb72b

add test

b96ae79

spotless

8d47bfe

Merge branch 'main' into enhancement/collapsed_objects

c6c3b2c

more tests

f825e00

spotless

7d12ca7

iter

227fe53

iter

6175fc2

spotless

9761211

docs

33d570f

javanna added :Search Foundations/Mapping Index mappings, including merging and defining field types >feature v8.3.0 labels Apr 28, 2022

Update docs/changelog/86166.yaml

5034bc1

Update docs/changelog/86166.yaml

4ebdd8f

romseygeek reviewed Apr 28, 2022

View reviewed changes

javanna added the release highlight label Apr 28, 2022

Update docs/changelog/86166.yaml

7a6143d

javanna marked this pull request as ready for review April 28, 2022 11:43

elasticmachine added the Team:Search Meta label for search team label Apr 28, 2022

javanna mentioned this pull request Apr 28, 2022

Add 'flatten' parameter to object mappers #78997

Closed

romseygeek approved these changes May 17, 2022

View reviewed changes

update changelog

6db29eb

javanna merged commit d45b19d into elastic:master May 17, 2022

This was referenced May 17, 2022

Support metrics with dots in their names elastic/apm#347

Open

"cluster settings" JSON structure is ambiguous with dotted .frozen settings #85789

Closed

sethmlarson mentioned this pull request May 17, 2022

Add support for new subobjects property for object mapping types elastic/elasticsearch-specification#1714

Closed

javanna mentioned this pull request May 18, 2022

Read subobjects mapping parameter in advance #86894

Merged

javanna mentioned this pull request May 19, 2022

Propagate MapperBuilderContext across merge calls #86946

Merged

nik9000 mentioned this pull request May 20, 2022

Synthetic Source #86603

Closed

50 tasks

nik9000 mentioned this pull request May 31, 2022

Synthetic source: tests for disabling subobjects #87261

Merged

ruflin mentioned this pull request Jun 2, 2022

Add support for subobjects: false elastic/package-spec#349

Closed

ruflin mentioned this pull request Jun 24, 2022

[filebeat] Add option to dedot keys to decode_json_fields processor elastic/beats#26154

Open

ruflin referenced this pull request in felixbarny/elasticsearch Jun 29, 2022

Logs Endpoint

c1d5478

kiju98 mentioned this pull request Jul 16, 2022

mapper_parsing_exception (Field is defined more than once) in 8.3.2 #88573

Closed

jdconrad mentioned this pull request Jul 28, 2022

Improve support of source fallback #88913

Open

swallez mentioned this pull request Aug 30, 2022

ObjectProperty missing the subobjects property elastic/elasticsearch-specification#1829

Closed

eyalkoren mentioned this pull request Jun 28, 2023

Fix fields API with subobjects: false #97092

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for dots in field names for metrics usecases #86166

Add support for dots in field names for metrics usecases #86166

javanna commented Apr 25, 2022 •

edited

Loading

elasticsearchmachine commented Apr 28, 2022

elasticsearchmachine commented Apr 28, 2022

romseygeek left a comment

romseygeek Apr 27, 2022

javanna Apr 28, 2022

javanna Apr 28, 2022

romseygeek Apr 28, 2022

romseygeek Apr 28, 2022

javanna Apr 28, 2022

elasticsearchmachine commented Apr 28, 2022

elasticmachine commented Apr 28, 2022

romseygeek left a comment

ruflin commented Jun 2, 2022

		parseObjectOrField(context, dynamicObjectMapper);
		context.path().setCollapsed(false);

Add support for dots in field names for metrics usecases #86166

Add support for dots in field names for metrics usecases #86166

Conversation

javanna commented Apr 25, 2022 • edited Loading

elasticsearchmachine commented Apr 28, 2022

elasticsearchmachine commented Apr 28, 2022

romseygeek left a comment

Choose a reason for hiding this comment

romseygeek Apr 27, 2022

Choose a reason for hiding this comment

javanna Apr 28, 2022

Choose a reason for hiding this comment

javanna Apr 28, 2022

Choose a reason for hiding this comment

romseygeek Apr 28, 2022

Choose a reason for hiding this comment

romseygeek Apr 28, 2022

Choose a reason for hiding this comment

javanna Apr 28, 2022

Choose a reason for hiding this comment

elasticsearchmachine commented Apr 28, 2022

elasticmachine commented Apr 28, 2022

romseygeek left a comment

Choose a reason for hiding this comment

ruflin commented Jun 2, 2022

javanna commented Apr 25, 2022 •

edited

Loading