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

Table function for supporting prometheus query_range function #875

Merged
merged 1 commit into from
Oct 20, 2022

Conversation

vamsi-amazon
Copy link
Member

@vamsi-amazon vamsi-amazon commented Sep 30, 2022

Description

  • This CR includes changes for initial implementation of table functions targeting prometheus API.
  • Table functions are functions which return tables as output. Table functions can be defined for any of the connector and will be pushed down.
  • New query_range table function is defined to support pass through promQL query directly to prometheus.

These functions can be called either with named parameters or positioned parameters.

Example queries for query_range.
Named parameters query.
source = prometheus.query_range(query="promQL", start=123, end=145, step=45)

Positioned parameters query.
source = prometheus.query_range("promQL", 123, 145, 45)

Issues Related:

Check List

  • [x ] New functionality includes testing.
    • [x ] All tests pass, including unit test, integration test and doctest
  • New functionality has been documented.
    • New functionality has javadoc added
    • New functionality has user manual doc added
  • [ x] Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@codecov-commenter
Copy link

codecov-commenter commented Sep 30, 2022

Codecov Report

Merging #875 (3526376) into 2.x (b30d156) will decrease coverage by 2.71%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##                2.x     #875      +/-   ##
============================================
- Coverage     97.90%   95.18%   -2.72%     
- Complexity     3072     3126      +54     
============================================
  Files           293      309      +16     
  Lines          7588     8392     +804     
  Branches        490      618     +128     
============================================
+ Hits           7429     7988     +559     
- Misses          158      350     +192     
- Partials          1       54      +53     
Flag Coverage Δ
query-workbench 62.76% <ø> (?)
sql-engine 97.94% <100.00%> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...c/main/java/org/opensearch/sql/expression/DSL.java 100.00% <ø> (ø)
...sql/prometheus/request/PrometheusQueryRequest.java 100.00% <ø> (ø)
...h/sql/prometheus/storage/PrometheusMetricScan.java 100.00% <ø> (ø)
...ain/java/org/opensearch/sql/analysis/Analyzer.java 100.00% <100.00%> (ø)
...expression/function/BuiltinFunctionRepository.java 100.00% <100.00%> (ø)
...java/org/opensearch/sql/storage/StorageEngine.java 100.00% <100.00%> (ø)
...c/main/java/org/opensearch/sql/ppl/PPLService.java 100.00% <100.00%> (ø)
...rg/opensearch/sql/ppl/config/PPLServiceConfig.java 100.00% <100.00%> (ø)
...java/org/opensearch/sql/ppl/parser/AstBuilder.java 100.00% <100.00%> (ø)
...ensearch/sql/ppl/utils/PPLQueryDataAnonymizer.java 100.00% <100.00%> (ø)
... and 16 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@vamsi-amazon vamsi-amazon marked this pull request as ready for review September 30, 2022 02:07
@vamsi-amazon vamsi-amazon requested a review from a team as a code owner September 30, 2022 02:07
@vamsi-amazon vamsi-amazon marked this pull request as draft September 30, 2022 02:33
@vamsi-amazon vamsi-amazon force-pushed the table_function branch 4 times, most recently from 913dbde to d95305d Compare September 30, 2022 17:54
@vamsi-amazon vamsi-amazon marked this pull request as ready for review September 30, 2022 18:10
@vamsi-amazon vamsi-amazon force-pushed the table_function branch 14 times, most recently from 9d05fca to f3b82e7 Compare October 17, 2022 21:12
@vamsi-amazon
Copy link
Member Author

I have some suggestion about the table function. In current implementation, it is not part of the existing expression system. In particular, the table function has its own interface ConnectorTableFunction and its signature is resolved in StorageEngine.getTableFunction. Can we introduce it into the existing expression system instead of making it a special case?

I checked our code and found the resolution logic is captured by FunctionResolver interface. There is a default one and RelevanceFunctionResolver which is similar to what table function wants.

// During initialization:
for storage : allStorageEngines:
    functionRepository.register(storage.getFunctions())

class PrometheusStorageEngine:
    def List<FunctionResolver> getFunctions():
        return [queryRange(), query() ...]

class Analyzer:
    def visitTableFunction(astNode):
        tableFunc = functionRepository.compile(astNode)
        env = { (arg.name, arg.value) for astNode.argument() }
        return LogicalRelation(tableFunc.valueOf(env)).  // use existing valueOf or add new interface if inappropriate

class PrometheusFunctions:
    def queryRange():
        return functionBuilder.build...

    def query():
        return functionBuilder.build...

Let me know if this makes sense. Thanks!

Similar to first iteration except that functions are exposed via storageengine. Will try to incorporate.

@vamsi-amazon vamsi-amazon force-pushed the table_function branch 5 times, most recently from be9e53d to ebda4b1 Compare October 17, 2022 22:58
@vamsi-amazon vamsi-amazon marked this pull request as ready for review October 17, 2022 23:22
Signed-off-by: vamsi-amazon <reddyvam@amazon.com>
@vamsi-amazon vamsi-amazon changed the title Table function for supporting prometheus Native Query. Table function for supporting prometheus query_range function Oct 19, 2022
Copy link
Collaborator

@penghuo penghuo left a comment

Choose a reason for hiding this comment

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

please update the docs.

@vamsi-amazon
Copy link
Member Author

please update the docs.

I will raise one this week for catalogs and all.

Copy link
Collaborator

@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!

@vamsi-amazon vamsi-amazon merged commit 863f751 into opensearch-project:2.x Oct 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants