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

[WIP] Update breaking changes doc for 7.0 #10632

Merged
merged 12 commits into from
Apr 8, 2019

Conversation

karenzone
Copy link
Contributor

No description provided.

@karenzone karenzone added the docs label Apr 3, 2019
@karenzone
Copy link
Contributor Author

FYI @guyboertje @colinsurprenant

Update structure to allow for previous and current changes
Copy link
Member

@yaauie yaauie 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 added a proposal for the plugin breaking changes, and a note about how we should call out the upstream breaking changes to the payloads Beats sends that I hope adds more clarity.

docs/static/breaking-changes.asciidoc Show resolved Hide resolved
@@ -29,7 +30,30 @@ and have the persistent queue enabled, we strongly recommend that you drain or
delete the persistent queue before you upgrade. See <<upgrading-logstash-pqs>>
for information and instructions.


Copy link
Member

Choose a reason for hiding this comment

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

section should be above breaking-pq section to maintain newest-first ordering

docs/static/breaking-changes.asciidoc Show resolved Hide resolved
docs/static/breaking-changes.asciidoc Outdated Show resolved Hide resolved

Beats fields have been renamed to map to Elastic Common Schema (ECS).

Starting with 7.0, the fields exported by {beats} conform to the
Copy link
Member

Choose a reason for hiding this comment

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

🤔

These breaking changes aren't in the Beats Input itself, but in the payloads that Beats sends. If a user upgrades Logstash before upgrading Beats, the payloads will continue to have the pre-ECS schema, and if they upgrade their Beats before upgrading Logstash they'll get payloads with the ECS schema in advance of any Logstash upgrade.

I would rather add this as a call-out than in bullet-points. See my proposed diff above for a WARNING call-out.

yaauie and others added 3 commits April 4, 2019 13:42
Adds info about field reference parser

Co-Authored-By: karenzone <35154725+karenzone@users.noreply.github.com>
Co-Authored-By: karenzone <35154725+karenzone@users.noreply.github.com>
exported fields have been renamed, so you may need to modify your pipeline
configurations to access them at their new locations prior to upgrading your
Beats. See {beats-ref}/breaking-changes-7.0.html[Beats Breaking changes in 7.0]
for the full list of changed names.
Copy link
Member

Choose a reason for hiding this comment

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

Let's add a note here that seeing a mapping conflict after upgrade is an indication that is change is influencing the data reaching existing indices.

These plugins were removed from the 7.0 default bundle based on usage data.
You can still install these plugins manually.

* removed-plugin-1
Copy link
Member

Choose a reason for hiding this comment

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

only the graphite output stopped being supported but it's still in the package

Copy link
Contributor Author

@karenzone karenzone Apr 4, 2019

Choose a reason for hiding this comment

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

Any notation required around that?
I'll remove this section entirely.

Would it be helpful to suggest that the user revisit the list of supported plugins as of 7.0? Or am I overthinking this. :-)

Copy link
Member

Choose a reason for hiding this comment

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

I think we can remove this section, commercial support is something only for our customers, which can be checked in our support matrix page

@karenzone
Copy link
Contributor Author

@robbavey @yaauie @jsvd What about ILM on by default? Does that belong in the elasticsearch-output plugin? Or is that more inline with the Beats/ECS info (in that it should be more prominent)?

[float]
===== Elasticsearch output

ILM on by default
Copy link
Member

Choose a reason for hiding this comment

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

@yaauie can we add here that in 7.0 ILM will be auto detected and enabled by default if the ES cluster supports it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here's a peek at what's coming in my next commit:

* {es} {ref}/index-lifecycle-management.html[Index lifecycle management (ILM)] is
auto-detected and enabled by default if your {es} cluster supports it.

Comments are welcomed, pre- or post-commit.

Please review the
https://www.elastic.co/support/matrix#matrix_logstash_plugins[Support Matrix for Logstash plugins]
for the latest on supported plugins.

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 can take out this link if y'all don't think it is needed or appropriate.

Copy link
Member

Choose a reason for hiding this comment

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

yes let's remove this reference to supported plugins please

@karenzone
Copy link
Contributor Author

karenzone commented Apr 5, 2019

I was making changes while comments came in. If you don't see your comment implement, the omission was not intentional. Please let me know and I'll handle it.

@karenzone karenzone marked this pull request as ready for review April 5, 2019 12:23
@karenzone karenzone requested review from jsvd and yaauie April 5, 2019 15:32
docs/static/breaking-changes.asciidoc Show resolved Hide resolved
warnings when encountering input that is ambiguous, and allowed an early opt-in
of strict-mode parsing either by providing the command-line flag
`--field-reference-parser STRICT` or by adding `config.field_reference.parser:
STRICT` to `logstash.yml`.
Copy link
Member

Choose a reason for hiding this comment

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

can we maybe have an example here? not sure if it will be too verbose:

Before:

logstash-6.7.0 % echo "hello"| bin/logstash -e 'filter { mutate { replace => { "message" => "%{[[]]message]} you" } } }'
[2019-04-05T16:52:18,691][WARN ][org.logstash.FieldReference] Detected ambiguous Field Reference `[[]]message]`, which we expanded to the path `[message]`; in a future release of Logstash, ambiguous Field References will not be expanded.
{
       "message" => "hello you",
      "@version" => "1",
    "@timestamp" => 2019-04-05T15:52:18.546Z,
          "type" => "stdin",
          "host" => "overcraft.lan"
}

After:

logstash-7.0.0 % echo "hello"| bin/logstash -e 'filter { mutate { replace => { "message" => "%{[[]]message]} you" } } }'
[2019-04-05T16:48:09,135][FATAL][logstash.runner          ] An unexpected error occurred! {:error=>java.lang.IllegalStateException: org.logstash.FieldReference$IllegalSyntaxException: Invalid FieldReference: `[[]]message]`
[2019-04-05T16:48:09,167][ERROR][org.logstash.Logstash    ] java.lang.IllegalStateException: Logstash stopped processing because of an error: (SystemExit) exit

@karenzone
Copy link
Contributor Author

Rendered here for your viewing/reviewing pleasure: https://preview-docs.firebaseapp.com/logstash-breaking/breaking-changes.html

@lcawl
Copy link
Contributor

lcawl commented Apr 8, 2019

I fixed the merge conflict with #10654 and tagged all of the 7.0 items as "notable" for inclusion in the Installation and Upgrade Guide. If you only consider a subset of those items noteworthy, we can tag individual sections.

Copy link
Member

@jsvd jsvd left a comment

Choose a reason for hiding this comment

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

added a minor comment about removing the commercial support link otherwise LGTM

@karenzone karenzone merged commit d96854a into elastic:master Apr 8, 2019
@karenzone karenzone deleted the breaking-7.0 branch April 8, 2019 20:02
karenzone added a commit to karenzone/logstash that referenced this pull request Apr 8, 2019
* Update breaking changes doc for 7.0

Update structure to allow for previous and current changes

* Update docs/static/breaking-changes.asciidoc

Adds info about field reference parser

Co-Authored-By: karenzone <35154725+karenzone@users.noreply.github.com>

* Populate content for plugin changes

Co-Authored-By: karenzone <35154725+karenzone@users.noreply.github.com>

* Fix asciidoc formatting

* Incorporate review comments and new content

* Minor change for clarity

* add anchors for linking

* Add anchor for ecs-beats

* Incorporate review comments

* [DOCS] Adds tagged region for notable breaking changes

* Incorporate review comments

Remove link
elasticsearch-bot pushed a commit that referenced this pull request Apr 8, 2019
* Update breaking changes doc for 7.0

Update structure to allow for previous and current changes

* Update docs/static/breaking-changes.asciidoc

Adds info about field reference parser

Co-Authored-By: karenzone <35154725+karenzone@users.noreply.github.com>

* Populate content for plugin changes

Co-Authored-By: karenzone <35154725+karenzone@users.noreply.github.com>

* Fix asciidoc formatting

* Incorporate review comments and new content

* Minor change for clarity

* add anchors for linking

* Add anchor for ecs-beats

* Incorporate review comments

* [DOCS] Adds tagged region for notable breaking changes

* Incorporate review comments

Remove link

Fixes #10666
elasticsearch-bot pushed a commit that referenced this pull request Apr 8, 2019
* Update breaking changes doc for 7.0

Update structure to allow for previous and current changes

* Update docs/static/breaking-changes.asciidoc

Adds info about field reference parser

Co-Authored-By: karenzone <35154725+karenzone@users.noreply.github.com>

* Populate content for plugin changes

Co-Authored-By: karenzone <35154725+karenzone@users.noreply.github.com>

* Fix asciidoc formatting

* Incorporate review comments and new content

* Minor change for clarity

* add anchors for linking

* Add anchor for ecs-beats

* Incorporate review comments

* [DOCS] Adds tagged region for notable breaking changes

* Incorporate review comments

Remove link

Fixes #10666
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants