-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
Support for reindexing APM indices #29845
Conversation
Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>
Pinging @elastic/kibana-operations |
@@ -30,12 +32,24 @@ export default function apmOss(kibana) { | |||
indexPattern: Joi.string().default('apm-*'), | |||
|
|||
// ES Indices | |||
sourcemapIndices: Joi.string().default('apm-*'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was missing.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
x-pack/plugins/upgrade_assistant/public/components/tabs/checkup/deprecations/list.tsx
Outdated
Show resolved
Hide resolved
...grade_assistant/public/components/tabs/checkup/deprecations/reindex/flyout/warnings_step.tsx
Outdated
Show resolved
Hide resolved
x-pack/plugins/upgrade_assistant/server/lib/reindexing/reindex_service.ts
Show resolved
Hide resolved
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>
💚 Build Succeeded |
Regarding template conflicts during reindexing: since we know there will most probably be conflicts while reindexing when using default settings, I suggest to either show the user upfront that they will need to clean up the templates or prompt for removal when the migration script is started. If feasible I'd prefer the options where we detect potential problematic templates and directly show them to the user, over pointing to an external documentation asking the user to query for the templates themselves. Regarding linking to the documentation, there is also a general APM release notes section. This section should give a high level overview of breaking changes for APM in general, and then link to the more detailed server section. It should also contain explanation around switching to ECS. I'd rather link to these more general APM docs from Kibana.
How about we put the detail information around field changes, explaining why we move to ECS and linking to it, into the APM docs and only put some more general information into the migration assistant, e.g. @bmorelli25 I'd appreciate your input regarding documentation linking and wording. |
Yup, let's link to this release notes section from Kibana. From there, I'll handle the linking to other sections of the APM documentation with more details.
I mostly agree with this. We should have a sentence (for the people who are curious, but not that curious) and the bulk of information should be in the APM documentation. I also thinks it makes more sense to remove the "More about ECS" link from this page. That's something I can link to from the APM documentation. I think directing users to the APM documentation initially is more beneficial. In other words. Instead of this:
I'm thinking something more like this: This index will be converted to the 7.x format |
We're working through an issue with conflicting field mapping, summarized with this example:
|
Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>
This comment has been minimized.
This comment has been minimized.
💚 Build Succeeded |
@graphaelli should we move forward with merging this PR and treat the ES error as a bug? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code LGTM, tested with APM data and was able to see the data in the APM app after reindexing. I'm out of the loop on the template conflict issue, but assuming there's a fix for that, the rest of this looks good.
I don't understand the conflict very well. The conflict is between 6.6 index mappings and 7.0 mappings, or between 7.0 mappings and 7.0-reindexed mappings? (If the former: why having a different version causes conflicts? If the later: doesn't make sense to me, what I am missing?) And silly question: does it have to be necessarily a |
Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>
💚 Build Succeeded |
We decided to get around the index template conflicts by pre-pending to the index name, which did here: #30166 |
Starting in 7.0, APM will be aligning with the property names defined in the Elastic Common Schema. When APM users upgrade to 7.x, they will need to migrate those indices created prior to 7.0 for the data to be present in the APM app.
Blocked by the finalizing of mappings and the reindex script here: elastic/apm-integration-testing#277
Discussion:
When we re-index, we are appending
-reindex-v7
to the current index name. With doing this, we will almost always run into conflicts with the APM template used to create the index in the first place.By default, APM creates an index template named
apm-%{[beat.version]}
with an index pattern ofapm-%{[beat.version]}-*
. The default index names are as follows:apm-%{[beat.version]}-sourcemap
apm-%{[beat.version]}-error-%{+yyyy.MM.dd}
apm-%{[beat.version]}-transaction-%{+yyyy.MM.dd}
apm-%{[beat.version]}-span-%{+yyyy.MM.dd}
apm-%{[beat.version]}-metric-%{+yyyy.MM.dd}
apm-%{[beat.version]}-onboarding-%{+yyyy.MM.dd}
Unfortunately, there is no way to ignore index templates on index creation or update an index pattern to not match indices ending in
-reindexed-v7
There have been a few options which have been discussed which require input from the APM team. @elastic/apm-server @bleskes
/_template?filter_path=*.index_patterns
to see if any patterns match.Please also review the language used in the following screenshots and let us know if it should be changed.
Documentation links to: https://www.elastic.co/guide/en/apm/server/master/breaking-changes.html
More about ECS links to: https://github.com/elastic/ecs
When clicking on "Reindex" for an APM index, currently presented is a single warning which the user must check acknowledge the data will be transformed.
Checklist
Use
strikethroughsto remove checklist items you don't feel are applicable to this PR.For maintainers