Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Heartbeat] Add synthetics deps to main heartbeat docker image #22837

Closed
andrewvc opened this issue Dec 1, 2020 · 11 comments · Fixed by #23274
Closed

[Heartbeat] Add synthetics deps to main heartbeat docker image #22837

andrewvc opened this issue Dec 1, 2020 · 11 comments · Fixed by #23274
Assignees
Labels
discuss Issue needs further discussion. Heartbeat Team:obs-ds-hosted-services Label for the Observability Hosted Services team

Comments

@andrewvc
Copy link
Contributor

andrewvc commented Dec 1, 2020

As part of moving synthetics to beta we need to have a better packaging strategy. This issue proposes that we include synthetics in our standard heartbeat docker release. This would, unfortunately have the result of dramatically increasing the size of this docker image due to the inclusion of the chrome browser, dependencies for it, and node runtime. This would increase our docker image size from 327MiB to 1.15GiB.

That's a significant change, one we should discuss here. The only alternative to that would be to continue releasing separate images but this is a bad idea for a few reasons:

  1. Heartbeat is usually installed on a dedicated server (esp. if it's installed as a docker image), so the extra disk space is probably not an issue.
  2. Maintaining two docker images will be confusing for users
  3. The heartbeat docker image will eventually lose favor to the agent docker image anyway, where we'll have to revisit this same issue, so putting the extra work into maintaining two docker images there still doesn't make sense.

The work to do this would essentially involve 👍

  1. Adding the synthetics Dockerfile to the current Heartbeat Dockerfile.
  2. Patching heartbeat to periodically update the synthetics node package to keep the browser evergreen

This would also mean that we would no longer release synthetics docker images on their own schedule, but rather, only with the stack.

@andrewvc andrewvc added discuss Issue needs further discussion. Heartbeat labels Dec 1, 2020
@andrewvc andrewvc self-assigned this Dec 1, 2020
@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Dec 1, 2020
@alvarolobato
Copy link

This would also mean that we would no longer release synthetics docker images on their own schedule
@andrewvc would this mean we won't be able to update Chrome version off-cycle?
I thought that was important.

@andrewvc
Copy link
Contributor Author

andrewvc commented Dec 1, 2020

@alvarolobato that's a great point, it's something we've discussed but I forgot to add to this ticket. What we've discussed as a solution to this is adding a feature to heartbeat to keep the synthetics library updated. So, the container wouldn't change, but the node package would be periodically attempting an upgrade.

I've updated the issue here to include that detail

@andrewvc andrewvc added Team:obs-ds-hosted-services Label for the Observability Hosted Services team and removed needs_team Indicates that the issue/PR needs a Team:* label labels Dec 1, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/uptime (Team:Uptime)

@ruflin
Copy link
Contributor

ruflin commented Dec 2, 2020

Would this have any direct affect on the Elastic Agent and the Elastic Agent docker image as soon as we support heartbeat or is this separate?

++ on having the default image with synthetics. If the larger size or the additional content causes issues for some users, we could still have in the future a second container -minimal if needed.

@andrewvc Can you go a bit more into details what parts of the image are updated "on demand" and which ones are not? Even though I agree the async update will be important i'm also concerned about the side affects of it. Would it mean if I run .../heartbeat:7.12.2 today and two days later, the content inside might be different because of a different browser version?

@paulb-elastic
Copy link
Contributor

I agree that having two images is just going to get confusing for users, especially if they have for example an HTTP Ping monitor working and then one day decide to add a Synthetics monitor, and it just doesn’t work because they have the wrong image.

Having it not bundled (but downloaded on demand, for example), means that the user has to 'pay the price' of the download each time Heartbeat is started up, as the container is ephemeral. This would be quite significant in size/bandwidth and time delay before Heartbeat can actually start monitoring, so seems unreasonable.

Whilst the larger image size will need deploying, at least it means that everything needed by any of the Heartbeat monitors, is deployed and ready.

Although it doesn’t directly impact the to bundle or not discussion, something that’s come up in a few conversations, is about Heartbeat auto-updating (Playwright/Chromium).

One concern is that if Heartbeat just updates as soon as there is a new version of Playwright/Chromium, this might introduce a breaking change that stops Synthetics from working. When we control the release and versions within it, we have at least run our tests to ensure that the packaged components work together.

In this instance, it would probably be best to disable auto updates by default.

An alternative that @alvarolobato mentioned is if we made it get updates from a location we control. This actually dovetails nicely with a separate previous concern that had been raised about where updates are downloaded from.

If we make that a location we control, we can ensure we only deploy updates to Playwright/Chromium when we have tested that it still works with Synthetics.

We might have to think through how we control this, maybe with some form of configuration alongside the downloads, as there will be different versions of the Synthetic Agent out there as well (e.g. Synthetic Agent version n will work with Playwright/Chromium versions a, b or c, but to update to Playwright/Chromium version d requires Synthetic Agent version n+1).

@alvarolobato
Copy link

Thanks Paul.

I would start with auto-updates disabled by default and favor stability, letting the user choose if they want to auto-update with a flag ie. if they find an important bug fix they want to pick up.
In case of security issues, we can always do a stack dot release with an update, so I don't see this situation critical from a security POV and we ensure we have the stability.

Once we have an update repository we control we can change the default to auto-update, but I don´t think this is needed for GA.

@andrewvc
Copy link
Contributor Author

andrewvc commented Dec 2, 2020

@alvarolobato I'm fine with that plan.

@ruflin As far as elastic agent, for an initial release that doesn't support synthetics the size of the agent release would not be affected. For synthetics support, that's the question. We have two choices: 1. Release a single agent docker image with synthetics support @ ~1GiB or 2. Start releasing multiple docker images, a minimal and 'full' one.

As far as what's updated, it's just the NPM synthetics package + deps (which are tiny) and chrome itself. The ~200 OS level deps + transitive deps for GUI stuff wouldn't be updated.

@paulb-elastic raises some good points about versioning, which we discussed during today's tech sync for synthetics. Putting it all together with what @alvarolobato has mentioned let's:

  1. Pin the heartbeat/agent docker image to a single version of synthetics (most current version at time of stack build)
  2. Still let users pick alternate versions of synthetics/playwright in suite package.json files. (for an example see the package.json in our todos example, which explicitly lists the playwright version it uses. This means we still run npm install on suites. This is sort of a requirement for node, which really expects to run inside a directory with a package.json for deps. @vigneshshanmugam and I have experimented with node deps at length and have found global deps unsuitable.
  3. Let's document the package.json situation Need to discuss package.json in synthetics docs observability-docs#284
  4. Implement in heartbeat an optional auto-update feature

Does this plan work for everybody? I'd also like @jahtalab and @vigneshshanmugam 's feedback in particular here

@vigneshshanmugam
Copy link
Member

Sounds good to me and ++ to all the points mentioned above plus disabling the auto-updates.

One concern is that if Heartbeat just updates as soon as there is a new version of Playwright/Chromium, this might introduce a breaking change that stops Synthetics from working.

We did a fix for this one recently by pinning the version of Synthetics to the playwright version elastic/synthetics#150 and disabled the updates of browsers. So we should be good here and I like the idea of controlling the updates from the location we control.

Implement in heartbeat an optional auto-update feature

@andrewvc Do we need this from heartbeat side? Could we just release a minor/patch version updates to our synthetic image on demand when we need an update to either Synthetics or Playwright versions.

@hmdhk
Copy link

hmdhk commented Dec 3, 2020

Sounds good @andrewvc ,

One suggestion (maybe an implementation detail), we could allow patch updates for the synthetics agent. This shouldn't make the package fail if it can't update, but if there's a patch update available on npm we can start using it. Since we control these patch releases and they can be done out of band of the stack release, this gives us flexibility with updating Playwright/Browsers.

re. Number 2. I think we need to warn users that they might be using an incompatible version of agent with heartbeat. Or we could limit the range of versions allowed in package.json. Otherwise it becomes hard to debug IMHO.

@ruflin
Copy link
Contributor

ruflin commented Dec 3, 2020

In the package.json linked, not a specific version that is tagged but it is ^...: https://github.com/elastic/synthetics/blob/master/examples/todos/package.json#L9 I guess this would still lead to different versions used?

@andrewvc
Copy link
Contributor Author

andrewvc commented Dec 4, 2020

At this point it seems we're having 3 separate discussions:

  1. Should we increase the size of the heartbeat docker image? The answer seems to be a clear yes
  2. How do we handle the synthetics/chromium update life cycle? Let's move that discussion to [Heartbeat] Plan for handling synthetics dependencies #22928
  3. Should we increase the size of the agent docker image, or figure out a multi image solution? Let's move that discussion to: [Heartbeat][Agent] Synthetics deps and Elastic Agent #22932

Does that sound right? If the first point about the heartbeat docker image is not felt to be resolved here let's continue the discussion in thread

@andrewvc andrewvc mentioned this issue Dec 23, 2020
6 tasks
andrewvc added a commit that referenced this issue Jan 6, 2021
Adjusts the heartbeat docker image to install deps for heartbeat synthetics. Fixes #22837
andrewvc added a commit to andrewvc/beats that referenced this issue Jan 6, 2021
Adjusts the heartbeat docker image to install deps for heartbeat synthetics. Fixes elastic#22837

(cherry picked from commit dbc61ed)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss Issue needs further discussion. Heartbeat Team:obs-ds-hosted-services Label for the Observability Hosted Services team
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants