[Heartbeat] Fix fields generation#21874
Conversation
Fields generation seems to have broken at some time in the past, but we never noticed, because we didn't add fields for quite a while. This fixes this.
|
Pinging @elastic/uptime (Team:Uptime) |
|
/test please |
andrewkroh
left a comment
There was a problem hiding this comment.
Summarizing the setup here:
- Only OSS heartbeat declares fields.
- All fields are contained in OSS heartbeat/_meta
- Each build (OSS and X-Pack) generates a fields.yml to include in packages for reference. It's not used by the beat software for anything.
- There is a include/fields.go file generated from the all-in-one fields.yml file to embed the fields data into the beat.
- Any fields.go files are imported somewhere to ensure they are included in the binary.
| _ "github.com/elastic/beats/v7/heartbeat/autodiscover/builder/hints" | ||
|
|
||
| // register default heartbeat monitors | ||
| _ "github.com/elastic/beats/v7/heartbeat/monitors/defaults" |
There was a problem hiding this comment.
This package looks broken. In the OSS magefile.go there is a generator for it. It looks like it's supposed to be creating imports for each monitor. I'd remove the package and the generation code if it's unused.
There was a problem hiding this comment.
The weird thing is this used to work, and somehow broke at some point. Now, when I run mage update these lines just get deleted. That said, probably easier to just remove it and make it manual since we don't add monitor types frequently.
heartbeat/scripts/mage/package.go
Outdated
| if err != nil { | ||
| return err | ||
| } | ||
| return devtools.GenerateAllInOneFieldsGo() |
There was a problem hiding this comment.
When this is run from x-pack/heartbeat it will create x-pack/heartbeat/include/fields.go, but since there are no new fields being introduced we can skip this part. We'd only want to introduce an x-pack/heartbeat/include/fields.go file if X-Pack heartbeat added new fields. And if this file was added we'd only want it to include the new fields (not any of the libbeat or OSS heartbeat fields) since they are already included on the OSS side an inherited by x-pack.
You could use some license based conditionals like in Winlogbeat
or do the same work in both OSS and X-Pack but write the output to OSS only
devtools.GenerateFieldsGo("fields.yml", devtools.OSSBeatDir("include/fields.go"))
There was a problem hiding this comment.
It will generate new fields when the synthetics stuff is merged in #21436 , and in fact this PR is a bit of a blocker for that one. That said, if it's easier, I'd rather just include all the fields (including x-pack fields) in the OSS heartbeat and keep things simple. Does that work for you?
There was a problem hiding this comment.
I went with the latter approach you suggested. Thanks for the tips!
heartbeat/docs/fields.asciidoc
Outdated
| -- | ||
|
|
||
| [[exported-fields-socks5]] | ||
| [[exported-fields-socks6]] |
There was a problem hiding this comment.
socks6? Where did that change come from?
There was a problem hiding this comment.
There was a problem hiding this comment.
Good catch, fixed!
heartbeat/Makefile
Outdated
| @@ -13,8 +13,7 @@ collect: imports kibana | |||
| # Generate imports for all monitors | |||
| .PHONY: imports | |||
| imports: | |||
There was a problem hiding this comment.
I think it's only used by heartbeat's own collect target so it can be removed if you want.
There was a problem hiding this comment.
Good catch, removed
* [Heartbeat] Fix fields generation Fields generation seems to have broken at some time in the past, but we never noticed, because we didn't add fields for quite a while. This fixes this. (cherry picked from commit 5d07b87)
|
I think that this change affects the packaging build, failures like the following one can be reproduced in master now when building x-pack heartbeat packages: Seen in #22883 |
Fixes a bug introduced in elastic#21874 where fields.yml generation in x-pack stopped happening.
Fixes a bug introduced in #21874 where fields.yml generation in x-pack stopped happening.
Fixes a bug introduced in elastic#21874 where fields.yml generation in x-pack stopped happening. (cherry picked from commit 78a8625)
Fields generation seems to have broken at some time in the past, but we never noticed, because we didn't add fields for quite a while. This fixes this. I had to centralize all field definitions in the top level
_metafolder, but I'm fine with this change, in many ways this is actually easier.