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

[EPM] [Endpoint] Disable dynamic mapping on a template #68380

Closed
jonathan-buttner opened this issue Jun 5, 2020 · 11 comments
Closed

[EPM] [Endpoint] Disable dynamic mapping on a template #68380

jonathan-buttner opened this issue Jun 5, 2020 · 11 comments
Assignees
Labels
enhancement New value added to drive a business result Team:Endpoint Data Visibility Team managing the endpoint resolver Team:Fleet Team label for Observability Data Collection Fleet team

Comments

@jonathan-buttner
Copy link
Contributor

jonathan-buttner commented Jun 5, 2020

The endpoint team would like to disable dynamic mappings for all the endpoint indices. The easiest way to do this I think is to disable it at the template level that is generated by the ingest manager.

To disable dynamic mappings, the dynamic field would need to be added here: https://github.com/elastic/kibana/blob/master/x-pack/plugins/ingest_manager/server/services/epm/elasticsearch/template/template.ts#L274

To make this configurable per package, we'd probably want a package to include a new field in its manifest to indicate to the ingest manager whether dynamic mappings should be disabled for the generated templates.

This could be added either at the packages top level manifest or maybe it'd be better to do it at the dataset level (this might not work since I believe multiple datasets can form a single template)? For endpoint, we'd like to disable it for all templates so doing at the package level would be fine.

We'd probably also want to add some validation to the package-registry to make sure the fields is one of true, false, or strict. If the field was not set, is should probably default to true.

The ingest manager would need to add the field to its data structure and pass it along to the where the template is generated.

Example mapping
PUT _template/my_index
{
    "index_patterns": [
    "my_index-*"
  ],
  "mappings": {
    "dynamic": "false", <-------
    "properties": {
      "user": {
        "type": "nested",
        "enabled": false,
        "properties": {
          "first": {
            "type": "keyword"
          },
          "last": {
            "type": "keyword"
          }
        }
      }
    }
  }
}
@jonathan-buttner jonathan-buttner added enhancement New value added to drive a business result Team:Endpoint Data Visibility Team managing the endpoint resolver Team:Fleet Team label for Observability Data Collection Fleet team labels Jun 5, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/endpoint-data-visibility-team (Team:Endpoint Data Visibility)

@elasticmachine
Copy link
Contributor

Pinging @elastic/ingest-management (Team:Ingest Management)

@ruflin
Copy link
Contributor

ruflin commented Jun 8, 2020

@jonathan-buttner I think this should be defined in the package on the dataset level. AFAIK it is not possible that multiple datasets can form a single template.

Could you open an issue on the package-registry repo with a proposal for the config?

UPDATE: Just found elastic/package-registry#497

@jonathan-buttner
Copy link
Contributor Author

@jfsiii @skh @neptunian @nchaulet I was going to take a stab at implementing these kibana side changes. My thought was to do a lodash _.merge() with any custom settings and custom mapping fields sent over via the package with the changes that @ruflin added in the package-registry PR: elastic/package-registry#552

I was going to add that after all the base stuff was computed here: https://github.com/elastic/kibana/blob/master/x-pack/plugins/ingest_manager/server/services/epm/elasticsearch/template/template.ts#L49

We'd want to do a recursive merge which lodash does: https://lodash.com/docs/2.4.2#merge that way we don't blow away the whole object for a particular field.

I was going to intentionally have the package's fields overwrite any of the base fields. What are you thoughts on this? This could potentially cause problems if the package defined some weird values, but maybe the risk is ok there?

@skh
Copy link
Contributor

skh commented Jun 30, 2020

@jonathan-buttner Would it be possible to put the new settings into a component template, and have elasticsearch sort out the merging?

See https://www.elastic.co/guide/en/elasticsearch/reference/current/indices-templates.html for how composing templates works now (this is the index templates v2 feature that has been introduced with 7.8).

@jonathan-buttner
Copy link
Contributor Author

@jonathan-buttner Would it be possible to put the new settings into a component template, and have elasticsearch sort out the merging?

See https://www.elastic.co/guide/en/elasticsearch/reference/current/indices-templates.html for how composing templates works now (this is the index templates v2 feature that has been introduced with 7.8).

Yeah we could do that. So if I'm tracking correctly, currently the registry has the ability to send mappings and settings over as full component templates. That's what we're doing with the base package I believe right? https://github.com/elastic/package-registry/tree/master/testdata/package/base/0.2.0/elasticsearch/component-template

That is then installed here:

await installPreBuiltComponentTemplates(paths, callCluster);

So would the easiest path here to just create a similar mapping and settings file within our package to set the dynamic mapping stuff there?

Otherwise if we want to leverage the functionality in elastic/package-registry#552 we'll need to construct a component template in kibana based on the values that are set in dataset's manifest.yml file.

Am I thinking about this correctly?

@skh
Copy link
Contributor

skh commented Jun 30, 2020

You can use pre-built templates, but I was thinking more in that direction:

Otherwise if we want to leverage the functionality in elastic/package-registry#552 we'll need to construct a component template in kibana based on the values that are set in dataset's manifest.yml file.

We have tracked this here: #64760

@ruflin
Copy link
Contributor

ruflin commented Jun 30, 2020

++ on using components for each part. This will make it simpler to update them independent.

@jonathan-buttner
Copy link
Contributor Author

PR to add the functionality is here: #70517

@ruflin ruflin assigned jonathan-buttner and unassigned ruflin Jul 2, 2020
@ruflin
Copy link
Contributor

ruflin commented Jul 2, 2020

@jonathan-buttner Assigned you here, hope that is ok. @ph FYI

@jonathan-buttner
Copy link
Contributor Author

Closing, implemented here: #70517

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New value added to drive a business result Team:Endpoint Data Visibility Team managing the endpoint resolver Team:Fleet Team label for Observability Data Collection Fleet team
Projects
None yet
Development

No branches or pull requests

4 participants