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

Add upgrade docs #11378

Merged
merged 12 commits into from
Apr 2, 2019
Merged

Add upgrade docs #11378

merged 12 commits into from
Apr 2, 2019

Conversation

dedemorton
Copy link
Contributor

@dedemorton dedemorton commented Mar 21, 2019

Initial draft of upgrade docs. This version simply modifies the existing content from 6.x. It's something we can build on for 7.x. We can even change for format if we feel it didn't work well for 6.x.

@urso I know there's a lot missing/incorrect here. I'm sure ECS has implications on migration. I hope that we will provide some kind of tooling to help folks migrate their configs. Can you get the team to help me flesh out this content? They can suggest changes, add comments, or whatever works best. I can create a feature branch, or give people permission to push to my branch.

I haven't hooked this into the platform reference yet because I don't want it to be in the published docs until we're further along.

To do: (feel free to edit this list or add a comment below):

@dedemorton dedemorton added docs in progress Pull request is currently in progress. v7.0.0 labels Mar 21, 2019
@dedemorton dedemorton requested a review from a team as a code owner March 21, 2019 17:40
@dedemorton dedemorton requested a review from urso March 21, 2019 17:40
Copy link
Member

@ruflin ruflin left a comment

Choose a reason for hiding this comment

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

Leaving a few notes on other potential migration pitfalls we should document:

  • Users using Logstash and having just 1 index or template. Going to 7.0 things will stop ingesting because of field conflict. Users need to use versioned templates and indices.
  • Dashboard upgrade: If users want to run 6.7 and 7.0 metricbeat in parallel, migration flag has to be used and old dashboards have to stay around.
  • Overwriting index pattern: Users upgrading all Beats to 7.0 should / must overwrite the Kibana index patter.

@ruflin Edited this list so I can mark off the items I feel we've covered or that we've decided not to cover. Please confirm by reviewing the changes I've pushed for this PR.

libbeat/docs/upgrading-to-7.asciidoc Outdated Show resolved Hide resolved
libbeat/docs/upgrading-to-7.asciidoc Outdated Show resolved Hide resolved
libbeat/docs/upgrading-to-7.asciidoc Outdated Show resolved Hide resolved
@dedemorton
Copy link
Contributor Author

@karenzone FYI

@dedemorton
Copy link
Contributor Author

There are some upgrade-related comments in this issue that need to be addressed: #11276

@ruflin
Copy link
Member

ruflin commented Mar 27, 2019

@dedemorton For the field changes we have a generated list in https://www.elastic.co/guide/en/beats/libbeat/master/breaking-changes-7.0.html Instead of just having 1 list, we potentially need it per Beat or a different place. We could also do a different format. Let me know what is the best way to document these and make it visible also in the upgrade docs.

@dedemorton
Copy link
Contributor Author

Noting here that there is a related discussion about upgrading dashboards in #11269.

@dedemorton
Copy link
Contributor Author

dedemorton commented Mar 27, 2019

@ruflin

Instead of just having 1 list, we potentially need it per Beat or a different place.

Let's start out with the list where it's stored currently, and I can point to it from the release notes. I think we'll get a lot of feedback next week from the group that's testing the upgrade docs. Then we can focus on improving the structure and flow based on specific problems that testers encounter.

@dedemorton
Copy link
Contributor Author

@ruflin

Overwriting index pattern: Users upgrading all Beats to 7.0 should / must overwrite the Kibana index patter.

I feel a little dense for asking, but when you say that users must overwrite the index pattern, do you mean that they must set setup.template.overwrite: true? Or is there some other setting I don't know about?

@ruflin
Copy link
Member

ruflin commented Mar 28, 2019

Leaving a few more notes here after helping some upgrades:

  • In the LS docs here we have an example with Beats as Input and ES as output but we don't use the metadata for the index names. If someone uses this example (and I think many users do) we basically recommend something they shouldn't use. We should modify this example: https://www.elastic.co/guide/en/logstash/current/advanced-pipeline.html#indexing-parsed-data-into-elasticsearch Potentially also add there a note about the template.
  • Users not using the ES output will have to do quite a few manual steps during upgrading and in the right order. Like making sure the right template with the right name is loaded before any data is ingested. We should also have some debugging notes, on what users can do to debug that the template was not loaded correctly (for example dynamic fields do not exist).
  • I expect quite a few users to first ingest data and not have migration flag enabled and then realise they need it, we should have docs on how to enabled it, wipe old data and overwrite template.

Minor edit to make these items a checklist for easier tracking ^^ [DM]

@dedemorton
Copy link
Contributor Author

dedemorton commented Mar 28, 2019

@karenzone Not Nicolas' comment above:

