Update the HTTP field set with ECS definitions as of beta 2 by webmat · Pull Request #9645 · elastic/beats

webmat · 2018-12-18T21:15:58Z

Caveat

We cannot alias the Packetbeat .body fields for this migration. This is a situation similar to Filebeat's source field.
http.request.method is not lowercased in this PR. This affects many Fb modules, Packetbeat, etc. I think it should be implemented via an index tokenizer, not by modifying _source

TODO

Confirm the move of http bodies to ...body.content is complete
Update documentation (see Andrew's comment)
Changelog
Delete x-pack/auditbeat/include/fields.go which should have been deleted in Update Auditbeat magefile.go #9724.

ruflin · 2018-12-19T07:51:17Z

Looks like this breaks some packetbeat tests?

webmat · 2018-12-19T14:41:39Z

@ruflin Yeah that's possible. I had problems running the test suite locally, so I decided to ask Jenkins and Travis for help. Hence the WIP / "in progress" ;-)

webmat · 2018-12-19T15:27:28Z

Error in question:

======================================================================
ERROR: Check that the http body is exported only for some http messages that have the
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/gopath/src/github.com/elastic/beats/packetbeat/tests/system/test_0063_http_body.py", line 49, in test_include_body_for_both_request_response
    objs = self.read_output()
  File "/home/travis/gopath/src/github.com/elastic/beats/packetbeat/tests/system/packetbeat.py", line 122, in read_output
    self.all_fields_are_expected(jsons, self.expected_fields)
  File "/home/travis/gopath/src/github.com/elastic/beats/packetbeat/tests/system/../../../libbeat/tests/system/beat/beat.py", line 471, in all_fields_are_expected
    .format(key))
Exception: Unexpected key 'http.response.body' found

webmat · 2018-12-19T16:28:22Z

@andrewkroh ECS 1 Beta 2 introduced the http field set with the bodies nested a bit deeper: http.*.body.content instead of http.*.body. I've made the change in Packetbeat, and the tests are now passing. I'd like it if you could look at this, to make sure I haven't forgotten anything.

andrewkroh

That looks like everything w.r.t to the Packetbeat code. I'm really hoping the test coverage for http is good in Packetbeat 🤞

I think there are some mentions in the documentation that also need updated. Like here.

webmat · 2018-12-20T20:41:51Z

I've modified the documentation and added changelog entries in two places (Pb/breaking and All beats/Added).

I'd like to merge this as is. There's a few caveats listed in the body of this PR. One for awareness / discussion (can't alias Packetbeat's body changes), and one is for follow-up work.

webmat · 2018-12-21T01:19:19Z

jenkins, test this

ruflin · 2018-12-21T12:29:17Z

packetbeat/protos/http/http.go

Probably better user here result.Put("body.content", string(m.body) so it creates the correct object.

Done. Although the rest of this file use the square bracket notation to set fields. I'm not adjusting the rest of this file.

packetbeat/protos/http/http.go

webmat · 2018-12-21T16:32:59Z

@ruflin One last look needed. My understanding is that these metricbeat failures are unrelated, correct?

https://beats-ci.elastic.co/job/elastic+beats+pull-request+multijob-linux/2877/testReport/

Once I merge this PR, the rebase conflicts that leave x-pack/auditbeat/include/fields.go broken will be a thing of the past.

webmat · 2018-12-21T16:42:11Z

Ok just confirmed metricbeat failure is unrelated. Just got fixed by #9749

- With the exception of http, captured in elastic#9645 - With the exception of os, which must be updated in a bunch of places

This addresses most of the differences, with the exception of http (#9645) and os (PR coming) Changes: - `client/server`, `source/destination` - Update each field set's definition - Add the `.address` field (except for `source`, which already had it) - `network`: update definitions and examples wrt lowercase directive - `user_agent.original` index is now `keyword` indexed (See elastic/ecs#262) - Update `ecs.version` example

ruflin · 2018-12-27T07:10:34Z

@webmat Nit: Could you adjust the title of the PR so if people come back to this PR it does not say WIP and it's merged ;-)

ruflin · 2018-12-27T07:11:25Z

@webmat Should this also show up in the ecs migration yml file?

Notes: - Can't be aliased since `body` is moving to `body.content`. - Currently only affects Packetbeat, so it's been listed only there, even if these are ECS field defs. - This will affect the ES Filebeat module logs as well. A note as been added to elastic#9293, so it doesn't get forgotten.

webmat · 2019-01-03T21:27:29Z

@ruflin Yes, good point about ecs-migration. I've opened #9878

…up. (#9878) Details: * Add two migrated fields to ecs-migration from #9645. Can't be aliased since `.body` is moving to `.body.content`. * Remove dupe headers for what's after the 'processors' section. Likely a rebase hiccup. * Fix typo in processors entries: `form` => `from`. * Move the processors heading above the docker processor section

…tic#9645) - Introduces fields for http size metrics - HTTP body field is now nested deeper: - `http.request.body` moves to `http.request.body.content` - `http.response.body` moves to `http.response.body.content` - packetbeat has been adjusted accordingly - Introduces missing field definition updates (mainly to lowercase `method`) - Unrelated: delete `x-pack/auditbeat/include/fields.go` which should have been deleted in elastic#9724.

This addresses most of the differences, with the exception of http (elastic#9645) and os (PR coming) Changes: - `client/server`, `source/destination` - Update each field set's definition - Add the `.address` field (except for `source`, which already had it) - `network`: update definitions and examples wrt lowercase directive - `user_agent.original` index is now `keyword` indexed (See elastic/ecs#262) - Update `ecs.version` example

webmat requested a review from a team as a code owner December 18, 2018 21:15

webmat self-assigned this Dec 18, 2018

webmat added in progress Pull request is currently in progress. ecs labels Dec 18, 2018

ruflin mentioned this pull request Dec 18, 2018

Transition Beats to ECS #8655

Closed

webmat mentioned this pull request Dec 18, 2018

Convert Filebeat elasticsearch.* to ECS #9293

Merged

8 tasks

andrewkroh reviewed Dec 20, 2018

View reviewed changes

webmat mentioned this pull request Dec 20, 2018

Convert Filebeat's traefik.access to ECS. #9005

Merged

19 tasks

webmat force-pushed the ecs-http-update branch from 1e3e1d2 to c9ea42d Compare December 20, 2018 20:21

webmat requested a review from ruflin December 20, 2018 20:42

webmat added review and removed in progress Pull request is currently in progress. labels Dec 20, 2018

This was referenced Dec 20, 2018

WIP Filebeat modules adjustments for ECS Beta 2 #9684

Closed

Convert Filebeat kibana.log to ECS #9301

Merged

webmat mentioned this pull request Dec 21, 2018

Finish importing ECS Beta 2 differences #9738

Merged

3 tasks

andrewkroh approved these changes Dec 21, 2018

View reviewed changes

ruflin reviewed Dec 21, 2018

View reviewed changes

webmat force-pushed the ecs-http-update branch from 8ec2907 to 4d540f7 Compare December 21, 2018 14:31

houndci-bot reviewed Dec 21, 2018

View reviewed changes

packetbeat/protos/http/http.go Outdated Show resolved Hide resolved

webmat force-pushed the ecs-http-update branch from 4d540f7 to da5b718 Compare December 21, 2018 14:32

Mathieu Martin added 2 commits December 21, 2018 10:34

Update the HTTP field set with ECS definitions as of beta 2

df38e82

HTTP request and response body is now at http.*.body.content.

d7922c4

Mathieu Martin added 3 commits December 21, 2018 10:35

Changelog

426399e

Change the body field names in the Packetbeat documentation

64ea13a

Use Put to set the body content field instead of an accessor

9ac91f5

webmat force-pushed the ecs-http-update branch from a1ba677 to 9ac91f5 Compare December 21, 2018 15:35

webmat merged commit caf07a3 into elastic:master Dec 21, 2018

webmat deleted the ecs-http-update branch December 21, 2018 16:49

webmat pushed a commit to webmat/beats that referenced this pull request Dec 21, 2018

First pass at including all missing ECS Beta 2 changes...

1356f03

- With the exception of http, captured in elastic#9645 - With the exception of os, which must be updated in a bunch of places

webmat mentioned this pull request Dec 21, 2018

Importing changes to os file set from ECS Beta 2 #9763

Merged

webmat changed the title ~~WIP Update the HTTP field set with ECS definitions as of beta 2~~ Update the HTTP field set with ECS definitions as of beta 2 Jan 3, 2019

Conversation

webmat commented Dec 18, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ruflin commented Dec 19, 2018

Uh oh!

webmat commented Dec 19, 2018

Uh oh!

webmat commented Dec 19, 2018

Uh oh!

webmat commented Dec 19, 2018

Uh oh!

andrewkroh left a comment

Choose a reason for hiding this comment

Uh oh!

webmat commented Dec 20, 2018

Uh oh!

webmat commented Dec 21, 2018

Uh oh!

ruflin Dec 21, 2018

Choose a reason for hiding this comment

Uh oh!

webmat Dec 21, 2018

Choose a reason for hiding this comment

Uh oh!

Uh oh!

webmat commented Dec 21, 2018

Uh oh!

webmat commented Dec 21, 2018

Uh oh!

ruflin commented Dec 27, 2018

Uh oh!

ruflin commented Dec 27, 2018

Uh oh!

webmat commented Jan 3, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

webmat commented Dec 18, 2018 •

edited

Loading