Port fields.yml collector to Golang by kvch · Pull Request #6911 · elastic/beats

kvch · 2018-04-20T13:23:13Z

Existing Python and shell code is ported to Golang. The previous Beat specific field collection is is generalized and moved to the Makefile of libbeat. A new variable is added to makefiles: FIELDS_FILE_PATH. This specifies where fields.yml are.

Misc

Packetbeat's fields_base.yml is renamed to fields.common.yml to be the same as in other Beats

Breaking change for community Beats

If your Beat has partial fields.yml files which need to be collected by make update, please fill in the FIELDS_FILE_PATH variable in your Makefile with the root directory of your fields files.

If you need a custom collector function you can implement it module_fields_collector.go with the following type signature:

func CollectModuleFiles(root string) ([]*fields.YmlFile, error)

houndci-bot · 2018-04-20T13:23:31Z

filebeat/generator/fileset/fileset.go

exported function Generate should have comment or be unexported

houndci-bot · 2018-04-20T13:23:31Z

filebeat/generator/module/module.go

exported function Generate should have comment or be unexported

houndci-bot · 2018-04-20T13:23:31Z

libbeat/generator/fields/module_fields_collector.go

exported function CollectModuleFiles should have comment or be unexported

houndci-bot · 2018-04-20T13:23:32Z

libbeat/generator/fields/fields_collector.go

exported function CollectFiles should have comment or be unexported

houndci-bot · 2018-04-20T13:23:32Z

libbeat/generator/fields/fields.go

exported function Generate should have comment or be unexported

houndci-bot · 2018-04-20T13:23:33Z

filebeat/generator/fields/fields.go

exported function Generate should have comment or be unexported

houndci-bot · 2018-04-20T13:23:33Z

filebeat/cmd/generate.go

error strings should not be capitalized or end with punctuation or a newline

houndci-bot · 2018-04-20T13:23:33Z

filebeat/cmd/generate.go

error strings should not be capitalized or end with punctuation or a newline

houndci-bot · 2018-04-20T13:23:33Z

filebeat/cmd/generate.go

error strings should not be capitalized or end with punctuation or a newline

houndci-bot · 2018-04-20T13:23:33Z

filebeat/cmd/generate.go

error strings should not be capitalized or end with punctuation or a newline

ruflin · 2018-04-20T13:30:34Z

Could this PR be split up in 2 parts:

Moving the make fields collection step to Golang
Introducing the subcommands

This should make reviewing easier and will separate the two different concerns.

kvch · 2018-04-20T14:14:35Z

@ruflin Done. Other PR: #6912

ruflin

Thanks for splitting up in 2 PR's. Review is probably to early but left some notes.

This is really great as soon all the fields.yml collection is directly in golang.

ruflin · 2018-04-20T14:38:25Z

filebeat/beater/filebeat.go

Leftover from the other PR?

ruflin · 2018-04-20T14:38:54Z

filebeat/Makefile

Does this only support one path? I could see especially in libbeat or filebeat that a module, processor and inputs have fields.yml.

Yes, but it can be changed easily to support lists.

ruflin · 2018-04-20T14:39:25Z

heartbeat/beater/heartbeat.go

ruflin · 2018-04-20T14:40:41Z

libbeat/cmd/instance/beat.go

Probably also part of the other PR?

kvch · 2018-04-23T17:21:03Z

I removed the leftovers from the other PR and added tests. However, I have a Winlogbeat issue I am still investigating.

ruflin

As this is a breaking change for all our community beats, this should be documented on the CHANGELOG-developer with a note on how to migrate.

ruflin · 2018-04-24T07:10:37Z

libbeat/generator/fields/fields.go

Should this be uncommented or removed? Same next line.

ruflin · 2018-04-24T07:13:27Z

libbeat/generator/fields/fields.go

Should we also add a newline after to make sure it ends with a newline?

ruflin · 2018-04-24T07:15:13Z

libbeat/generator/fields/fields.go

Shouldn't the global fields.yml end up outside the _meta directory?

In Makefiles of Beats the generated file ends up under _meta. See for example Metricbeat: https://github.com/elastic/beats/blob/master/metricbeat/Makefile#L28

ruflin · 2018-04-24T07:21:37Z

libbeat/generator/fields/module_fields_collector.go

should we call this collectModuleas this is quite specific to modules?

ruflin · 2018-04-24T07:24:46Z

libbeat/generator/fields/fields.go

Is this used somewhere except for the tests? Should this be moved to the tests?

houndci-bot · 2018-04-26T17:22:00Z

libbeat/generator/fields/fields.go

exported type YmlFile should have comment or be unexported

ruflin · 2018-04-27T11:50:56Z

I tried the code locally by run make clean and then make update in the beats directory and got the following error:

Updating generated files for libbeat
cat: _meta/beat.yml: No such file or directory
cat: _meta/beat.yml: No such file or directory
Generated global fields.yml under _meta/fields.generated.yml
ERROR: Field <error.> is duplicated. Please update and try again.exit status 1
make[1]: *** [update] Error 1
make: *** [update] Error 1

ruflin · 2018-04-27T11:54:36Z

An other thing I spotted when testing is that the indentation of key: cloud is different from before. Looks like a general issue with the processor fields collection.

kvch · 2018-04-27T16:50:41Z

Yes, I started to refactor the code, so things got messed up and a few things are not working properly. I will ping you when it's ready for a next round of review.

kvch · 2018-05-04T09:27:08Z

@ruflin I fixed the problems in the code. It should be ready to be reviewed.

ruflin · 2018-05-04T13:49:56Z

@kvch Could you have a look at the generator tests? The change seems to break it.

per elastic/beats#6911 set FIELDS_FILE_PATH accordingly.

* use beats provided fields target per elastic/beats#6911 set FIELDS_FILE_PATH accordingly. * Update beats framework to 245b3e1 * removes fields target from top-level Makefile * vendors github.com/elastic/beats/libbeat/generator/fields * cleans up github.com/ericchiang/k8s/...

kvch added in progress Pull request is currently in progress. review :Generator Related to code generators for building custom Beats or modules. labels Apr 20, 2018

houndci-bot reviewed Apr 20, 2018

View reviewed changes

kvch force-pushed the feature/filebeat/global-fields-yml branch from 2530189 to 21922fd Compare April 20, 2018 14:01

kvch mentioned this pull request Apr 20, 2018

New subcommand: generate #6912

Closed

2 tasks

kvch changed the title ~~Expose generators as subcommands && port fields.yml collector to Golang~~ Port fields.yml collector to Golang Apr 20, 2018

ruflin reviewed Apr 20, 2018

View reviewed changes

kvch force-pushed the feature/filebeat/global-fields-yml branch from 21922fd to 05cb292 Compare April 23, 2018 17:15

ruflin requested changes Apr 24, 2018

View reviewed changes

kvch force-pushed the feature/filebeat/global-fields-yml branch from 05cb292 to 92e2e85 Compare April 26, 2018 17:21

houndci-bot reviewed Apr 26, 2018

View reviewed changes

libbeat/generator/fields/fields.go Outdated

Copy link
Copy Markdown

houndci-bot Apr 26, 2018

Choose a reason for hiding this comment

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

exported type YmlFile should have comment or be unexported

kvch removed the review label May 3, 2018

kvch force-pushed the feature/filebeat/global-fields-yml branch 3 times, most recently from 11f5363 to f59fd4b Compare May 4, 2018 09:09

kvch added review and removed in progress Pull request is currently in progress. labels May 4, 2018

kvch force-pushed the feature/filebeat/global-fields-yml branch 4 times, most recently from 2c5415d to 133509e Compare May 8, 2018 15:23

kvch added 23 commits May 30, 2018 19:01

rename packetbeat's field file

6fa6659

add scripts for generating fields.yml of beats

9ed201e

add test for generating fields

34b29ae

run fields.yml generator from makefiles

b324d7a

address review comments

5976927

fix makefiles for generators

befbc51

add breaking change to dev changelog

0643e41

move make variables to their proper places

72edf89

fix messages of generator

25d4374

fix path separator

dbfca67

concat fields of libbeat and beat_name

9556c3a

remove concating fields.ymls from Makefile

ed703b9

change order

ffe25f7

fix winlogbeat

476e4ac

fix multiplication of fields

f76c40f

fix generators to pass tests

96d70b1

feed the hound

926743b

update asset data of every beat

d7a8e57

address review notes: permissions

4e97a8e

address review notes: stricter error checking

f3a6021

simplify indentation of lines

9c613ca

update asset files

12e2a58

simplify reading lines

e982a79

kvch force-pushed the feature/filebeat/global-fields-yml branch from 91794a4 to e982a79 Compare May 30, 2018 17:02

kvch added 2 commits May 30, 2018 19:45

add missing file closing

12ad073

correct permissions

56a4591

jsoriano approved these changes May 30, 2018

View reviewed changes

jsoriano merged commit 2b0df2f into elastic:master May 30, 2018

graphaelli added a commit to graphaelli/apm-server that referenced this pull request Jun 4, 2018

use beats provided fields target

da3fda4

per elastic/beats#6911 set FIELDS_FILE_PATH accordingly.

Conversation

kvch commented Apr 20, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Misc

Breaking change for community Beats

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ruflin commented Apr 20, 2018

Uh oh!

kvch commented Apr 20, 2018

Uh oh!

ruflin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kvch commented Apr 23, 2018

Uh oh!

ruflin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ruflin commented Apr 27, 2018

Uh oh!

ruflin commented Apr 27, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kvch commented Apr 27, 2018

Uh oh!

kvch commented May 4, 2018

Uh oh!

ruflin commented May 4, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

kvch commented Apr 20, 2018 •

edited

Loading

ruflin commented Apr 27, 2018 •

edited

Loading