Add xpack info and usage endpoints for runtime fields #65600

javanna · 2020-11-30T13:11:40Z

Relates to #59332

elasticmachine · 2020-11-30T13:11:43Z

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

romseygeek · 2020-11-30T13:26:27Z

...e/src/main/java/org/elasticsearch/xpack/core/runtimefields/RuntimeFieldsFeatureSetUsage.java

+            Set<String> indexFieldTypes = new HashSet<>();
+            MappingMetadata mappingMetadata = indexMetadata.mapping();
+            if (mappingMetadata != null) {
+                Object runtimeObject = mappingMetadata.getSourceAsMap().get("runtime");


We have MappingVisitor which already implements some of this, although it looks in properties rather than runtime - might be worth extending or re-using some of that?

I am aware, those rely on standard field type structure also. To reuse the existing mappings visitor I should have made a runtime visitor that looks in the runtime section instead of properties, and also plugin a function to process fields differently for runtime fields, as we want to have additional stats for them. Also, the standard mappings visitor looks at multi fields recursively which is not something that we support for runtime fields. The only call to this visitor would be from the runtime fields usage api, hence I thought it's overkill. I believe it would be more lines added for the added flexibility than shared between the two implementations. What do you think?

I figure if we see the need later we'll add a visitor but that this works fine for now.

javanna · 2020-11-30T14:36:49Z

run elasticsearch-ci/1

nik9000

I left some small stuff but LGTM either way.

nik9000 · 2020-11-30T18:11:00Z

...k/plugin/core/src/main/java/org/elasticsearch/xpack/core/action/XPackUsageFeatureAction.java

+    static final List<XPackUsageFeatureAction> ALL = List.of(
+        SECURITY, MONITORING, WATCHER, GRAPH, MACHINE_LEARNING, LOGSTASH, EQL, SQL, ROLLUP, INDEX_LIFECYCLE, SNAPSHOT_LIFECYCLE, CCR,
+        TRANSFORM, VECTORS, VOTING_ONLY, FROZEN_INDICES, SPATIAL, ANALYTICS, DATA_STREAMS, SEARCHABLE_SNAPSHOTS, DATA_TIERS,
+        AGGREGATE_METRIC, RUNTIME_FIELDS


👍

While you are here could you make them one per line and alphabetical order? I know it is a pain but it'll push everyone who comes after you to be neat and tidy.

nik9000 · 2020-11-30T18:12:01Z

...e/src/main/java/org/elasticsearch/xpack/core/runtimefields/RuntimeFieldsFeatureSetUsage.java

+            Set<String> indexFieldTypes = new HashSet<>();
+            MappingMetadata mappingMetadata = indexMetadata.mapping();
+            if (mappingMetadata != null) {
+                Object runtimeObject = mappingMetadata.getSourceAsMap().get("runtime");


I figure if we see the need later we'll add a visitor but that this works fine for now.

nik9000 · 2020-11-30T18:12:55Z

...e/src/main/java/org/elasticsearch/xpack/core/runtimefields/RuntimeFieldsFeatureSetUsage.java

+                Object runtimeObject = mappingMetadata.getSourceAsMap().get("runtime");
+                if (runtimeObject instanceof Map) {
+                    @SuppressWarnings("unchecked")
+                    Map<String, Object> runtimeMappings = (Map<String, Object>) runtimeObject;


If you cast to Map<?, ?> you don't need to suppress and .values will still return Object. You can't .put on the map but you don't need to.

Just a sneaky way to save a line.

nik9000 · 2020-11-30T18:14:35Z

...e/src/main/java/org/elasticsearch/xpack/core/runtimefields/RuntimeFieldsFeatureSetUsage.java

+            }
+            Set<String> indexFieldTypes = new HashSet<>();
+            MappingMetadata mappingMetadata = indexMetadata.mapping();
+            if (mappingMetadata != null) {


Is there any chance you could flip this so it continues if mappingMetadata is null. I find quick continues to be easier to short cut in my head when reading. With these indenting ifs I have to scroll down and check if there is an else.

nik9000 · 2020-11-30T18:15:37Z

...e/src/main/java/org/elasticsearch/xpack/core/runtimefields/RuntimeFieldsFeatureSetUsage.java

+                                Object scriptObject = runtimeFieldMapping.get("script");
+                                if (scriptObject == null) {
+                                    stats.scriptLessCount++;
+                                } else if (scriptObject instanceof Map) {


If script isn't an instanceof Map can it be a String? Would we get the short form here?

It is my understanding that regardless of how it was submitted, the script always gets outputted in the long form. I will double check though. That is also why I am using the Script class in the unit tests

nik9000 · 2020-11-30T18:16:28Z

...e/src/main/java/org/elasticsearch/xpack/core/runtimefields/RuntimeFieldsFeatureSetUsage.java

+        }
+        RuntimeFieldStats[] runtimeFieldStats = fieldTypes.values().toArray(new RuntimeFieldStats[0]);
+        Arrays.sort(runtimeFieldStats, Comparator.comparing(RuntimeFieldStats::type));
+        return new RuntimeFieldsFeatureSetUsage(runtimeFieldStats);


Any reason not to use a List in the response? I know we sometimes use List and sometimes arrays but I've never been sure why.

I generally use a list when elements need to be added, and an array otherwise. I guess you'd like to use a list because it can be made truly immutable?

Mostly because we accumulated a list so it feels like we may as well keep it that way. Either way is fine.

nik9000 · 2020-11-30T18:17:25Z

...e/src/main/java/org/elasticsearch/xpack/core/runtimefields/RuntimeFieldsFeatureSetUsage.java

+        private long maxSourceUsages = 0;
+        private long totalSourceUsages = 0;
+        private long maxDocUsages = 0;
+        private long totalDocUsages = 0;


= 0 is the default, right?

I guess, but I always forget.

nik9000

I think this is good.

I also think it means we'll need totally separate stuff for grok or geoip or whatever.

nik9000 · 2020-11-30T21:05:29Z

...e/src/main/java/org/elasticsearch/xpack/core/runtimefields/RuntimeFieldsFeatureSetUsage.java

+                                        String scriptSource = sourceObject.toString();
+                                        int chars = scriptSource.length();
+                                        long lines = scriptSource.lines().count();
+                                        int docUsages = countOccurrences(scriptSource, "doc[\\[\\.]");


I think its worth double checking if this matches stuff like doc\. I never can remember the escaping rules for character classes.

I think it's ok, but I am not sure I can be trusted, I will double check

javanna added 4 commits November 25, 2020 22:05

Add xpack info and usage endpoints for runtime fields

0e02542

wip

4bfe510

Merge branch 'master' into enhancement/runtime_fields_telemetry

782a558

iter

5773c0b

javanna added >enhancement :Search Foundations/Mapping Index mappings, including merging and defining field types v8.0.0 v7.11.0 labels Nov 30, 2020

javanna requested a review from nik9000 November 30, 2020 13:11

elasticmachine added the Team:Search Meta label for search team label Nov 30, 2020

javanna mentioned this pull request Nov 30, 2020

Add support for runtime fields #59332

Closed

30 tasks

spotless

82979ec

romseygeek reviewed Nov 30, 2020

View reviewed changes

docs failures

2958266

nik9000 approved these changes Nov 30, 2020

View reviewed changes

javanna added 5 commits December 1, 2020 10:05

Merge branch 'master' into enhancement/runtime_fields_telemetry

c45f6cd

iter

b8447c3

Merge branch 'master' into enhancement/runtime_fields_telemetry

16854e3

iter

7a870e8

Merge branch 'master' into enhancement/runtime_fields_telemetry

c976a46

javanna merged commit 20d6fbc into elastic:master Dec 8, 2020

This was referenced Dec 8, 2020

Add xpack info and usage endpoints for runtime fields #66027

Merged

[CI] /mixed_cluster/100_analytics_usage/Basic test for usage stats on analytics indices failure #66021

Closed

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add xpack info and usage endpoints for runtime fields #65600

Add xpack info and usage endpoints for runtime fields #65600

javanna commented Nov 30, 2020

elasticmachine commented Nov 30, 2020

romseygeek Nov 30, 2020

javanna Nov 30, 2020

nik9000 Nov 30, 2020

javanna commented Nov 30, 2020

nik9000 left a comment

nik9000 Nov 30, 2020

nik9000 Nov 30, 2020

nik9000 Nov 30, 2020

nik9000 Nov 30, 2020

javanna Dec 1, 2020

nik9000 Nov 30, 2020

nik9000 Nov 30, 2020

javanna Nov 30, 2020

nik9000 Nov 30, 2020

javanna Nov 30, 2020

nik9000 Nov 30, 2020

nik9000 Nov 30, 2020

javanna Nov 30, 2020

nik9000 left a comment

nik9000 Nov 30, 2020

javanna Nov 30, 2020

Add xpack info and usage endpoints for runtime fields #65600

Add xpack info and usage endpoints for runtime fields #65600

Conversation

javanna commented Nov 30, 2020

elasticmachine commented Nov 30, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

javanna commented Nov 30, 2020

nik9000 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nik9000 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment