Allow rollover alias to use reference like agent.version or agent.name #12285
Conversation
|
@simitt I think this bug is a hard place, if we make auto append of the version for the rollover alias we are a risk of breaking existing configuration, instead I've decided to allow the And WDYT about backporting it down to 7.0? |
|
It was not implemented on purpose initially. I think historically we allowed too much flexibility in the index patterns used which caused issues like no template used, dashboard does not show the data etc. Moving forward we should be more strict on what we allow. I would like to learn more on what the use case here is that this solves. Seems in #12233 the goal was to get almost the same behaviour as the default? |
I do understand we want to move to a more strict system, flexibility isn't easy to deal with when you try to give a nice user experience (and uniform experience) but also I do understand that in some case allowing people to override options is a good things.
Yes, my point is more if you do override de defaults you are more on the path to choose your own adventure. So in that case I though it made more sense to not auto append the version to the alias name, but still allow user to get it by using field reference. |
|
During the migration from 5 to 6 and 6 to 7 or upgrades in minors were often caused by users not having the version inside. So I'm very hesitant to allow this again. If a user wants to go on his own adventure and not have versioning, the old indexing scheme could be used and ILM alias have to be set up manually. With this any flexibility is possible and using any field in the index name is possible. Perhaps @fpompermaier can chime to describe his use case a bit more? |
|
Actually I don't have any use case, I was just playing around with variables and I discovered that the documentation is wrong |
|
@ruflin The things is they can still decide to not have the version in the index name, no? |
|
Yes, I think so, but not convinced it's a good thing ;-) As you see I'm on the fence here so if you think this should go in, please move forward. |
|
I agree with this PRs take on fixing this issue |
|
@urso WDYT? |
|
ping @urso urso |
|
I agree on having this change and add support for To me this change doesn't really change anything in how flexible users can configure beats. No matter if we're strict or not, it should be consistent. There are a many resources that need to be configured when preparing an index. This requires a number of settings to be applied consistently. The more 'severe' issue is that we have a number of related settings, which need to be configured independently, but in a consistent manner instead of relying on a common set of settings. |
libbeat/idxmgmt/ilm/config.go
Outdated
| return fmt.Errorf("rollover_alias must be set when ILM is not disabled") | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| func defaultConfig(info beat.Info) Config { | ||
| name := fmt.Sprintf("%s-%s", info.Beat, info.Version) | ||
| const name = "%{[beat.name]}-%{[beat.version]}" |
There was a problem hiding this comment.
I think beat.name can be overwritten using the name setting. Also beat.version is deprecated and should be agent.version.
Proposal: name := info.Beat + "-%{[agent.version]}"
Previously you were not able to defined fields in the roll over alias, this commit allow to use some of the global values. We also update the docs to remove the prefix part. Fixes elastic#12233
ph
force-pushed
the
fix/fix-ilm-rollover-alias
branch
from
7d3a704 to
b087738
Compare
|
@urso I've made the change can you take another look? |
|
jenkins test this please |
|
Jenkins, test this. |
|
unrelated issue |
|
@ph Any idea when it's going to be released? |
Previously you were not able to defined fields in the roll over alias,
this commit allow to use some of the global values. We also update the
docs to remove the prefix part.
Fixes #12233