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

[Fleet] Analyzer in index template settings are not working #128209

Closed
nchaulet opened this issue Mar 21, 2022 · 9 comments · Fixed by #128498
Closed

[Fleet] Analyzer in index template settings are not working #128209

nchaulet opened this issue Mar 21, 2022 · 9 comments · Fixed by #128498
Assignees
Labels
blocker bug Fixes for quality problems that affect the customer experience impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. Team:Fleet Team label for Observability Data Collection Fleet team v8.2.0

Comments

@nchaulet
Copy link
Member

Description

It's not possible to install the windows package anymore you got the following error

{"body":{"statusCode":500,"error":"Internal Server Error","message":"mapper_parsing_exception: [mapper_parsing_exception] Reason: Failed to parse mapping: analyzer [powershell_script_analyzer] has not been configured in mappings"},"status":500,"took":22.412}

I think it's related to moving the mapping outside of the main index template, as the analyzer is define in index template settings.

@nchaulet nchaulet added bug Fixes for quality problems that affect the customer experience impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. Team:Fleet Team label for Observability Data Collection Fleet team v8.2.0 labels Mar 21, 2022
@elasticmachine
Copy link
Contributor

Pinging @elastic/fleet (Team:Fleet)

@hop-dev
Copy link
Contributor

hop-dev commented Mar 24, 2022

@nchaulet One approach we could take is to merge the @mappings and @settings templates to prevent this error. I already had to do a similar hack where we have to port over settings.index.mapping from the settings template to the mappings template in the case where APM overrides the nested fields limit.

@joshdover suggested this could be called the @package template as it contains all the info generated from the package. I don't think it would be a lot of work other than all the tests it will break.

@nchaulet
Copy link
Member Author

@nchaulet One approach we could take is to merge the @mappings and @settings templates to prevent this error. I already had to do a similar hack where we have to port over settings.index.mapping from the settings template to the mappings template in the case where APM overrides the nested fields limit.
@joshdover suggested this could be called the @Package template as it contains all the info generated from the package. I don't think it would be a lot of work other than all the tests it will break.

Yes this seems a good solution, I have a working solution now where I just moved the analisys part from @settings to @mappings, but I guess it's not as future proof than a new @package template .

@hop-dev
Copy link
Contributor

hop-dev commented Mar 24, 2022

@ruflin just checking you are happy, we are proposing merging the @mapping and @settings component templates into one component template called @package. This prevents us having to port over any mapping related settings from the settings component template to the mappings component template (which has already caused 2 bugs).

before 8.2 these templates only appeared if a package set custom mappings or settings.

@ruflin
Copy link
Member

ruflin commented Mar 28, 2022

It always felt natural to separate data stream settings from mappings but unfortunately there are 2 types of settings I think. Some settings like analyzer belong to the mappings and others like shards change are more related to data streams / indices setting itself. I wonder if for some users this could be a breaking change. My understanding is that both @mappings and @settings should not have been modified by users but there are chances these were modified. Under @custom which the users could modified, we combine @mappings and @settings?

There is a bigger concern related to our constant changes to templates, naming of templates etc. I expect in many of these changes we rollover all data streams to make sure the changes are applied. This is convenient for us but less then ideal for Elasticsearch as by just upgrading we double the number of indices. If someone has a rollover of 90 days, 50GB but each upgrade a rollover happens, the number of indices keeps increasing.

Do we know how many packages have these custom mappings?

@joshdover
Copy link
Contributor

I wonder if for some users this could be a breaking change. My understanding is that both @mappings and @settings should not have been modified by users but there are chances these were modified.

I get where this is coming from, but if we can't treat these assets as managed then we can't make any changes, which is the whole purpose of making them managed. If this is a problem that impacts many customers, we need to prioritize blocking edits to managed assets. I think we should consider that separate from making the template structure coherent overall.

Under @custom which the users could modified, we combine @mappings and @settings?

Yes, we have a single @custom template that can contain both mappings and settings. Prior to #124013 this component template could not override the data stream mappings defined by the package, which was a bug. I don't think combining these templates into @package changes anything else though.

There is a bigger concern related to our constant changes to templates, naming of templates etc.

Agreed. The good news here is that this change wouldn't introduce a new rollover since we're already making a change in 8.2. We do make a best-effort attempt to only apply a rollover if there's a breaking change in mappings, which would only be the case if the user had mapping overrides in @custom that weren't being applied before (which again, was a bug). I don't expect this change to require a rollover for the vast majority of users.

As we stabilize the template structure, we should have a goal of minimizing future changes and be sure the model we're moving to can support all use cases we intend to support. I think we're getting quite close to that with this change and #121118

@ruflin
Copy link
Member

ruflin commented Mar 30, 2022

I remember @hop-dev put together somewhere a list of existing templates. Lets make sure we have this change documented. Ideally, we would have this documented in public for everyone to look up.

@joshdover
Copy link
Contributor

I've opened a documentation issue here: elastic/ingest-docs#111 and have updated #121118 with the existing scheme as of 8.2 and the new proposed scheme

@hop-dev
Copy link
Contributor

hop-dev commented Mar 30, 2022

I've created a dev doc for the template hierarchy and some other bits while we wait for something in observability docs

#128896

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocker bug Fixes for quality problems that affect the customer experience impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. Team:Fleet Team label for Observability Data Collection Fleet team v8.2.0
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants