Cherry-pick #9603 to 6.x: Fixes parsing of GC entries in elasticsearch server log

[ycombinator]

Cherry-pick of PR #9603 to 6.x branch. Original message:

Resolves #9513.

This PR:

• removes the incorrectly-parsed gc_overhead field. Turns out what we were parsing was actually an insignificant sequential number, not GC overhead,
• parses out a new gc.collection_duration field, e.g. 1.2s, which is the time spent performing GC, and
• parses out a new gc.observation_duration field, e.g. 1.8s, which is the overall time over which GC collection was performed

It also splits up the long grok expression in the ingest pipeline into smaller patterns and references those patterns, hopefully for easier readability.

[ycombinator]

If you look at the diff of this PR and compare it with the diff of the original PR to master, you will notice that this PR's diff has a lot more lines changed. That's misleading because most of these changes are formatting changes which change the whitespace. So best to view this PR's diff while ignoring whitespace, like so: https://github.com/elastic/beats/pull/9810/files?w=1

[ruflin]

Should this PR have a changelog? Is this a breaking change?

[tsouza]

This is an interesting question Nicholas. It's technically a breaking change since we are changing the structure of the data being collected, but I find it hard that anyone was using these messages for anything useful since the semantics of the collected data was wrong anyway. But yeah, it's a breaking change.

…

On Fri, Dec 28, 2018, 04:41 Nicolas Ruflin ***@***.*** wrote: Should this PR have a changelog? Is this a breaking change? — You are receiving this because your review was requested. Reply to this email directly, view it on GitHub <#9810 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAHeQxu6z5tho-dL_y4tlZ24XcWBGkAoks5u9b0EgaJpZM4ZjGlN> .

[ycombinator]

Technically this is a breaking change, but as @tsouza pointed out the old field was meaningless. Tangential note: this module is in beta, so hopefully its okay to make the breaking change in a minor version?

Regardless, I think this deserves an entry in the CHANGELOG. I missed it in the original PR to master (#9603) but will add an entry here.

[ycombinator]

@ruflin I added two CHANGELOG entries, or rather the same entry under two sections: breaking changes and bug fixes. Do you think this is okay?

[ruflin]

@ycombinator I'm also good if we agree that it was a bug :-) Can you add this also the .next Changelog in case you are already touching the PR?

[ycombinator]

@ruflin I've updated this PR per your feedback and CI is green now. Mind taking another look at it, when you get a chance? Thanks.