[Heartbeat] Build elastic agent with synthetics support / offline mode#27052
[Heartbeat] Build elastic agent with synthetics support / offline mode#27052andrewvc merged 15 commits intoelastic:masterfrom
Conversation
|
Pinging @elastic/uptime (Team:Uptime) |
💚 Build Succeeded
Expand to view the summary
Build stats
Test stats 🧪
Trends 🧪💚 Flaky test reportTests succeeded. Expand to view the summary
Test stats 🧪
|
|
Linking to #26398 |
|
I see a checkbox for documentation. We documented the docker container here https://github.com/elastic/observability-docs/issues/622. Can you add content for the offline mode or at least create an issue to track adding the docs? |
|
As a first step I think the most important part is that it does not affect the current build we have. Would be good to validate this with a package build. I did not test it locally or go into the detail about overall LGTM. @andrewkroh @blakerouse Could you take a closer look at this as you are more familiar with the agent docker configs and the build system overall and might spot some side effects of this? |
dev-tools/mage/dockerbuilder.go
Outdated
|
|
||
| func (b *dockerBuilder) Build() error { | ||
| variants := []string{""} | ||
| if os.Getenv("OFFLINE_DOCKER_IMAGE") == "true" || b.ServiceName == "elastic-agent" { |
There was a problem hiding this comment.
We should document this somewhere in the Beats repo?
There was a problem hiding this comment.
Is there a place we already document similar things? It's my understanding that we don't have such a place. I'd be glad to document it, my only question is where?
There was a problem hiding this comment.
I started to put things into elastic agent readme: https://github.com/elastic/beats/tree/master/x-pack/elastic-agent Lets put it there so we have it and can figure out a better place later.
There was a problem hiding this comment.
I've since removed this and replaced it in the package spec, where it has an explanatory comment. I've also added a note to the README you mentioned about the behavior of the package spec.
@mostlyjason we have this captured in https://github.com/elastic/observability-docs/issues/663 (I've added a reference to the existing docs page you mentioned) |
andrewkroh
left a comment
There was a problem hiding this comment.
If there is an "offline" image available I would expect it to include other artifacts that are not currently bundled with Agent like osquerybeat. Do others have this expectation? Should we add that as well?
| @@ -58,6 +59,11 @@ func newDockerBuilder(spec PackageSpec) (*dockerBuilder, error) { | |||
| } | |||
|
|
|||
| func (b *dockerBuilder) Build() error { | |||
There was a problem hiding this comment.
This doesn't seem like the right place for any project specific build options. IIRC this code path doesn't have knowledge of elastic-agent at all.
If there are new artifacts that the Elastic Agent build should produce those should be defined in a package spec YAML (elastic-agent appears to be adding artifacts to the https://github.com/elastic/beats/blob/master/dev-tools/packaging/packages.yml#L938-L939).
|
@andrewkroh ++ on your proposal that the "offline" image contains "all" of it. |
Co-authored-by: Manuel de la Peña <social.mdelapenya@gmail.com>
|
My latest push removes the assigning of |
|
|
||
| # Contains the elastic agent image variant, an empty string for the standard variant | ||
| # or "offline" for the offline one. | ||
| ENV ELASTIC_AGENT_IMAGE_VARIANT={{.Variant}} |
There was a problem hiding this comment.
What do you think about omitting this from the environment when .Variant is empty?
There was a problem hiding this comment.
I'm neutral on it. I do kind of like having it empty because it makes it maybe more explicit that this is the "" variant.
There was a problem hiding this comment.
IIUC, this particular variable does nothing but reporting the docker image is a variant? If so, maybe the docker metadata attributes could be a better approach?
ruflin
left a comment
There was a problem hiding this comment.
Code change LGTM. Only one part I'm not sure how it is related.
I assume you did a full package run to see if things still work as expected? Lets make sure as soon as we get this in we keep an eye on the builds to make sure these still pass.
For the publishing of the images, I assume some additional work in the release manager will be needed?
| fallthrough | ||
| case "screenshot/block": | ||
| add_data_stream_index.SetEventDataset(event, "browser_screenshot") | ||
| add_data_stream_index.SetEventDataset(event, "browser.screenshot") |
There was a problem hiding this comment.
The main purpose of this PR, really, is to enable the integration, which breaks without this. I agree it's a bit unrelated, but I think it makes sense to include because this PR isn't useful without it.
| {{- if (and (eq .Variant "offline") (not (contains .from "ubi-minimal"))) }} | ||
| # Setup synthetics env vars | ||
| ENV ELASTIC_SYNTHETICS_CAPABLE=true | ||
| ENV SUITES_DIR={{ $beatHome }}/suites |
There was a problem hiding this comment.
Are we using a dedicated folder instead of /tmp for copying suites? I forgot what wat was the real use of this flag.
There was a problem hiding this comment.
This can probably be nixed. It's not really used for anything.
| add_data_stream_index.SetEventDataset(event, "browser.screenshot") | ||
| case "journey/network_info": | ||
| add_data_stream_index.SetEventDataset(event, "browser_network") | ||
| add_data_stream_index.SetEventDataset(event, "browser.network") |
There was a problem hiding this comment.
wouldn't it break for existing users?
There was a problem hiding this comment.
No, existing users don't use datasets, only people using the integration do. For existing users it's all under heartbeat-*
|
@Mergifyio backport 7.x |
#27052) Creates a new offline tagged docker image that includes the dependencies required for synthetics. Building elastic-agent docker images will now create additional -offline tagged images with these extras. We may need to do additional work to ensure that these additional images are incorporated into the release process and onto the public website. These changes also set the ELASTIC_SYNTHETICS_CAPABLE env flag, as done in heartbeat, to enable synthetics features in the docker environment. This dovetails with the work @dominiqueclarke is doing in elastic/integrations#1064 and elsewhere Fixes #22932 (cherry picked from commit 4727470)
|
Command
|
#27052) (#27324) Creates a new offline tagged docker image that includes the dependencies required for synthetics. Building elastic-agent docker images will now create additional -offline tagged images with these extras. We may need to do additional work to ensure that these additional images are incorporated into the release process and onto the public website. These changes also set the ELASTIC_SYNTHETICS_CAPABLE env flag, as done in heartbeat, to enable synthetics features in the docker environment. This dovetails with the work @dominiqueclarke is doing in elastic/integrations#1064 and elsewhere Fixes #22932 (cherry picked from commit 4727470) Co-authored-by: Andrew Cholakian <andrew@andrewvc.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
|
@Mergifyio backport 7.15 |
#27052) Creates a new offline tagged docker image that includes the dependencies required for synthetics. Building elastic-agent docker images will now create additional -offline tagged images with these extras. We may need to do additional work to ensure that these additional images are incorporated into the release process and onto the public website. These changes also set the ELASTIC_SYNTHETICS_CAPABLE env flag, as done in heartbeat, to enable synthetics features in the docker environment. This dovetails with the work @dominiqueclarke is doing in elastic/integrations#1064 and elsewhere Fixes #22932 (cherry picked from commit 4727470) # Conflicts: # dev-tools/mage/dockerbuilder.go
|
Command
|
#27052) (#27324) Creates a new offline tagged docker image that includes the dependencies required for synthetics. Building elastic-agent docker images will now create additional -offline tagged images with these extras. We may need to do additional work to ensure that these additional images are incorporated into the release process and onto the public website. These changes also set the ELASTIC_SYNTHETICS_CAPABLE env flag, as done in heartbeat, to enable synthetics features in the docker environment. This dovetails with the work @dominiqueclarke is doing in elastic/integrations#1064 and elsewhere Fixes #22932 (cherry picked from commit 4727470) Co-authored-by: Andrew Cholakian <andrew@andrewvc.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> (cherry picked from commit f9b2610) # Conflicts: # dev-tools/mage/dockerbuilder.go
|
This pull request does not have a backport label. Could you fix it @andrewvc? 🙏
NOTE: |
Creates a new
offlinetagged docker image that includes the dependencies required for synthetics. Building elastic-agent docker images will now create additional-offlinetagged images with these extras. We may need to do additional work to ensure that these additional images are incorporated into the release process and onto the public website.These changes also set the
ELASTIC_SYNTHETICS_CAPABLEenv flag, as done in heartbeat, to enable synthetics features in the docker environment. This dovetails with the work @dominiqueclarke is doing in elastic/integrations#1064 and elsewhereFixes #22932
CC @ruflin @v1v @mostlyjason @drewpost @vigneshshanmugam @dominiqueclarke
Checklist
- [ ] I have made corresponding change to the default configuration files- [ ] I have added tests that prove my fix is effective or that my feature worksCHANGELOG.next.asciidocorCHANGELOG-developer.next.asciidoc.How to test this PR locally
TODO: Test this with the upcoming synthetics support in integrations (see elastic/integrations#1064)