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

Add flags/capabilities system #292

Closed
andrewvc opened this issue May 27, 2021 · 5 comments · Fixed by #295
Closed

Add flags/capabilities system #292

andrewvc opened this issue May 27, 2021 · 5 comments · Fixed by #295
Labels
enhancement New feature or request

Comments

@andrewvc
Copy link
Contributor

We need a system whereby heartbeat can let the synthetics library know how wants to behave at a feature level. As we develop new features they may require new functionality in kibana or elasticsearch or heartbeat to function correctly. The problem is that the synthetics library versioning is completely independent of the rest of the stack. We could solve a number of ways, but one way that does not work is adding additional CLI flags. The main reason that is not a good idea is that a newer version of heartbeat would not work with an older version of the synthetics library that didn't support a newer flag.

I can think of two ways we can do this and I'm not certain which one is the best:

  1. Add a --stack-version=X.Y.Z flag to synthetics, which can be used to determine which features are on/off
  2. Add a --capability screenshot_refs --capability somenewcthing flag, to enable new features on a one-by-one basis

The advantage of the first approach is that no updates to the heartbeat code have to happen to add a new capability. The advantage of the second approach is that it's quite a bit cleaner and it is more related CLI testing.

We could also do a combination of approaches where setting the stack version sets a certain set of capabilities.

@andrewvc andrewvc added enhancement New feature or request [zube]: Ready labels May 27, 2021
@andrewvc
Copy link
Contributor Author

This came up in #290 and as a blocker for that issue

@paulb-elastic
Copy link
Contributor

I assume the starting point for this has to be a version (in terms of how Heartbeat itself is triggered to launch a test), in which case I would think it makes sense to pass this into Synthetics to work out how to behave (it seems more complex to have Heartbeat track the Synthetic capabilities based on the stack version and then send these to Synthetics).

Irrespective of which option to go with, how does Heartbeat know the stack version?

  • In the case of Fleet, it’s probably easier as I suspect we could include --stack-version=X.Y.Z in the configuration automatically.
  • In the case of a self managed heartbeat.yml, would Heartbeat have to make a query first to determine the stack version?
  • In the case of the Synthetics Service, the requesting platform (Heartbeat running in Cloud, or self-managed) would need to include the destination stack version in the API request when initiating the test.

In all cases, Heartbeat passing the version to Synthetics seems simpler (or Heartbeat would need to know how to convert these into Synthetic capabilities - which seems like a job for Synthetics).

@vigneshshanmugam
Copy link
Member

Thanks for starting it Andrew, While working on the PR I was struck by the same thing and thought of moving the feature inside feature flag to not break the Uptime UI.

One good thing about Synthetics/APM agents is the faster release cycle without affecting the stack releases, so in any case I think we should still retain that.

Here are my thoughts

  1. Every new change that is going to break the UI/Heartbeat in some way need to be behind a feature flag in Synthetics agent and we should still support the previous way. An example here is Screenshots/Moving network fields to ECS, we need to keep the old step/screenshot and write them as screenshots/blocks only when the new flag --capabilities <screenshots> is passed.

  2. Config grouping which we already discussed Add --rich-metadata flag to enable all rich metrics #289 that would allow heartbeat to use new features of Synthetics without modifying heartbeat or uptime UI code. We could accomplish this using --rich-events or some other flag that would let synthetics spit out events like screenshots, network, core vitals. An example here is the new Core vitals metrics, which are behind the flag --trace in Synthetics code, but when --rich-events is passed all these metrics would be activated.

--rich-events would switch on --network, --screenshot, --trace, --filmstrip in Synthetics code. I am still not convinced if we need to index these new events by default as it could result in increased storage for users using a new version of synthetics without actually utilizing them on the UI.

  1. If the feature cannot be behind feature flags or if its breaking change that needs changes in both Synthetics and Heartbeat, we release a new major version of Synthetics and release it as part of new stack release and change the validation in heartbeat to support only from that current version. We already do validation now in heartbeat - https://github.com/elastic/beats/blob/a1c768e00d07ac1c1c658363063097396226942c/x-pack/heartbeat/monitors/browser/source/validatepackage.go#L19

Thoughts @andrewvc @paulb-elastic

@andrewvc
Copy link
Contributor Author

@paulb-elastic it's much simpler than that, heartbeat knows its version when its compiled :) Looking through the rest of the comments now

@andrewvc
Copy link
Contributor Author

Great point about how this would work with a synthetics service @paulb-elastic I think in that case we could provide a version override in the config sent to the service for execution "tell synthetics we are version X.Y.Z". Otherwise, I think we can assume the compiled version. It's really about heartbeat compat, not ES or kibana.

@vigneshshanmugam +1 on your thoughts. including major versioning

In terms of action it sounds like we should:

  1. Enable --rich-events as discussed in Add --rich-metadata flag to enable all rich metrics #289
  2. Add --heartbeat-compatible X.Y.Z
  3. Add --capabilities for feature flags.

Any objection to doing all the things here?

andrewvc pushed a commit that referenced this issue Jun 3, 2021
fix #292
Allows for a way to pass an variadic args to --capability flag that controls the features inside the agent.
npx @elastic/synthetics examples/todos --capability metrics trace --capability filmstrips
Also deprecated the previous --trace, --metrics, --filmstrips flag which are not required anymore and not used by heartbeat currently.
@zube zube bot added [zube]: Done and removed [zube]: Ready labels Jun 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants