[WIP] Introduce local fields generation #9507

ruflin · 2018-12-12T09:35:18Z

This introduces a fields.go for libbeat instead of having one global fields.go in each Beat with all values. Already today this exists in Metricbeat but is a bit of a hack.

The advantage of having many smaller fields.go files is to it causes less conflicts on pull requests as when libbeat fields are changed not all Beats need an update. This should also allow for more granular fields generation in the future like Metricbeat has today.

To facilitate this change during the generation of the full fields.yml, mutliple smaller fields.yml files in the build directory are generated which then are used to generate the fields.go files.

TODO:

Test changes, make sure no changes to fields.yml, including order
Update makefiles for Filebeat / Packetbeat / Auditbeat to make sure also module files are generated
Remove old fields.go as now renamed to beat.go
Update developer changelog

elasticmachine · 2018-12-12T09:35:20Z

Pinging @elastic/infrastructure

andrewkroh · 2018-12-12T14:56:29Z

I’m glad to see this. This will make each beat consistent with how fields data is incorporated into the binary.

I want to ensure that each beat directory (both oss and x-pack) has a mage fields target that does both the generation of the fields.yml and then generates the fields.go files that it needs. x-pack/metricbeat/magefile is an example of this. I can help with this part if you’d like.

ruflin · 2018-12-13T09:01:54Z

@andrewkroh Would be great if you could help with this.

ruflin · 2018-12-17T08:59:18Z

@andrewkroh Tests seem now to mostly pass. There is an issue in the x-pack tests and I assume it could be related to that x-pack Filebeat has local .go files. and that the libbeat file is in the wrong place. Overall I think we are close. Let's sync up today on this one.

libbeat/include/doc.go

dev-tools/mage/fields.go

filebeat/include/module.go

dev-tools/mage/fields.go

ruflin · 2018-12-28T09:58:43Z

@andrewkroh This still needs some more cleanup but would be good to already get your feedback on the usage of the mage fields command here or if you think it should be done differently.

auditbeat/magefile.go

ruflin · 2018-12-28T09:55:18Z

metricbeat/Makefile

@@ -76,6 +76,3 @@ test-module: python-env update metricbeat.test
 .PHONY: assets
 assets:
 	go run ${ES_BEATS}/metricbeat/scripts/assets/assets.go ${ES_BEATS}/metricbeat/module


In a follow up PR, this should also be moved to mage.

metricbeat/magefile.go

auditbeat/magefile.go

andrewkroh · 2019-01-08T15:53:36Z

dev-tools/cmd/asset/asset.go

@@ -101,6 +101,8 @@ func main() {
 	if name == "" {
 		name = file
 	}
+	// Capitalise first letter
+	name = strings.Title(name)


To generate a friendlier method name consider something like this https://github.com/elastic/ecs/blob/69de90eb6493e0804405321f48adfdfa488d6498/scripts/cmd/gocodegen/gocodegen.go#L305-L313. This would change AssetFile_integrity to AssetFileIntegrity.

copy/pasted your code :-)

dev-tools/cmd/asset/asset.go

auditbeat/magefile.go

libbeat/generator/fields/fields.go

andrewvc

I'm +1 on the approach. It seems that @andrewkroh has you covered as far as review goes. LMK if you need a second set of eyes on the actual code.

andrewkroh · 2019-01-10T06:36:21Z

x-pack/metricbeat/module/mssql/fields.go

@@ -16,7 +16,8 @@ func init() {
 	}
 }

-// AssetMssql returns asset data
+// AssetMssql returns asset data.
+// This is the base64 encoded gzipped contents of /Users/ruflin/Dev/gopath/src/github.com/elastic/beats/x-pack/metricbeat/module/mssql.


Can you modify something so that this isn't any absolute path, but instead is relative to the beat's directory like ./module/mssql/fields.go. I didn't test to see if this would work, but try changing this line to not construct an abs path.

beats/dev-tools/mage/fields.go

Line 104 in e92c6b8

filepath.Join(CWD(), moduleDir),

Seems to do the trick but need to do some more testing.

ruflin

Sorry, comments will still pending, sent it now.

ruflin · 2019-01-09T13:17:18Z

dev-tools/cmd/asset/asset.go

@@ -101,6 +101,8 @@ func main() {
 	if name == "" {
 		name = file
 	}
+	// Capitalise first letter
+	name = strings.Title(name)


copy/pasted your code :-)

dev-tools/cmd/asset/asset.go

libbeat/generator/fields/fields.go

ruflin · 2019-01-10T09:40:39Z

x-pack/auditbeat/magefile.go

@@ -82,8 +83,13 @@ func TestPackages() error {
 }

 // Update is an alias for running fields, dashboards, config.
-func Update() {
+func Update() error {
+	err := sh.Run("make", "-C", "../../auditbeat", "update")


@andrewkroh I had to introduce this hack as for the generation of the x-pack Beats fields.yml they depend on the build/fields/* files in the OSS Beats.

This also made me think if Auditbeat does the right thing here. As it depends on Metricbeat it should also include the beat.yml data and in this case depending on metricbeat update? Does priority matter here? I would assume it's ok that the metricbeat and auditbeat beat.yml have the same priority as they should not conflict with each other. To be save, would be nice to have a way to set different priorities here.

All the above becomes obsolete as soon as we generate the fields.yml by beat export fields as then each beat can only generate it's own files and it then depends on the internal dependency / priority how the fields.yml is exported.

Why does x-pack/auditbeat depend on anything in OSS auditbeat/build/fields? That should not be the case. When building fields.yml files in x-pack/auditbeat the should use the checked in source to derive things. This is how fields and dashboards work. (It's been a while since I last looked at this PR and it's Friday so I could be missing something. Maybe should should discuss this part next week.)

I need to check again, but I remember some fields / fields from OSS were missing otherwise. Will get back to you on this one as soon as I got the time.

The problem is around here: https://github.com/elastic/beats/pull/9507/files#diff-408f5df10fc1cfd92fc66134de4d8edbR77 The generation of the fields.yml still works, but it can't create the new fields.go files. The problem is that currently the build/fields/*.yml files are only built for OSS. This becomes now more complex, as X-Pack needs to also build these files to then derive the modules and potentially specific beats files for the new local fields.go files. Need to think through on how we deal with it best. The x-pack beat does not need knowledge about the beat fields of the non x-pack one and can also skip libbeat.

ruflin · 2019-01-22T08:14:31Z

@andrewkroh I extracted a subset of this PR here: #10168 Could you have a look? Goal is to reduce the size of this PR and the number of files it touches.

This introduces a fields.go for libbeat instead of having one global fields.go in each Beat with all values. Already today this exists in Metricbeat but is a bit of a hack. The advantage of having many smaller fields.go files is to it causes less conflicts on pull requests as when libbeat fields are changed not all Beats need an update. This should also allow for more granular fields generation in the future like Metricbeat has today. To facilitate this change during the generation of the full fields.yml, mutliple smaller fields.yml files in the build directory are generated which then are used to generate the fields.go files. Changes: * Update all `mage Fields` scripts to only generate the `*.go` files that are needed * Call `mage fields` from the Makefile to have code only in one place for all Beats * Call `mage clean` also from `make clean` to have code to clean up generated files only in one place. More cleanup should happen. * Remove `fields.go` files from all Beats as it's now in Libbeat. * Implemented Named Assets method to be able to have multiple Asset methods in the include package. TODO: * Test changes, make sure no changes to fields.yml, including order * Update makefiles for Filebeat / Packetbeat / Auditbeat to make sure also module files are generated update fields add support for multiple fields.go file in one package by adding a name to the asset funtion add new fields update fields and remove unneded ones improve handling update fields and update packetbeat build command small change update makefile check commit diff update metricbeat files add generated files add upper case fix packetbeat add mage clean to makefile command add cleanup step add license header delete fields.go for filebeat remove fields.go from packetbeat remove all in one fields.go generation from mage, needs to be introduce later again and moved from makefile fix for hound move generation to mage file add generated files update metricbeat makefile add winlogbeat and packetbeat beat file add function beat file add mage dependency to update update fields address review comments fix relative paths remove some fields? update docker fields

houndci-bot · 2019-03-15T13:55:21Z

filebeat/inputsource/udp/config.go

@@ -20,6 +20,7 @@ package udp
 import (
 	"time"

+<<<<<<< HEAD:filebeat/inputsource/udp/config.go


expected 'STRING', found '<<' (and 6 more errors)

houndci-bot · 2019-03-15T13:55:26Z

metricbeat/module/aerospike/testing.go

@@ -21,6 +21,7 @@ import (
 	"os"
 )

+<<<<<<< HEAD:metricbeat/module/aerospike/testing.go


expected declaration, found '<<'

houndci-bot · 2019-03-15T13:55:27Z

libbeat/common/seccomp/policy_linux_arm.go

@@ -20,6 +20,7 @@ package seccomp
 import "github.com/elastic/go-seccomp-bpf"

 func init() {
+<<<<<<< HEAD:libbeat/common/seccomp/policy_linux_arm.go


expected statement, found '<<' (and 3 more errors)

houndci-bot · 2019-03-15T13:55:27Z

libbeat/common/file/fileinfo_windows.go

@@ -21,15 +21,28 @@ import (
 	"os"
 )

+<<<<<<< HEAD:libbeat/common/file/fileinfo_windows.go


expected declaration, found '<<'

ruflin · 2019-03-20T09:30:22Z

filebeat/module/apache2/fields.go

@@ -0,0 +1,36 @@
+// Licensed to Elasticsearch B.V. under one or more contributor


Should not be here. Removing.

ruflin · 2019-03-20T09:32:47Z

metricbeat/docs/fields.asciidoc

@@ -14,7 +14,6 @@ grouped in the following categories:

 * <<exported-fields-aerospike>>
 * <<exported-fields-apache>>
-* <<exported-fields-aws>>


this should not change, seems something of with x-pack collection.

ruflin · 2019-04-29T18:19:57Z

Quite a few changes happen in #11679 which probably should incorporated into this PR too.

@exekias @andrewkroh I wonder what we should do with this PR moving forward. I think it's still valuable to have a clear separation between the libbeat and other fields but I run in some dependency / update issue (see above). I'm guessing if we retry this PR when we unified the Beats on top of mage things become much easier. #11679 is a huge step in this direction.

andrewkroh · 2019-05-01T13:32:32Z

I'm guessing if we retry this PR when we unified the Beats on top of mage things become much easier.

I very much agree with this assessment. The recently merged #11679 brings in a subset of the changes from #9842 which migrated all projects. If we migrate the remaining projects they will be consistent and these should be simpler to apply in each beat.

ruflin · 2019-05-02T14:38:48Z

Thanks @andrewkroh for the feedback. Will close this for now and we should reevaluate when the migration is finished.

ruflin added in progress Pull request is currently in progress. libbeat Team:Integrations Label for the Integrations team labels Dec 12, 2018

ruflin self-assigned this Dec 12, 2018

ruflin force-pushed the collect-local-files branch 3 times, most recently from bcc1aa8 to 67bf8c9 Compare December 14, 2018 15:22

ruflin requested a review from a team as a code owner December 14, 2018 15:22

ruflin force-pushed the collect-local-files branch from 67bf8c9 to e7ad623 Compare December 17, 2018 09:04

ruflin force-pushed the collect-local-files branch from e7ad623 to a59916a Compare December 28, 2018 08:03

houndci-bot reviewed Dec 28, 2018

View reviewed changes

libbeat/include/doc.go Show resolved Hide resolved

ruflin force-pushed the collect-local-files branch from 9ac9d13 to 4829fdd Compare December 28, 2018 08:53

ruflin commented Dec 28, 2018

View reviewed changes

dev-tools/mage/fields.go Outdated Show resolved Hide resolved

filebeat/include/module.go Outdated Show resolved Hide resolved

houndci-bot reviewed Dec 28, 2018

View reviewed changes

dev-tools/mage/fields.go Outdated Show resolved Hide resolved

ruflin force-pushed the collect-local-files branch from e61e458 to fb5d808 Compare December 28, 2018 09:57

ruflin commented Dec 28, 2018

View reviewed changes

ruflin force-pushed the collect-local-files branch from efa2b3e to ca725f6 Compare January 8, 2019 09:21

ruflin requested review from a team as code owners January 8, 2019 09:21

andrewkroh reviewed Jan 8, 2019

View reviewed changes

andrewvc reviewed Jan 8, 2019

View reviewed changes

andrewkroh reviewed Jan 10, 2019

View reviewed changes

ruflin commented Jan 10, 2019

View reviewed changes

ruflin force-pushed the collect-local-files branch from 40af1f3 to 325ca7e Compare January 10, 2019 09:00

ruflin commented Jan 10, 2019

View reviewed changes

ruflin force-pushed the collect-local-files branch from cc061a4 to f579e7c Compare January 18, 2019 13:25

ruflin mentioned this pull request Jan 23, 2019

Improve and simplify generation of fields #10168

Merged

ruflin added 6 commits March 15, 2019 14:36

parallelise libbeat part

7dc8bcf

add missing files

3e21a9e

manage merge conflicts and update nats file

156acdd

update fields.go files

9b0b0c0

rebase and update

f41dc71

ruflin force-pushed the collect-local-files branch from 38ba897 to f41dc71 Compare March 15, 2019 13:55

houndci-bot reviewed Mar 15, 2019

View reviewed changes

fix diffs

9cb0b57

ruflin commented Mar 20, 2019

View reviewed changes

one more update

4b12ddb

ruflin commented Mar 20, 2019

View reviewed changes

fix metricbeat field generatin

0b0af3d

ruflin added the [zube]: Ready label Apr 23, 2019

ruflin closed this May 2, 2019

zube bot added [zube]: Done and removed [zube]: Ready labels May 2, 2019

andresrc removed the [zube]: Done label May 8, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Introduce local fields generation #9507

[WIP] Introduce local fields generation #9507

ruflin commented Dec 12, 2018 •

edited

Loading

elasticmachine commented Dec 12, 2018

andrewkroh commented Dec 12, 2018

ruflin commented Dec 13, 2018

ruflin commented Dec 17, 2018

ruflin commented Dec 28, 2018

ruflin Dec 28, 2018

andrewkroh Jan 8, 2019

ruflin Jan 9, 2019

andrewvc left a comment

andrewkroh Jan 10, 2019

ruflin Jan 10, 2019

ruflin left a comment

ruflin Jan 9, 2019

ruflin Jan 10, 2019

andrewkroh Jan 18, 2019

ruflin Jan 22, 2019

ruflin Mar 20, 2019

ruflin commented Jan 22, 2019

houndci-bot Mar 15, 2019

houndci-bot Mar 15, 2019

houndci-bot Mar 15, 2019

houndci-bot Mar 15, 2019

ruflin Mar 20, 2019

ruflin Mar 20, 2019

ruflin commented Apr 29, 2019

andrewkroh commented May 1, 2019

ruflin commented May 2, 2019

		@@ -0,0 +1,36 @@
		// Licensed to Elasticsearch B.V. under one or more contributor

[WIP] Introduce local fields generation #9507

[WIP] Introduce local fields generation #9507

Conversation

ruflin commented Dec 12, 2018 • edited Loading

elasticmachine commented Dec 12, 2018

andrewkroh commented Dec 12, 2018

ruflin commented Dec 13, 2018

ruflin commented Dec 17, 2018

ruflin commented Dec 28, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrewvc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ruflin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ruflin commented Jan 22, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ruflin commented Apr 29, 2019

andrewkroh commented May 1, 2019

ruflin commented May 2, 2019

ruflin commented Dec 12, 2018 •

edited

Loading