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

[Rest Api Compatibility] Framework support for EQL and SQL endpoints #92710

Open
pgomulka opened this issue Jan 5, 2023 · 7 comments
Open

[Rest Api Compatibility] Framework support for EQL and SQL endpoints #92710

pgomulka opened this issue Jan 5, 2023 · 7 comments
Assignees
Labels
:Analytics/EQL EQL querying :Analytics/SQL SQL querying :Core/Infra/REST API REST infrastructure and utilities >enhancement Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) Team:Core/Infra Meta label for core/infra team

Comments

@pgomulka
Copy link
Contributor

pgomulka commented Jan 5, 2023

Description

Rest api compatibility was not covering the support for eql and sql plugins (not supporting any plugins) at the time when we were implementing it.

SQL and EQL have a custom logic on creating XContentBuilder.
Those two places (and possibly more) should be revisited to make sure the right parameters are passed in when creating XContentBuilder

XContentBuilder builder = channel.newBuilder(request.getXContentType(), type, false);

XContentBuilder builder = channel.newBuilder(request.getXContentType(), XContentType.JSON, true);

I recon at some point we can consider extending the rest api compatibility coverage to plugins and fix this for EQL and SQL. The rest api compatibility framework should support additional media types used in SQL (just headers formatting, not request/respone parsing)

steps to reproduce currently not working behaviour

curl --request PUT \
  --url http://localhost:9200/xxx \
  --header 'Accept: application/vnd.elasticsearch+json;compatible-with=8' \
  --header 'Authorization: Basic ZWxhc3RpYzpwYXNzd29yZA==' \
  --header 'Content-Type: application/vnd.elasticsearch+json;compatible-with=8'

curl --request GET \
  --url http://localhost:9200/xxx/_eql/search \
  --header 'Accept: application/vnd.elasticsearch+json;compatible-with=8' \
  --header 'Authorization: Basic ZWxhc3RpYzpwYXNzd29yZA==' \
  --header 'Content-Type: application/vnd.elasticsearch+json;compatible-with=8' \
  --data '{"keep_on_completion":true,"query":"any where true","timestamp_field":"timestamp","wait_for_completion_timeout":"1nanos"}
' -v
*   Trying 127.0.0.1:9200...
* Connected to localhost (127.0.0.1) port 9200 (#0)
> GET /xxx/_eql/search HTTP/1.1
> Host: localhost:9200
> User-Agent: curl/7.79.1
> Accept: application/vnd.elasticsearch+json;compatible-with=8
> Authorization: Basic ZWxhc3RpYzpwYXNzd29yZA==
> Content-Type: application/vnd.elasticsearch+json;compatible-with=8
> Content-Length: 122
> 
* Mark bundle as not supporting multiuse
< HTTP/1.1 200 OK
< X-elastic-product: Elasticsearch
< content-type: application/json;compatible-with=8
< content-length: 150
< 
* Connection #0 to host localhost left intact
{"id":"Fnc2czQ2Vk1KVHRteWo2MUQxdlVtblEaWU4zYVNxOTlTWS1hNzctckN5U3hrZzo1ODA=","is_partial":true,"is_running":true,"took":3,"timed_out":false,"hits":{}}%    
@pgomulka pgomulka added >enhancement :Core/Infra/REST API REST infrastructure and utilities :Analytics/SQL SQL querying :Analytics/EQL EQL querying needs:triage Requires assignment of a team area label labels Jan 5, 2023
@pgomulka pgomulka self-assigned this Jan 5, 2023
@elasticsearchmachine elasticsearchmachine added Team:Core/Infra Meta label for core/infra team Team:QL (Deprecated) Meta label for query languages team labels Jan 5, 2023
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (Team:Core/Infra)

@elasticsearchmachine elasticsearchmachine removed the needs:triage Requires assignment of a team area label label Jan 5, 2023
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-ql (Team:QL)

@stevejgordon
Copy link
Contributor

Thanks for raising this, @pgomulka. We have a linked issue in the .NET client which may be caused by potential differences in the response. I'll look into that more deeply on our end to check what seems to have changed. It would be helpful to ensure clients running in compatibility mode can use EQL and SQL. Right now, it's the only way for .NET consumers to work with a v8 server in .NET, so consumption may be broken without it.

@sethmlarson
Copy link
Contributor

+1 on having compatibility mode preserve the structure of responses in 7.x for EQL and SQL, going forward all API endpoints should respect compatibility mode negotiation.

@bpintea
Copy link
Contributor

bpintea commented Jan 26, 2023

Those two places (and possibly more) should be revisited to make sure the right parameters are passed in when creating XContentBuilder

Linking #56030 to potentially also revisit and #52587 just for behavior awareness.

@wchaparro wchaparro added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label Jan 2, 2024
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-analytics-geo (Team:Analytics)

@elasticsearchmachine elasticsearchmachine removed the Team:QL (Deprecated) Meta label for query languages team label Jan 2, 2024
@rjernst rjernst assigned rjernst and unassigned pgomulka Aug 6, 2024
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-analytical-engine (Team:Analytics)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Analytics/EQL EQL querying :Analytics/SQL SQL querying :Core/Infra/REST API REST infrastructure and utilities >enhancement Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) Team:Core/Infra Meta label for core/infra team
Projects
None yet
Development

No branches or pull requests

7 participants