From 47f8904667786e5f03427d7ba7ccdaca76b877ca Mon Sep 17 00:00:00 2001 From: Parvesh Chaudhary Date: Sun, 8 Sep 2024 17:29:11 +0530 Subject: [PATCH] Use DisableLogsFieldSearch flag to deduce es logs field mapping in template Co-authored-by: Raghav Gupta Signed-off-by: Parvesh Chaudhary --- cmd/es-rollover/app/init/action.go | 2 +- cmd/esmapping-generator/app/flags.go | 2 +- .../app/renderer/render.go | 20 ++-- .../app/renderer/render_test.go | 54 +++------ plugin/storage/es/factory.go | 1 + plugin/storage/es/mappings/fieldType.go | 58 ---------- plugin/storage/es/mappings/fieldType_test.go | 109 ------------------ ...er-span-with-logs-fieldtype-object-6.json} | 0 ...er-span-with-logs-fieldtype-object-7.json} | 0 ...er-span-with-logs-fieldtype-object-8.json} | 0 plugin/storage/es/mappings/jaeger-span-6.json | 12 +- plugin/storage/es/mappings/jaeger-span-7.json | 12 +- plugin/storage/es/mappings/jaeger-span-8.json | 12 +- plugin/storage/es/mappings/mapping.go | 2 +- plugin/storage/es/mappings/mapping_test.go | 16 +-- plugin/storage/es/options.go | 4 +- 16 files changed, 75 insertions(+), 229 deletions(-) delete mode 100644 plugin/storage/es/mappings/fieldType.go delete mode 100644 plugin/storage/es/mappings/fieldType_test.go rename plugin/storage/es/mappings/fixtures/{jaeger-span-with-object-fieldtype-logs-6.json => jaeger-span-with-logs-fieldtype-object-6.json} (100%) rename plugin/storage/es/mappings/fixtures/{jaeger-span-with-object-fieldtype-logs-7.json => jaeger-span-with-logs-fieldtype-object-7.json} (100%) rename plugin/storage/es/mappings/fixtures/{jaeger-span-with-object-fieldtype-logs-8.json => jaeger-span-with-logs-fieldtype-object-8.json} (100%) diff --git a/cmd/es-rollover/app/init/action.go b/cmd/es-rollover/app/init/action.go index 360be70fbc9..4ebdaac034f 100644 --- a/cmd/es-rollover/app/init/action.go +++ b/cmd/es-rollover/app/init/action.go @@ -40,7 +40,7 @@ func (c Action) getMapping(version uint, templateName string) (string, error) { UseILM: c.Config.UseILM, ILMPolicyName: c.Config.ILMPolicyName, EsVersion: version, - LogsFieldsType: mappings.ParseFieldType(c.Config.DisableLogsFieldSearch), + DisableLogsFieldSearch: c.Config.DisableLogsFieldSearch, } return mappingBuilder.GetMapping(templateName) } diff --git a/cmd/esmapping-generator/app/flags.go b/cmd/esmapping-generator/app/flags.go index 92fc21b3c0c..6434edac5eb 100644 --- a/cmd/esmapping-generator/app/flags.go +++ b/cmd/esmapping-generator/app/flags.go @@ -16,7 +16,7 @@ type Options struct { IndexPrefix string UseILM string // using string as util is being used in python and using bool leads to type issues. ILMPolicyName string - DisableLogsFieldSearch string + DisableLogsFieldSearch string // using string as util is being used in python and using bool leads to type issues. } const ( diff --git a/cmd/esmapping-generator/app/renderer/render.go b/cmd/esmapping-generator/app/renderer/render.go index eb2dfe850be..531162c0bf9 100644 --- a/cmd/esmapping-generator/app/renderer/render.go +++ b/cmd/esmapping-generator/app/renderer/render.go @@ -24,16 +24,20 @@ func GetMappingAsString(builder es.TemplateBuilder, opt *app.Options) (string, e if err != nil { return "", err } + disableLogsFieldSearch, err := strconv.ParseBool(opt.DisableLogsFieldSearch) + if err != nil { + return "", err + } mappingBuilder := mappings.MappingBuilder{ - TemplateBuilder: builder, - Shards: opt.Shards, - Replicas: opt.Replicas, - EsVersion: opt.EsVersion, - IndexPrefix: opt.IndexPrefix, - UseILM: enableILM, - ILMPolicyName: opt.ILMPolicyName, - LogsFieldsType: mappings.ParseFieldType(opt.DisableLogsFieldSearch), + TemplateBuilder: builder, + Shards: opt.Shards, + Replicas: opt.Replicas, + EsVersion: opt.EsVersion, + IndexPrefix: opt.IndexPrefix, + UseILM: enableILM, + ILMPolicyName: opt.ILMPolicyName, + DisableLogsFieldSearch: disableLogsFieldSearch, } return mappingBuilder.GetMapping(opt.Mapping) } diff --git a/cmd/esmapping-generator/app/renderer/render_test.go b/cmd/esmapping-generator/app/renderer/render_test.go index 8f7318d9f03..a1e3024da9e 100644 --- a/cmd/esmapping-generator/app/renderer/render_test.go +++ b/cmd/esmapping-generator/app/renderer/render_test.go @@ -53,16 +53,16 @@ func Test_getMappingAsString(t *testing.T) { IndexPrefix: "test-", UseILM: "true", ILMPolicyName: "jaeger-test-policy", - DisableLogsFieldSearch: "false", + DisableLogsFieldSearch: "true", }, wantedMapping: &mappings.MappingBuilder{ - EsVersion: 7, - Shards: 5, - Replicas: 1, - IndexPrefix: "test-", - UseILM: true, - ILMPolicyName: "jaeger-test-policy", - LogsFieldsType: mappings.NestedFieldType, + EsVersion: 7, + Shards: 5, + Replicas: 1, + IndexPrefix: "test-", + UseILM: true, + ILMPolicyName: "jaeger-test-policy", + DisableLogsFieldSearch: true, }, want: "ES version 7", }, @@ -79,13 +79,13 @@ func Test_getMappingAsString(t *testing.T) { DisableLogsFieldSearch: "false", }, wantedMapping: &mappings.MappingBuilder{ - EsVersion: 7, - Shards: 5, - Replicas: 1, - IndexPrefix: "test-", - UseILM: false, - ILMPolicyName: "jaeger-test-policy", - LogsFieldsType: mappings.ObjectFieldType, + EsVersion: 7, + Shards: 5, + Replicas: 1, + IndexPrefix: "test-", + UseILM: false, + ILMPolicyName: "jaeger-test-policy", + DisableLogsFieldSearch: false, }, wantErr: errors.New("parse error"), }, @@ -100,16 +100,8 @@ func Test_getMappingAsString(t *testing.T) { UseILM: "foo", ILMPolicyName: "jaeger-test-policy", }, - wantedMapping: &mappings.MappingBuilder{ - EsVersion: 7, - Shards: 5, - Replicas: 1, - IndexPrefix: "test-", - UseILM: false, - ILMPolicyName: "jaeger-test-policy", - LogsFieldsType: mappings.ObjectFieldType, - }, - wantErr: errors.New("strconv.ParseBool: parsing \"foo\": invalid syntax"), + wantedMapping: &mappings.MappingBuilder{}, + wantErr: errors.New("strconv.ParseBool: parsing \"foo\": invalid syntax"), }, { name: "Parse bool error for DisableLogsFieldSearch", @@ -123,16 +115,8 @@ func Test_getMappingAsString(t *testing.T) { ILMPolicyName: "jaeger-test-policy", DisableLogsFieldSearch: "foo", }, - wantedMapping: &mappings.MappingBuilder{ - EsVersion: 7, - Shards: 5, - Replicas: 1, - IndexPrefix: "test-", - UseILM: false, - ILMPolicyName: "jaeger-test-policy", - LogsFieldsType: mappings.ObjectFieldType, - }, - wantErr: errors.New("strconv.ParseBool: parsing \"foo\": invalid syntax"), + wantedMapping: &mappings.MappingBuilder{}, + wantErr: errors.New("strconv.ParseBool: parsing \"foo\": invalid syntax"), }, } for _, tt := range tests { diff --git a/plugin/storage/es/factory.go b/plugin/storage/es/factory.go index 3ad4628af61..bf8e204085b 100644 --- a/plugin/storage/es/factory.go +++ b/plugin/storage/es/factory.go @@ -323,6 +323,7 @@ func mappingBuilderFromConfig(cfg *config.Configuration) mappings.MappingBuilder PrioritySpanTemplate: cfg.PrioritySpanTemplate, PriorityServiceTemplate: cfg.PriorityServiceTemplate, PriorityDependenciesTemplate: cfg.PriorityDependenciesTemplate, + DisableLogsFieldSearch: cfg.DisableLogsFieldSearch, } } diff --git a/plugin/storage/es/mappings/fieldType.go b/plugin/storage/es/mappings/fieldType.go deleted file mode 100644 index 0b73d5e3ef6..00000000000 --- a/plugin/storage/es/mappings/fieldType.go +++ /dev/null @@ -1,58 +0,0 @@ -// Copyright (c) 2023 The Jaeger Authors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package mappings - -import ( - "fmt" - "strconv" -) - -type FieldType int - -func ParseFieldType(v any) FieldType { - if value, ok := v.(bool); ok { - if value { - return ObjectFieldType - } - return NestedFieldType - } else if value, ok := v.(string); ok { - switch value { - case "object", "true": - return ObjectFieldType - default: - return NestedFieldType - } - } - return NestedFieldType -} - -func (field FieldType) Format(f fmt.State, verb rune) { - str := "object" - if field == NestedFieldType { - str = "nested" - } - - switch verb { - case 'v': - _, _ = f.Write([]byte(str)) - default: - _, _ = f.Write([]byte(strconv.Itoa(int(field)))) - } -} - -const ( - NestedFieldType FieldType = iota - ObjectFieldType -) diff --git a/plugin/storage/es/mappings/fieldType_test.go b/plugin/storage/es/mappings/fieldType_test.go deleted file mode 100644 index 15616d2a4c8..00000000000 --- a/plugin/storage/es/mappings/fieldType_test.go +++ /dev/null @@ -1,109 +0,0 @@ -// Copyright (c) 2023 The Jaeger Authors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package mappings - -import ( - "fmt" - "testing" - - "github.com/stretchr/testify/assert" -) - -func TestParseFieldType(t *testing.T) { - tests := []struct { - name string - fieldType any - expected FieldType - }{ - { - fieldType: "nested", - expected: NestedFieldType, - name: "parse string nested as NestedFieldType", - }, - { - fieldType: "false", - expected: NestedFieldType, - name: "parse string nested as NestedFieldType", - }, - { - fieldType: false, - expected: NestedFieldType, - name: "parse bool false as NestedFieldType", - }, - { - fieldType: true, - expected: ObjectFieldType, - name: "parse bool true as ObjectFieldType", - }, - { - fieldType: "true", - expected: ObjectFieldType, - name: "parse string nested as NestedFieldType", - }, - { - fieldType: "object", - expected: ObjectFieldType, - name: "parse string object as ObjectFieldType", - }, - { - fieldType: 12, - expected: NestedFieldType, - name: "parse any other type as NestedFieldType", - }, - } - - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - actual := ParseFieldType(test.fieldType) - assert.Equal(t, test.expected, actual) - }) - } -} - -func TestFormat(t *testing.T) { - tests := []struct { - fieldType FieldType - expected string - format string - }{ - { - fieldType: NestedFieldType, - expected: "0", - format: "%s", - }, - { - fieldType: ObjectFieldType, - expected: "1", - format: "%s", - }, - { - fieldType: ObjectFieldType, - expected: "object", - format: "%v", - }, - { - fieldType: NestedFieldType, - expected: "nested", - format: "%v", - }, - } - - for _, test := range tests { - t.Run(fmt.Sprintf("should be able to format with specifier %s for field type %v", test.format, test.fieldType), func(t *testing.T) { - actual := fmt.Sprintf(test.format, test.fieldType) - assert.Equal(t, test.expected, actual) - }) - } -} diff --git a/plugin/storage/es/mappings/fixtures/jaeger-span-with-object-fieldtype-logs-6.json b/plugin/storage/es/mappings/fixtures/jaeger-span-with-logs-fieldtype-object-6.json similarity index 100% rename from plugin/storage/es/mappings/fixtures/jaeger-span-with-object-fieldtype-logs-6.json rename to plugin/storage/es/mappings/fixtures/jaeger-span-with-logs-fieldtype-object-6.json diff --git a/plugin/storage/es/mappings/fixtures/jaeger-span-with-object-fieldtype-logs-7.json b/plugin/storage/es/mappings/fixtures/jaeger-span-with-logs-fieldtype-object-7.json similarity index 100% rename from plugin/storage/es/mappings/fixtures/jaeger-span-with-object-fieldtype-logs-7.json rename to plugin/storage/es/mappings/fixtures/jaeger-span-with-logs-fieldtype-object-7.json diff --git a/plugin/storage/es/mappings/fixtures/jaeger-span-with-object-fieldtype-logs-8.json b/plugin/storage/es/mappings/fixtures/jaeger-span-with-logs-fieldtype-object-8.json similarity index 100% rename from plugin/storage/es/mappings/fixtures/jaeger-span-with-object-fieldtype-logs-8.json rename to plugin/storage/es/mappings/fixtures/jaeger-span-with-logs-fieldtype-object-8.json diff --git a/plugin/storage/es/mappings/jaeger-span-6.json b/plugin/storage/es/mappings/jaeger-span-6.json index 91f829baa12..d1dd104a348 100644 --- a/plugin/storage/es/mappings/jaeger-span-6.json +++ b/plugin/storage/es/mappings/jaeger-span-6.json @@ -65,14 +65,22 @@ "type":"integer" }, "logs":{ - "type":"{{ .LogsFieldsType }}", + {{- if .DisableLogsFieldSearch }} + "type":"object", + {{- else }} + "type":"nested", + {{- end }} "dynamic":false, "properties":{ "timestamp":{ "type":"long" }, "fields":{ - "type":"{{ .LogsFieldsType }}", + {{- if .DisableLogsFieldSearch }} + "type":"object", + {{- else }} + "type":"nested", + {{- end }} "dynamic":false, "properties":{ "key":{ diff --git a/plugin/storage/es/mappings/jaeger-span-7.json b/plugin/storage/es/mappings/jaeger-span-7.json index d72778f91b6..ba0fddbdeee 100644 --- a/plugin/storage/es/mappings/jaeger-span-7.json +++ b/plugin/storage/es/mappings/jaeger-span-7.json @@ -69,14 +69,22 @@ "type":"integer" }, "logs":{ - "type":"{{ .LogsFieldsType }}", + {{- if .DisableLogsFieldSearch }} + "type":"object", + {{- else }} + "type":"nested", + {{- end }} "dynamic":false, "properties":{ "timestamp":{ "type":"long" }, "fields":{ - "type":"{{ .LogsFieldsType }}", + {{- if .DisableLogsFieldSearch }} + "type":"object", + {{- else }} + "type":"nested", + {{- end }} "dynamic":false, "properties":{ "key":{ diff --git a/plugin/storage/es/mappings/jaeger-span-8.json b/plugin/storage/es/mappings/jaeger-span-8.json index 5f63fab39db..50c57689bae 100644 --- a/plugin/storage/es/mappings/jaeger-span-8.json +++ b/plugin/storage/es/mappings/jaeger-span-8.json @@ -72,14 +72,22 @@ "type": "integer" }, "logs": { - "type": "{{ .LogsFieldsType }}", + {{- if .DisableLogsFieldSearch }} + "type": "object", + {{- else }} + "type": "nested", + {{- end }} "dynamic": false, "properties": { "timestamp": { "type": "long" }, "fields": { - "type": "{{ .LogsFieldsType }}", + {{- if .DisableLogsFieldSearch }} + "type": "object", + {{- else }} + "type": "nested", + {{- end }} "dynamic": false, "properties": { "key": { diff --git a/plugin/storage/es/mappings/mapping.go b/plugin/storage/es/mappings/mapping.go index de04aac53f9..65d010a29d5 100644 --- a/plugin/storage/es/mappings/mapping.go +++ b/plugin/storage/es/mappings/mapping.go @@ -29,7 +29,7 @@ type MappingBuilder struct { IndexPrefix string UseILM bool ILMPolicyName string - LogsFieldsType FieldType + DisableLogsFieldSearch bool } // GetMapping returns the rendered mapping based on elasticsearch version diff --git a/plugin/storage/es/mappings/mapping_test.go b/plugin/storage/es/mappings/mapping_test.go index 1da7dbc33d2..fed42819331 100644 --- a/plugin/storage/es/mappings/mapping_test.go +++ b/plugin/storage/es/mappings/mapping_test.go @@ -30,17 +30,17 @@ func TestMappingBuilderGetMapping(t *testing.T) { jaegerDependencies = "jaeger-dependencies" ) tests := []struct { - mapping string - esVersion uint - fixtureName string - logsFieldType FieldType + mapping string + esVersion uint + fixtureName string + disableLogsFieldSearch bool }{ {mapping: jaegerSpan, esVersion: 8, fixtureName: "jaeger-span-8"}, {mapping: jaegerSpan, esVersion: 7, fixtureName: "jaeger-span-7"}, {mapping: jaegerSpan, esVersion: 6, fixtureName: "jaeger-span-6"}, - {mapping: jaegerSpan, esVersion: 8, logsFieldType: ObjectFieldType, fixtureName: "jaeger-span-with-object-fieldtype-logs-8"}, - {mapping: jaegerSpan, esVersion: 7, logsFieldType: ObjectFieldType, fixtureName: "jaeger-span-with-object-fieldtype-logs-7"}, - {mapping: jaegerSpan, esVersion: 6, logsFieldType: ObjectFieldType, fixtureName: "jaeger-span-with-object-fieldtype-logs-6"}, + {mapping: jaegerSpan, esVersion: 8, fixtureName: "jaeger-span-with-logs-fieldtype-object-8", disableLogsFieldSearch: true}, + {mapping: jaegerSpan, esVersion: 7, fixtureName: "jaeger-span-with-logs-fieldtype-object-7", disableLogsFieldSearch: true}, + {mapping: jaegerSpan, esVersion: 6, fixtureName: "jaeger-span-with-logs-fieldtype-object-6", disableLogsFieldSearch: true}, {mapping: jaegerService, esVersion: 8, fixtureName: "jaeger-service-8"}, {mapping: jaegerService, esVersion: 7, fixtureName: "jaeger-service-7"}, {mapping: jaegerService, esVersion: 6, fixtureName: "jaeger-service-6"}, @@ -61,7 +61,7 @@ func TestMappingBuilderGetMapping(t *testing.T) { IndexPrefix: "test-", UseILM: true, ILMPolicyName: "jaeger-test-policy", - LogsFieldsType: tt.logsFieldType, + DisableLogsFieldSearch: tt.disableLogsFieldSearch, } got, err := mb.GetMapping(tt.mapping) require.NoError(t, err) diff --git a/plugin/storage/es/options.go b/plugin/storage/es/options.go index ed8201fbe5c..63b66db8760 100644 --- a/plugin/storage/es/options.go +++ b/plugin/storage/es/options.go @@ -281,8 +281,8 @@ func addFlags(flagSet *flag.FlagSet, nsConfig *namespaceConfig) { flagSet.Bool( nsConfig.namespace+suffixDisableLogsFieldSearch, nsConfig.DisableLogsFieldSearch, - "(experimental) Option to disable search on logs.field field of jaeger span index. Setting this option to true would set mapping of "+ - "logs.field field span index to object type. Its set to false by default.", + "(experimental) When true, turns off the ability to search within log fields, and uses "+ + "more efficient 'object' mapping for logs[].fields[], instead of 'nested' mapping.", ) if nsConfig.namespace == archiveNamespace {