In the LS docs here we have an example with Beats as Input and ES as output but we don't use the metadata for the index names. If someone uses this example (and I think many users do) we basically recommend something they shouldn't use. We should modify this example: https://www.elastic.co/guide/en/logstash/current/advanced-pipeline.html#indexing-parsed-data-into-elasticsearch Potentially also add there a note about the template.

@dedemorton
Copy link
Contributor Author

@ruflin

Some follow-up questions:

Users not using the ES output will have to do quite a few manual steps during upgrading and in the right order

Can you be more specific so I can add this to the doc?

I expect quite a few users to first ingest data and not have migration flag enabled and then realise they need it, we should have docs on how to enabled it, wipe old data and overwrite template.

By wiping the old data, do you mean deleting the index, or is there a less dramatic way to do this?

@ruflin
Copy link
Member

ruflin commented Mar 29, 2019

Some more notes from an other issue:

  • Document how field aliases can be added to all indicies and which aliases have to be added to make data from MB < 6.6 compatible with Infrastructure / Logs UI. Potentially provide a script.
  • Document how to migrate from 6.7 to 7.0 for Infrastructure / Logging UI (not different from rest of stack, so perhaps just link to it).
  • Document which fields were changed / breaking changes which affect the solutions
  • Document how to use migration flag in Beats to have 6.x dashboards compatible.

@ruflin
Copy link
Member

ruflin commented Mar 29, 2019

@dedemorton I was thinking about the index pattern in Kibana. But it turned out I don't think we need it.

For the migration steps with other outputs:

  • Users have to load the index template with the correct version to ES before ingesting any data. @cjcenizal What else was there?

For the deletion: Yes, the index has to be deleted with the incorrect mapping. I don't think there is an other way :-(

@cjcenizal
Copy link

Users have to load the index template with the correct version to ES before ingesting any data. What else was there?

If users are ingesting Beats data via Logstash, they also need to ensure that the resulting index name (defined by the Logstash configuration) will match the index pattern in the Beats index template. So for example, if the Beats index template expects indices to match metricbeat-7.0.0-rc1-* but the Logstash config ingests documents into the metricbeat-7-{date} index, then things will break in a very opaque manner.

@dedemorton
Copy link
Contributor Author

@ruflin A few more follow up questions for your solution-docs list:

which aliases have to be added to make data from MB < 6.6 compatible with Infrastructure / Logs UI. Potentially provide a script.

Do users on Metricbeat versions < 6.6 need to set up the aliases manually? If so, that's not a very good experience for the type of user we are targeting. We should provide a migration script. Is that something we can get in place for 7.0?

Document how to migrate from 6.7 to 7.0 for Infrastructure / Logging UI (not different from rest of stack, so perhaps just link to it). Document which fields were changed / breaking changes which affect the solutions

Can you identify the fields and breaking changes that only affect solutions? For the list of fields, we can probably take the existing list and add some conditional logic to create something specific for solutions. Not sure how to best do the breaking changes. IMO, we can't expect folks using our curated UIs to spend time digging through the breaking changes in the release notes. We need to make the upgrade as simple and clear as possible (for all users, but especially folks using our curated UIs).

Copy link

@cjcenizal cjcenizal left a comment

Choose a reason for hiding this comment

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

This is looking terrific! Had a few suggestions.

configuration options is renamed to `<beatname>.reference.yml` starting with
Beats 6.0. We recommend using this file as a reference only. It's not intended
to be used in production.
Before upgrading to 7.0, make sure {ls} uses versioned templates and indices.

Choose a reason for hiding this comment

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

I know it's implied, but maybe it's worth explicitly stating, "If you're using Logstash to ingest your Beats data..." so that people won't think LS is required for upgrading.

Choose a reason for hiding this comment

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

Also, what does "versioned templates" mean in this case? The link below goes to plugins-inputs-beats.html which currently only mentions versioned indices.

Choose a reason for hiding this comment

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

