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

ILM: restrict customization of alias to suffix. #3905

Merged
merged 5 commits into from Jun 23, 2020

Conversation

simitt
Copy link
Contributor

@simitt simitt commented Jun 18, 2020

Motivation/summary

Change how rollover aliases can be configured to only allow customizing a suffix. This change intends to make the ILM setup less error prone and to make the transition to the new index_templates and data streams easier.

Checklist

- [] I have signed the Contributor License Agreement.

I have considered changes for:

How to test these changes

Configure index_suffix for event types in apm-server.ilm.setup.mappings and check that templates are created accordingly.

Related issues

Follow up on #3826
closes #3895

Change how rollover aliases can be configured to only allow
customizingi a suffix.This change intends to make the ILM
setup less error prone and to make the transition to the
new index_templates and data streams easier.

Follow up on elastic#3826
closes elastic#3895
@apmmachine
Copy link
Collaborator

apmmachine commented Jun 18, 2020

💚 Build Succeeded

Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: [Pull request #3905 updated]

  • Start Time: 2020-06-23T07:50:13.010+0000

  • Duration: 45 min 59 sec

Test stats 🧪

Test Results
Failed 0
Passed 3217
Skipped 147
Total 3364

Steps errors

Expand to view the steps failures

  • Name: Compress

    • Description: tar --exclude=coverage-files.tgz -czf coverage-files.tgz coverage

    • Duration: 0 min 0 sec

    • Start Time: 2020-06-23T08:04:48.376+0000

    • log

  • Name: Compress

    • Description: tar --exclude=system-tests-linux-files.tgz -czf system-tests-linux-files.tgz system-tests

    • Duration: 0 min 0 sec

    • Start Time: 2020-06-23T08:16:29.237+0000

    • log

  • Name: Test Sync

    • Description: ./script/jenkins/sync.sh

    • Duration: 2 min 29 sec

    • Start Time: 2020-06-23T07:59:51.203+0000

    • log

if err != nil {
return Config{}, errors.Wrap(err, "variable part of rollover_alias cannot be resolved")
return Config{}, errors.Wrap(err, "variable part of index suffix cannot be resolved")
Copy link
Contributor

Choose a reason for hiding this comment

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

variable part of index suffix ==> isn't this just the "index"?

Copy link
Contributor Author

@simitt simitt Jun 23, 2020

Choose a reason for hiding this comment

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

Technically yes, but everything except the suffix is fixed by us. The error message reuses the term index suffix as this is what the users can configure and are used to from the apm-server.yml.

apm-server.yml Outdated
# need to be changed accordingly to match the created indices
# The configured event types, policies and index suffixes will be merged with the default setup.
# Only configure the mappings that should be customized.
# Index names will always start with `apm-%{[observer.version]}-%{[event.type]}, e.g. apm-7.9.0-span.
Copy link
Contributor

Choose a reason for hiding this comment

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

I find the phrasing in all this section a bit confusing/convoluted.
Can't we just say that index are named like apm-%{[observer.version]}-%{[event.type]}-{mapping.index_suffix}, where {mapping.index_suffix} is the setting bellow and it is optional?
Or something like that, I don't know. Just an idea. Or maybe seek assistance from professionals...

Also confused about "Only configure the mappings that should be customized": not sure if that is trivial or if I am missing something.
Also the last note, "ensure they are aligned with the ILM indices": what does "aligned" mean? what are "ILM indices" (aren't they all ILM)? etc.

Copy link
Member

Choose a reason for hiding this comment

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

I like Juan's idea here. What about something like this?

Customized mappings will be merged with the default setup, so you only need to configure mappings for the event types, policies, and index suffixes that you want to customize.
Indices are named in this way: `apm-%{[observer.version]}-%{[event.type]}-{index_suffix}`, e.g., apm-7.9.0-span-custom*.
The `index_suffix` is optional.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you both, I changed the wording.

Copy link
Member

@bmorelli25 bmorelli25 left a comment

Choose a reason for hiding this comment

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

I have a few docs suggestions.

docs/ilm-reference.asciidoc Outdated Show resolved Hide resolved
docs/ilm-reference.asciidoc Outdated Show resolved Hide resolved
docs/ilm.asciidoc Outdated Show resolved Hide resolved
docs/ilm.asciidoc Outdated Show resolved Hide resolved
docs/ilm.asciidoc Outdated Show resolved Hide resolved
docs/ilm.asciidoc Outdated Show resolved Hide resolved
docs/ilm.asciidoc Outdated Show resolved Hide resolved
docs/ilm.asciidoc Outdated Show resolved Hide resolved
docs/ilm.asciidoc Outdated Show resolved Hide resolved
apm-server.yml Outdated
# need to be changed accordingly to match the created indices
# The configured event types, policies and index suffixes will be merged with the default setup.
# Only configure the mappings that should be customized.
# Index names will always start with `apm-%{[observer.version]}-%{[event.type]}, e.g. apm-7.9.0-span.
Copy link
Member

Choose a reason for hiding this comment

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

I like Juan's idea here. What about something like this?

Customized mappings will be merged with the default setup, so you only need to configure mappings for the event types, policies, and index suffixes that you want to customize.
Indices are named in this way: `apm-%{[observer.version]}-%{[event.type]}-{index_suffix}`, e.g., apm-7.9.0-span-custom*.
The `index_suffix` is optional.

@simitt simitt merged commit fc3b62a into elastic:master Jun 23, 2020
@simitt simitt deleted the ilm-customizable-suffix branch June 23, 2020 09:01
simitt added a commit to simitt/apm-server that referenced this pull request Jun 23, 2020
Change how rollover aliases can be configured to only allow
customizing a suffix. This change intends to make the ILM
setup less error prone and to make the transition to the
new index_templates and data streams easier.

Follow up on elastic#3826
closes elastic#3895

Co-authored-by: Brandon Morelli <bmorelli25@gmail.com>
simitt added a commit that referenced this pull request Jun 23, 2020
Change how rollover aliases can be configured to only allow
customizing a suffix. This change intends to make the ILM
setup less error prone and to make the transition to the
new index_templates and data streams easier.

Follow up on #3826
closes #3895

Co-authored-by: Brandon Morelli <bmorelli25@gmail.com>
jalvz added a commit to jalvz/apm-server that referenced this pull request Jul 2, 2020
@axw axw mentioned this pull request Jul 21, 2020
4 tasks
@axw
Copy link
Member

axw commented Jul 21, 2020

Verified with BC2. Procedure:

  • Created stack with apm-it
  • Added apm-server config -E apm-server.ilm.setup.mapping=[{event_type: 'transaction', index_suffix: 'suffolks'}]
  • Started apm-server
  • Checked that (only) the transaction index has "-suffolks" appended:
green open apm-7.9.0-transaction-suffolks-000001 PnTAEchiQOy9aMK5wXSZXw 1 0   10 0   203kb   203kb
green open apm-7.9.0-metric-000001               5_5GltpHSrCVbTFuCe_NpA 1 0  107 0 137.8kb 137.8kb
green open apm-7.9.0-profile-000001              EaNGAmL9SyKfBjfHxKWaKg 1 0  388 0 753.5kb 753.5kb
green open apm-7.9.0-error-000001                EFqjo-qTSnuWLH-9vRSJLw 1 0    0 0    208b    208b
green open apm-7.9.0-span-000001                 su3VI2PQTIGUITaRFFHw_Q 1 0 2174 0 667.5kb 667.5kb
green open apm-7.9.0-onboarding-2020.07.21       zcJLiM4dRxGmGmhHMsoEUA 1 0    5 0    33kb    33kb
  • Checked that the transaction index has the expected ILM policy and rollover alias:
  "apm-7.9.0-transaction-suffolks-000001" : {
    "settings" : {
      "index" : {
        "lifecycle" : {
          "name" : "apm-rollover-30-days",
          "rollover_alias" : "apm-7.9.0-transaction-suffolks"
        },
        ...

I initially started apm-server before updating config, which caused the template to be created without the suffix, restarted the server, and had two templates. The default template name ended up matching the created indices, so I had to delete everything and restart the server again.

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.

[ILM] Only allow custom suffixes for ILM aliases
5 participants