Implement index template generation in Golang by ruflin · Pull Request #3603 · elastic/beats

ruflin · 2017-02-16T09:42:22Z

The index template generation is moved to golang.

Changes

Improve defaults as they were (accidentally?) not used before as the properties were always overwritten by fresh properties
Use go run to execute command
Template generation is part of libbeat, only execution command is outside as it as has some additional logic on how to read the files.
Add assumption that $ES_BEATS is always a relative path
Add support for more fine grained versioning in template generation
Remove python script as not needed anymore

Blocked by #3655. Needs update after it is merged.

tsg · 2017-02-23T10:02:18Z

filebeat/filebeat.template-es2x.json

Hmm, I think "index": true is not valid for 2x. It should be "index": "analyzed", or simply leave the default.

Ok, lets remove it from the defaults then.

tsg · 2017-02-23T10:03:05Z

filebeat/filebeat.template-es2x.json

I'm not sure if this indexing the @timestamp makes sense.

tsg · 2017-02-23T10:09:58Z

packetbeat/packetbeat.template-es2x.json

Since we almost never want to disable "doc_values", I wonder if it's worth adding it explicitly to the templates.

As long as the defaults stay the same, there is no need to add it. Lets remove this too for the moment to keep it the same as it was previously and introduce defaults in case we need it. Defaults "should have been there" in the python script but were ignored ...

tsg · 2017-02-23T11:03:26Z

filebeat/filebeat.template-es2x.json

This is probably ignored by ES 2.x, but I guess it's harmless.

Will only apply it if not 2x as we have the logic for that. We should also make use of it :-)

tsg · 2017-02-23T11:11:46Z

libbeat/scripts/Makefile

What's this target for?

argh, testing target left over. will remove it

tsg

LGTM.

tsg · 2017-02-23T11:16:50Z

There seem to be some unit test failures, atm.

The index template generation is moved to golang. Changes * Use `go run` to execute command * Template generation is part of libbeat, only execution command is outside as it as has some additional logic on how to read the files. * Add assumption that $ES_BEATS is always a relative path * Add support for more fine grained versioning in template generation * Remove python script as not needed anymore

ruflin · 2017-02-23T11:23:34Z

I rebased and squashed now. Test failures were do to changes in the defaults.

monicasarbu · 2017-02-23T12:43:41Z

LGTM

andrewkroh

Nice! Overall it looks great and just had a few minor comments.

andrewkroh · 2017-02-23T17:01:52Z

dev-tools/cmd/index_template/index_template.go

+func main() {
+
+	version := flag.String("es-version", "", "Elasticsearch version")
+	inputFiles := flag.String("files", "", "List of files, comma seperated. This files must be passed with the full path.")


I think it would be more conventional to "args" instead of a flag for specifying files. https://golang.org/pkg/flag/#Args

andrewkroh · 2017-02-23T17:10:56Z

dev-tools/cmd/index_template/index_template.go

+
+func main() {
+
+	version := flag.String("es-version", "", "Elasticsearch version")


It would be nice to say what the default value is here. Could you call beat.GetDefaultVersion() here to populate the default value so that it shows up in the help.

andrewkroh · 2017-02-23T17:12:27Z

dev-tools/cmd/index_template/index_template.go

+
+	version := flag.String("es-version", "", "Elasticsearch version")
+	inputFiles := flag.String("files", "", "List of files, comma seperated. This files must be passed with the full path.")
+	beatName := flag.String("beatname", "", "Base index name. Normally {beatname}")


Is a value required? Probably it is so I would mention that it's required in the description and validate that the value is non-empty.

done, same for beatname

andrewkroh · 2017-02-23T17:16:59Z

dev-tools/cmd/index_template/index_template.go

+
+	templateString, err := template.GetTemplate(*version, *beatName, existingFiles)
+	if err != nil {
+		fmt.Printf("Error generating template: %+v", err)


For any prints or logs I would route them to stderr. fmt.Fprinf(os.Stderr, ...)

andrewkroh · 2017-02-23T17:29:13Z

libbeat/template/fields.go

+		case "array":
+			mapping = field.array()
+		case "group":
+			var newPath = ""


Use var newPath string instead.

andrewkroh · 2017-02-23T17:35:12Z

libbeat/template/template.go

+
+func loadYaml(path string) (Fields, error) {
+	keys := []Field{}
+	cfg, err := common.LoadFile(path)


You probably shouldn't use this method since it expects to be loading config files and checks the permissions on the files it reads.

Can you use yaml.Unmarshal more directly?

I changed it to

cfg, err := yaml.NewConfigWithFile(path) if err != nil { return nil, err } cfg.Unpack(&keys)

I really like to use go-ucfg here in case we add some config magic to this in the future.

Yeah, that's better since it bypasses the file mode check.

ruflin added the in progress Pull request is currently in progress. label Feb 16, 2017

ruflin mentioned this pull request Feb 17, 2017

Lower the default number of primary shards #3431

Closed

ruflin force-pushed the dynamic-template-generation branch from 72ab1c0 to 967d3a0 Compare February 22, 2017 20:44

ruflin mentioned this pull request Feb 23, 2017

Dynamically generate and load index templates #3654

Closed

14 tasks

ruflin force-pushed the dynamic-template-generation branch from 02bea93 to 1f9701f Compare February 23, 2017 09:33

ruflin added review and removed in progress Pull request is currently in progress. labels Feb 23, 2017

tsg reviewed Feb 23, 2017

View reviewed changes

filebeat/filebeat.template-es2x.json Outdated

Copy link
Copy Markdown

Contributor

tsg Feb 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if this indexing the @timestamp makes sense.

tsg reviewed Feb 23, 2017

View reviewed changes

ruflin force-pushed the dynamic-template-generation branch from e8144ab to c76b112 Compare February 23, 2017 10:58

tsg reviewed Feb 23, 2017

View reviewed changes

tsg approved these changes Feb 23, 2017

View reviewed changes

ruflin force-pushed the dynamic-template-generation branch from c76b112 to 76d4278 Compare February 23, 2017 11:23

ruflin mentioned this pull request Feb 23, 2017

Increase priority of the 6.0 template to overwrite the 5.x settings #3656

Closed

monicasarbu mentioned this pull request Feb 23, 2017

Increase priority of the 6.0 template to overwrite the 5.x settings #3657

Closed

andrewkroh reviewed Feb 23, 2017

View reviewed changes

implement review feedback

a64a7b4

andrewkroh merged commit 4e1a173 into elastic:master Feb 24, 2017

ruflin deleted the dynamic-template-generation branch February 27, 2017 13:27


		func main() {

		version := flag.String("es-version", "", "Elasticsearch version")

Conversation

ruflin commented Feb 16, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

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!

tsg left a comment

Choose a reason for hiding this comment

Uh oh!

tsg commented Feb 23, 2017

Uh oh!

ruflin commented Feb 23, 2017

Uh oh!

monicasarbu commented Feb 23, 2017

Uh oh!

andrewkroh 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!

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!

andrewkroh Feb 23, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ruflin commented Feb 16, 2017 •

edited

Loading

andrewkroh Feb 23, 2017 •

edited

Loading