CC @ruflin ^

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm modifying this section a bit to show the settings (including manage_template => false, which let's Beats manage the template).


We recommend that you fully upgrade {es} and {kib} to version 7.0
before upgrading {beats}. If you're on Beats 6.0 through 6.6,
<<upgrading-to-6.7,upgrade to Beats 6.7>> *before* doing the {es} upgrade.

Choose a reason for hiding this comment

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

Really minor, but when I was reading the generated doc, I saw this link and immediately thought to open it in a new tab because it looks like a link to a related resource. However, it's a link to the anchor tag a few lines down. Does it help the user that much or do we think that they'll find the content naturally and intuitively without the link? It might be worth removing it if so. Just a thought.

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yah that's a valid point. I'll remove the link.

configuration file.
// REVIEWERS: I'm not sure the following statement is correct. I was not able
// to use the dashboards I saved from 6.x with 7.0. I did a very quick and dirty
// upgrade of the stack tho so might have missed something critical.
Copy link

Choose a reason for hiding this comment

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

@ruflin Any gotchas/limitations we want to mention here?

@dedemorton dedemorton added the needs_backport PR is waiting to be backported to other branches. label Apr 1, 2019

[source,shell]
------------------------------------------------------------------------------
----
curl -XPUT -H'Content-Type: application/json' http://localhost:9200/_template/metricbeat -d @metricbeat.template.json
Copy link
Member

Choose a reason for hiding this comment

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

We should not recommend this as users must put in the correct version for the template. Already this example is wihtout the version so we rather remove it completely and force users to use the beat directly to load it which will to the right thing.

Choose a reason for hiding this comment

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

If we're going to drop this, it might be worth adding a note that it's no longer recommended:

Note: In previous versions, we advised users to create the Metricbeat index template from the command line. This is no longer recommended. Use the metricbeat command outlined above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

yes

Copy link
Member

Choose a reason for hiding this comment

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

At least there we have version numbers but I would rather recommend loading it with running the beat TBH.

libbeat/docs/upgrading.asciidoc Show resolved Hide resolved
libbeat/docs/upgrading.asciidoc Outdated Show resolved Hide resolved
libbeat/docs/upgrading.asciidoc Outdated Show resolved Hide resolved
libbeat/docs/upgrading.asciidoc Outdated Show resolved Hide resolved
libbeat/docs/upgrading.asciidoc Outdated Show resolved Hide resolved
libbeat/docs/upgrading.asciidoc Show resolved Hide resolved
libbeat/docs/upgrading.asciidoc Show resolved Hide resolved
libbeat/docs/upgrading.asciidoc Outdated Show resolved Hide resolved
libbeat/docs/upgrading.asciidoc Show resolved Hide resolved
Copy link

@cjcenizal cjcenizal left a comment

Choose a reason for hiding this comment

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

This looks great! I had a few suggestions, but if you or @ruflin disagree with anything I suggested then I'll defer to the two of you.

libbeat/docs/upgrading.asciidoc Outdated Show resolved Hide resolved
libbeat/docs/upgrading.asciidoc Outdated Show resolved Hide resolved

[source,shell]
------------------------------------------------------------------------------
----
curl -XPUT -H'Content-Type: application/json' http://localhost:9200/_template/metricbeat -d @metricbeat.template.json

Choose a reason for hiding this comment

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

If we're going to drop this, it might be worth adding a note that it's no longer recommended:

Note: In previous versions, we advised users to create the Metricbeat index template from the command line. This is no longer recommended. Use the metricbeat command outlined above.

libbeat/docs/upgrading.asciidoc Outdated Show resolved Hide resolved
libbeat/docs/upgrading.asciidoc Outdated Show resolved Hide resolved
Copy link
Contributor

@webmat webmat left a comment

Choose a reason for hiding this comment

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

I've noted a few small things and one bigger worry about Kibana index templates.

As discussed with @dedemorton, we can proceed to merge & backport, regardless of my review comments.

We need to publish a preliminary version of this page today, for people to be able to start testing.

We'll keep iterating in another PR.

libbeat/docs/upgrading.asciidoc Outdated Show resolved Hide resolved
[source,json]
----
manage_template => false
index => "%{[@metadata][beat]}-%{[@metadata][version]}-%{+YYYY.MM.dd}"
Copy link
Contributor

Choose a reason for hiding this comment

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

Will @metadata still be populated in 7.0? This was a Logstash-only concept, correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, I believe the metadata is just part of the event...not just a LS concept.

libbeat/docs/upgrading.asciidoc Outdated Show resolved Hide resolved
libbeat/docs/upgrading.asciidoc Show resolved Hide resolved
libbeat/docs/upgrading.asciidoc Outdated Show resolved Hide resolved
@dedemorton dedemorton merged commit 0caa944 into elastic:master Apr 2, 2019
@dedemorton dedemorton deleted the update_migration_info branch April 2, 2019 20:05
dedemorton added a commit to dedemorton/beats that referenced this pull request Apr 2, 2019
* Add upgrade docs

* Apply suggestions from code review

* Add more detail based on review feedback

* A few clarifications

* Remove old file

* Rename file to original name

* More changes from review

* Add changes from review before moving stuff around

* Move some content and clean it up

* Add more fixes from the review

* Add more changes based on review comments
dedemorton added a commit that referenced this pull request Apr 2, 2019
* Add upgrade docs

* Apply suggestions from code review

* Add more detail based on review feedback

* A few clarifications

* Remove old file

* Rename file to original name

* More changes from review

* Add changes from review before moving stuff around

* Move some content and clean it up

* Add more fixes from the review

* Add more changes based on review comments
@dedemorton dedemorton removed the needs_backport PR is waiting to be backported to other branches. label Apr 2, 2019
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.

5 participants