Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

Added metrics for SQL query requests in new engine #905

Merged

Conversation

chloe-zh
Copy link
Member

@chloe-zh chloe-zh commented Dec 10, 2020

Issue #, if available:

Description of changes:

Currently the strategy of the rest query action can inform the metrics and increment the request total before deciding which engine the query should go into. But after the query comes to the new engine, the metrics publish is missing if any error turns up during executing or explaining in new engine (while the old engine can still publish the metrics if meets with errors).

This pull request is to add metrics publish when new engine meets request failure. If the exception QueryEngineException type of its extension type, the stats for failed request count in server (FAILED_REQ_COUNT_SYS) increments. Else the stats for failed request count in client (FAILED_REQ_COUNT_CUS) increments.

Besides here also adds the metrics integration tests of success request cases for SQL query requests in new engine.

The failure cases are not tested in integration test because time amount to finish the metrics update might vary. Here attaches the manual sanity test of related stats in metrics, the sleep time was set to 30s, the stats were obtained from local ES log:

  • Execute a query supported in new engine and does not meet any errors
[2020-12-10T15:43:31,936][INFO ][c.a.o.s.l.p.RestSqlAction] [147dda5ddf8a.ant.amazon.com] 
Before executing SQL query:
Request total: 0,
Failed client request count: 0,
Failed server request count 0
[2020-12-10T15:43:32,004][INFO ][c.a.o.s.l.p.RestSqlAction] [147dda5ddf8a.ant.amazon.com] [9d2e3afa-4663-492d-acee-8dcc1a7d28d7] Incoming request /_opendistro/_sql?pretty=true: ( SELECT DATE('string_literal') )
[2020-12-10T15:43:32,626][INFO ][c.a.o.s.l.p.RestSqlAction] [147dda5ddf8a.ant.amazon.com] [9d2e3afa-4663-492d-acee-8dcc1a7d28d7] Request SQLQueryRequest(jsonContent={"query":"SELECT DATE('2020-12-10')"}, query=SELECT DATE('2020-12-10'), path=/_opendistro/_sql, format=jdbc) is handled by new SQL query engine
[2020-12-10T15:43:42,681][INFO ][c.a.o.s.l.p.RestSQLQueryAction] [147dda5ddf8a.ant.amazon.com] 
After executing SQL query:
Request total: 1,
Failed client request count: 0,
Failed server request count 0
  • Execute a query supported in new engine but meet some errors in core engine computing:
[2020-12-10T15:43:48,180][INFO ][c.a.o.s.l.p.RestSqlAction] [147dda5ddf8a.ant.amazon.com] 
Before executing SQL query:
Request total: 1,
Failed client request count: 0,
Failed server request count 0
[2020-12-10T15:43:48,181][INFO ][c.a.o.s.l.p.RestSqlAction] [147dda5ddf8a.ant.amazon.com] [0f2e4929-353c-4a92-b722-8e995e315cd9] Incoming request /_opendistro/_sql?pretty=true: ( SELECT DATE('string_literal') )
[2020-12-10T15:43:48,202][INFO ][c.a.o.s.l.p.RestSqlAction] [147dda5ddf8a.ant.amazon.com] [0f2e4929-353c-4a92-b722-8e995e315cd9] Request SQLQueryRequest(jsonContent={"query":"SELECT DATE('0')"}, query=SELECT DATE('0'), path=/_opendistro/_sql, format=jdbc) is handled by new SQL query engine
[2020-12-10T15:43:48,203][ERROR][c.a.o.s.l.p.RestSQLQueryAction] [147dda5ddf8a.ant.amazon.com] Error happened during query handling
com.amazon.opendistroforelasticsearch.sql.exception.SemanticCheckException: date:0 in unsupported format, please use yyyy-MM-dd
	at com.amazon.opendistroforelasticsearch.sql.data.model.ExprDateValue.<init>(ExprDateValue.java:49) ~[core-1.12.0.0.jar:?]
	at com.amazon.opendistroforelasticsearch.sql.expression.datetime.DateTimeFunction.exprDate(DateTimeFunction.java:459) ~[core-1.12.0.0.jar:?]
	at com.amazon.opendistroforelasticsearch.sql.expression.function.FunctionDSL.lambda$nullMissingHandling$5e7b55fb$1(FunctionDSL.java:245) ~[core-1.12.0.0.jar:?]

...

[2020-12-10T15:43:58,217][INFO ][c.a.o.s.l.p.RestSQLQueryAction] [147dda5ddf8a.ant.amazon.com] 
After executing SQL query:
Request total: 2,
Failed client request count: 0,
Failed server request count 1
  • Stats API behavior:
// before executing
GET /_opendistro/_sql/stats

{"ppl_request_total":0,"failed_request_count_cb":0,"default_cursor_request_count":0,"default_cursor_request_total":0,"ppl_failed_request_count_syserr":0,"failed_request_count_cuserr":0,"circuit_breaker":0,"request_total":0,"ppl_failed_request_count_cuserr":0,"ppl_request_count":0,"request_count":0,"failed_request_count_syserr":0}

// executed a good query
POST _opendistro/_sql
{
  "query": "SELECT DATE('2020-12-10')"
}

GET /_opendistro/_sql/stats

{"ppl_request_total":0,"failed_request_count_cb":0,"default_cursor_request_count":0,"default_cursor_request_total":0,"ppl_failed_request_count_syserr":0,"failed_request_count_cuserr":0,"circuit_breaker":0,"request_total":1,"ppl_failed_request_count_cuserr":0,"ppl_request_count":0,"request_count":0,"failed_request_count_syserr":0}

// executed a bad query and threw `SemanticCheckException`
POST /_opendistro/_sql
{
  "query": "SELECT DATE('0')"
}

After around 30 sec...

GET /_opendistro/_sql/stats

{"ppl_request_total":0,"failed_request_count_cb":0,"default_cursor_request_count":0,"default_cursor_request_total":0,"ppl_failed_request_count_syserr":0,"failed_request_count_cuserr":0,"circuit_breaker":0,"request_total":2,"ppl_failed_request_count_cuserr":0,"ppl_request_count":0,"request_count":2,"failed_request_count_syserr":1}

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@chloe-zh chloe-zh self-assigned this Dec 10, 2020
@chloe-zh chloe-zh added the SQL label Dec 10, 2020
@codecov
Copy link

codecov bot commented Dec 10, 2020

Codecov Report

Merging #905 (40a0bb6) into develop (0cb3240) will increase coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop     #905      +/-   ##
=============================================
+ Coverage      99.85%   99.86%   +0.01%     
- Complexity      2148     2315     +167     
=============================================
  Files            216      230      +14     
  Lines           4850     5329     +479     
  Branches         320      346      +26     
=============================================
+ Hits            4843     5322     +479     
  Misses             5        5              
  Partials           2        2              
Impacted Files Coverage Δ Complexity Δ
...opendistroforelasticsearch/sql/expression/DSL.java 100.00% <0.00%> (ø) 128.00% <0.00%> (+9.00%)
...endistroforelasticsearch/sql/executor/Explain.java 100.00% <0.00%> (ø) 28.00% <0.00%> (+1.00%)
...ndistroforelasticsearch/sql/analysis/Analyzer.java 100.00% <0.00%> (ø) 44.00% <0.00%> (+1.00%)
...distroforelasticsearch/sql/data/type/ExprType.java 100.00% <0.00%> (ø) 7.00% <0.00%> (+1.00%)
...troforelasticsearch/sql/sql/parser/AstBuilder.java 100.00% <0.00%> (ø) 28.00% <0.00%> (+6.00%)
...roforelasticsearch/sql/data/type/ExprCoreType.java 100.00% <0.00%> (ø) 10.00% <0.00%> (+1.00%)
...relasticsearch/sql/ppl/domain/PPLQueryRequest.java 100.00% <0.00%> (ø) 6.00% <0.00%> (+3.00%)
...relasticsearch/sql/sql/domain/SQLQueryRequest.java 100.00% <0.00%> (ø) 21.00% <0.00%> (+8.00%)
...relasticsearch/sql/planner/DefaultImplementor.java 100.00% <0.00%> (ø) 16.00% <0.00%> (+1.00%)
...elasticsearch/sql/analysis/ExpressionAnalyzer.java 100.00% <0.00%> (ø) 31.00% <0.00%> (+1.00%)
... and 45 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0cb3240...40a0bb6. Read the comment docs.

@chloe-zh chloe-zh marked this pull request as ready for review December 11, 2020 00:07
Copy link
Member

@dai-chen dai-chen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the changes!

@chloe-zh chloe-zh merged commit ca9f578 into opendistro-for-elasticsearch:develop Dec 16, 2020
penghuo pushed a commit to penghuo/sql that referenced this pull request Dec 17, 2020
…asticsearch#905)

* added metrics in sql new engine query action when errors occur during query execution

* addressed comments

* update

* take all errors from new query engine as server errors
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants