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

[Elastic Agent] Discuss: Use EQL for constraints in Elastic Agent #19068

Closed
ruflin opened this issue Jun 9, 2020 · 6 comments
Closed

[Elastic Agent] Discuss: Use EQL for constraints in Elastic Agent #19068

ruflin opened this issue Jun 9, 2020 · 6 comments
Assignees
Labels
discuss Issue needs further discussion.

Comments

@ruflin
Copy link
Member

ruflin commented Jun 9, 2020

The Elastic Agent supports constraints/conditions in its configuration to apply certain configs only in certain environments. Details of the discussion can be found here: #16409

The config currently looks as following (examples):

constraints:
  - %{[hello.var]} == 'hello'
  - "validate_version(%{[agent.version]}, '1.0.0 - 7.0.0')"

Elasticsearch is currently in the process of adding support for EQL (https://www.elastic.co/guide/en/elasticsearch/reference/master//eql.html). Looking at the basic syntax of EQL it looks very similar to what we have today: https://eql.readthedocs.io/en/latest/query-guide/basic-syntax.html#conditions

Our users should have to learn as few query languages as possible, that is why I'm proposing we switch to the same syntax as EQL in our constraints.

Currently on our end we use %{[hello.var]} for variables which in EQL would just become hello.var. I'm wonder if this is possible? If not, could we switch to something like moustache as at least this is used in other parts of the stack for variables, for example in Elasticsearch ingest pipelines so it would become {{hello.var}}.

We would not support all functions and options of EQL but a small subset and potentially add a few of our own functions like validate_version. If we name some of the functions the same, behaviour should be the same.

@ruflin
Copy link
Member Author

ruflin commented Jun 9, 2020

@ph @michalpristas @jpountz Would be great to get your thoughts on this.

@ruflin ruflin changed the title Discuss: Use EQL for constraints in Elastic Agent [Elastic Agent] Discuss: Use EQL for constraints in Elastic Agent Jun 9, 2020
@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Jun 9, 2020
@ruflin ruflin added discuss Issue needs further discussion. Team:Ingest Management labels Jun 9, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/ingest-management (Team:Ingest Management)

@botelastic botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Jun 9, 2020
@jpountz
Copy link
Contributor

jpountz commented Jun 9, 2020

I don't think it would be a bad idea to make them consistent, but I don't think it's a requirement. My understanding is that people configuring agents and using EQL would typically be different users (admins vs. analysts), plus EQL is one out of many languages that we plan to support.

@ph
Copy link
Contributor

ph commented Jun 10, 2020

I don't have a strong opinion on variables %{[]} vs {{}}, this was create before EQL exists on our
side.

Currently on our end we use %{[hello.var]} for variables which in EQL would just become hello.var. I'm wonder if this is possible? If not, could we switch to something like moustache as at least this is used in other parts of the stack for variables, for example in Elasticsearch ingest pipelines so it would become {{hello.var}}.

This is possible, without too much effort, I think its even work today (minus the ".", we need to add this to the allowed list of chars.) By default our parser would consider this as a function call that doesn't exist.

it's a 20 lines code changes and regenerate the state machine.

@michalpristas
Copy link
Contributor

from my point of view it would be better to have a single syntax across products it makes look our products more compact and interconnected. as ph said change wont be that difficult parser change + existing configuration (there's not much of it)

@blakerouse
Copy link
Contributor

Closing this because of #20784

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss Issue needs further discussion.
Projects
None yet
Development

No branches or pull requests

6 participants