Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dynamically generate template on startup #3681

Merged
merged 2 commits into from
Apr 4, 2017

Conversation

ruflin
Copy link
Contributor

@ruflin ruflin commented Feb 27, 2017

So far the template was loaded from the beatname.template.json files. This now uses the fields.yml file to dynamically generate the template based on the ES version connected to. The name of the template is set dynamically based on template.name. By default it has the value {beatname}.

  • Remove generated templates
  • Package fields.yml
  • Clean up config options as 2.x option is not needed anymore
  • Update docs
  • Update and test packaging

Part of #3654

@ruflin ruflin added discuss Issue needs further discussion. in progress Pull request is currently in progress. libbeat labels Feb 27, 2017
@ruflin ruflin force-pushed the dynamically-load-template branch 2 times, most recently from f05a8fa to 6ddff24 Compare February 28, 2017 07:37
@ruflin ruflin mentioned this pull request Mar 1, 2017
@ruflin ruflin force-pushed the dynamically-load-template branch 3 times, most recently from 14492df to 62360ba Compare March 1, 2017 11:44
@ruflin ruflin force-pushed the dynamically-load-template branch from 618a273 to 228ec31 Compare March 2, 2017 11:04
ruflin added a commit to ruflin/beats that referenced this pull request Mar 2, 2017
Templates do not need to be checked in as they are generated before the tests and packaging. This is a prerequisit for elastic#3681 to simplify the PR.
tsg pushed a commit that referenced this pull request Mar 3, 2017
Templates do not need to be checked in as they are generated before the tests and packaging. This is a prerequisit for #3681 to simplify the PR.
@ruflin ruflin force-pushed the dynamically-load-template branch from 228ec31 to d0013bb Compare March 3, 2017 08:53
ruflin added a commit to ruflin/beats that referenced this pull request Mar 3, 2017
This PR is to reduce the number of changes in elastic#3681 .
tsg pushed a commit that referenced this pull request Mar 3, 2017
@ruflin ruflin force-pushed the dynamically-load-template branch from d0013bb to e9be118 Compare March 3, 2017 11:47
@ruflin ruflin force-pushed the dynamically-load-template branch 3 times, most recently from e7abc0c to 19a4e0d Compare March 28, 2017 12:33
@ruflin ruflin added the review label Mar 28, 2017
# Path to template file
#template.path: "${path.config}/heartbeat.template.json"
# Path to fields.yml file to generate the template
#template.path: "${path.config}/fields.yml"
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume this approach (re-using the template.path setting) just for the prototype phase or the actual plan? I'd say we probably want to rethink the configuration options.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree on rethinking the config options. I added it to #3654

@tsg
Copy link
Contributor

tsg commented Mar 28, 2017

My 2c on this.

Config option is called template.path which points to fields.yml. Is the name still ok?

I'd say it's better to use a different name, since this is quite different conceptually. Also, I'd suggest it is a good idea to let the user provide a fully custom template file (which they can generate/edit whatever name they like).

I wouldn't worry too much about backwards compatibility in this case, since I'd guess (and hope) most people didn't use a custom template path.

Should template config be moved to the top level (like dashboard)

Yeah, configuration is tricky here, perhaps we have a global fields section where the fields.yml is configured, and the rest of the things pull from that?

What is our recommendation on loading the template manually? Generate first with index_template binary and then load manually?

I agree there should be a way to write the template to a file. But I guess the primary way of adjusting things will be by editing the fields.yml file, which will be considered configuration?

Do we ship in index_template binary similar to import_dashboards? Can we integrate it into beats directly? -> setup

A binary has the disadvantage of adding to the package size. We could consider creating a beats-tools package that contains only these tools. This would be for things that you don't need for getting started, but are helpful in some situations.

@tsg
Copy link
Contributor

tsg commented Mar 28, 2017

Another, kind of random, thought: I'd like to make it fairly easy to change some index settings (that typically go in the template), like _source: on/off, number_of_shards, number_of_replicas, etc. We could put these in fields.yml or in the main Beat configuration file, but in both cases we should be careful about how much we want to duplicate the ES settings in the Beats config.

@ruflin
Copy link
Contributor Author

ruflin commented Mar 29, 2017

@tsg Agreement on the points above. I kind of start to like the idea of a beats-tools package which is mainly for sysadmins etc for the setup phase but could also be used in the future to do some "checks".

I added all the points above to the meta issue to be solved #3654 I suggest moving forward with this PR as is and to further changes in follow up PR's.

So far the template was loaded from the `beatname.template.json` files. This now uses the `fields.yml` file to dynamically generate the template based on the ES version connected to. The name of the template is set dynamically based on `template.name`. By default it has the value {beatname}.

* [x] Remove generated templates
* [x] Package fields.yml
* [x] Clean up config options as 2.x option is not needed anymore
* [x] Update docs
* [x] Update and test packaging

Further changes:

* Remove tests in libbeat to load all templates. This requires global knowledge of the other beats in libbeat which is not good.
* Create new makefile target to generate template for testing purpose.

Questions:
* Config option is called `template.path` which points to `fields.yml`. Is the name still ok?
* What is our recommendation on loading the template manually? Generate first with `index_template` binary and then load manually?
  * Do we ship in index_template binary similar to import_dashboards? Can we integrate it into beats directly? -> setup
* Should template config be moved to the top level (like `dashboard`)

Part of elastic#3654
@ruflin ruflin force-pushed the dynamically-load-template branch from 19a4e0d to 9d2a5c8 Compare March 29, 2017 08:39
@ruflin ruflin removed discuss Issue needs further discussion. in progress Pull request is currently in progress. labels Apr 3, 2017
@ruflin ruflin force-pushed the dynamically-load-template branch from 3eaa456 to 8c3cb2e Compare April 3, 2017 12:58
@ruflin
Copy link
Contributor Author

ruflin commented Apr 3, 2017

jenkins, retest it

@tsg tsg merged commit bfa3128 into elastic:master Apr 4, 2017
@ruflin ruflin deleted the dynamically-load-template branch April 4, 2017 11:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants