Prometheus select metric and stats queries. #956

vamsimanohar · 2022-10-23T13:20:05Z

Description

[Describe what this change achieves]

Covers normal metric commands and stats queries.
Write Integration tests.
Removed support for query_range function in PPL syntax and suppressed tests/coverage for the same.
([Metrics] Bring back support for query_range table function support for prometheus connector. #1019)
Documentation for prometheus connector.

Issues Resolved

#622

(ITs and doctests are followed)

Check List

New functionality includes testing.
- 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
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 · 2022-10-23T13:27:14Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.68%. Comparing base (c230532) to head (e0d6be4).
Report is 472 commits behind head on 2.x.

Additional details and impacted files

@@             Coverage Diff              @@
##                2.x     #956      +/-   ##
============================================
- Coverage     98.20%   95.68%   -2.52%     
- Complexity     3244     3325      +81     
============================================
  Files           313      334      +21     
  Lines          8119     9057     +938     
  Branches        532      672     +140     
============================================
+ Hits           7973     8666     +693     
- Misses          142      334     +192     
- Partials          4       57      +53

Flag	Coverage Δ
query-workbench	`62.76% <ø> (?)`
sql-engine	`98.26% <100.00%> (+0.05%)`	⬆️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: Vamsi Manohar <[email protected]>

integ-test/src/test/java/org/opensearch/sql/ppl/PrometheusCatalogCommandsIT.java

dai-chen · 2022-11-02T21:06:25Z

.../src/main/java/org/opensearch/sql/prometheus/storage/model/PrometheusResponseFieldNames.java

+
+@Getter
+@Setter
+public class PrometheusResponseFieldNames {


As I understand, this carries the metadata of Prometheus response, right? If so, because we represent dynamic Prometheus query as a Table, I think this should be removed and the logic should be moved to PrometheusMetricTable.getFieldTypes() ?

The above code is specifically to handle stats output field names which cannot be decided using getFieldTypes.

In the current state.
Logical project decides the field names in the response schema. It analyzes stat expressions and other expressions to come up with the field names in output response. In case of opensearch, the field name matches with the output response from the opensearch. But prometheus doesn't have any field names associated.

For eg:
source = x | stats avg(@value) by handler;
Opensearch produces output with avg(@value) as field name for average.
Prometheus we need to map the output to the corresponding key.

Also for dynamic prometheus query, there is no easy way to figure out the output structure in prior without running the query.

So prometheusMetricTable.getFieldTypes() doesn't work in case of stats and dynamic queries. [Will try to address this after 2.4]

prometheus/src/main/java/org/opensearch/sql/prometheus/response/PrometheusResponse.java

.../main/java/org/opensearch/sql/prometheus/request/system/PrometheusDescribeMetricRequest.java

prometheus/src/main/java/org/opensearch/sql/prometheus/request/PrometheusQueryRequest.java

integ-test/src/test/java/org/opensearch/sql/ppl/PrometheusCatalogCommandsIT.java

.../main/java/org/opensearch/sql/prometheus/request/system/PrometheusDescribeMetricRequest.java

integ-test/src/test/java/org/opensearch/sql/ppl/PrometheusCatalogCommandsIT.java

penghuo · 2022-11-02T21:35:24Z

...ain/java/org/opensearch/sql/prometheus/storage/implementor/PrometheusDefaultImplementor.java

+    if (spanExpression.isEmpty()) {
+      throw new RuntimeException(
+          "Prometheus Catalog doesn't support aggregations without span expression");
+    }


If span is required, the exception should be thrown at logical -> physical stage, instead of here.

Right, but that would cause issues with opensearch catalog,,,since it would be allowed for them. Don't want to distinguish catalogs in Analyzer component.

Can I do somewhere else?

Introduce validation of logical plan?

Signed-off-by: Vamsi Manohar <[email protected]>

* Prometheus select metric and stats queries. Signed-off-by: reddyvam-amazon <[email protected]> (cherry picked from commit be4512e)

* Prometheus select metric and stats queries. Signed-off-by: vamsi-amazon <[email protected]>

vamsimanohar force-pushed the prom-select-stats branch 13 times, most recently from 8f7f0fd to f39f5c4 Compare October 25, 2022 08:44

vamsimanohar changed the title ~~Prom select stats~~ Prometheus select metric and stats queries. Oct 25, 2022

vamsimanohar linked an issue Oct 25, 2022 that may be closed by this pull request

Implement PPL grammar for Prometheus #622

Closed

vamsimanohar force-pushed the prom-select-stats branch 2 times, most recently from 688f640 to 716d188 Compare October 28, 2022 21:35

vamsimanohar added enhancement New feature or request v2.4.0 'Issues and PRs related to version v2.4.0' labels Oct 28, 2022

vamsimanohar force-pushed the prom-select-stats branch 2 times, most recently from 5d13fea to 4a79875 Compare November 1, 2022 05:27

vamsimanohar added the backport 2.4 label Nov 1, 2022

vamsimanohar force-pushed the prom-select-stats branch 2 times, most recently from 4a190d5 to 619a06b Compare November 1, 2022 10:07

Prometheus select metric and stats queries.

ef37e30

Signed-off-by: Vamsi Manohar <[email protected]>

vamsimanohar force-pushed the prom-select-stats branch 4 times, most recently from 141a018 to a43d827 Compare November 1, 2022 18:29

vamsimanohar force-pushed the prom-select-stats branch 8 times, most recently from e3e74a4 to e86f7f5 Compare November 2, 2022 16:44

vamsimanohar marked this pull request as ready for review November 2, 2022 16:46

vamsimanohar requested a review from a team as a code owner November 2, 2022 16:46

Removed labels

97badc3

Signed-off-by: Vamsi Manohar <[email protected]>

vamsimanohar force-pushed the prom-select-stats branch from e86f7f5 to 97badc3 Compare November 2, 2022 16:53

dai-chen reviewed Nov 2, 2022

View reviewed changes

penghuo reviewed Nov 2, 2022

View reviewed changes

integ-test/src/test/java/org/opensearch/sql/ppl/PrometheusCatalogCommandsIT.java Show resolved Hide resolved

vamsimanohar commented Nov 2, 2022

View reviewed changes

.../main/java/org/opensearch/sql/prometheus/request/system/PrometheusDescribeMetricRequest.java Outdated Show resolved Hide resolved

vamsimanohar commented Nov 2, 2022

View reviewed changes

integ-test/src/test/java/org/opensearch/sql/ppl/PrometheusCatalogCommandsIT.java Show resolved Hide resolved

penghuo reviewed Nov 2, 2022

View reviewed changes

vamsimanohar force-pushed the prom-select-stats branch 5 times, most recently from ea4a107 to b030c32 Compare November 3, 2022 08:26

Bug Fix

e0d6be4

Signed-off-by: Vamsi Manohar <[email protected]>

vamsimanohar force-pushed the prom-select-stats branch from b030c32 to e0d6be4 Compare November 3, 2022 09:26

penghuo approved these changes Nov 3, 2022

View reviewed changes

ps48 approved these changes Nov 3, 2022

View reviewed changes

vamsimanohar merged commit be4512e into opensearch-project:2.x Nov 3, 2022

opensearch-trigger-bot bot pushed a commit that referenced this pull request Nov 3, 2022

Prometheus select metric and stats queries. (#956)

060ec84

* Prometheus select metric and stats queries. Signed-off-by: reddyvam-amazon <[email protected]> (cherry picked from commit be4512e)

opensearch-trigger-bot bot mentioned this pull request Nov 3, 2022

[Backport 2.4] Prometheus select metric and stats queries. #1020

Merged

vamsimanohar pushed a commit that referenced this pull request Nov 3, 2022

Prometheus select metric and stats queries. (#956) (#1020)

59202f1

* Prometheus select metric and stats queries. Signed-off-by: vamsi-amazon <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prometheus select metric and stats queries. #956

Prometheus select metric and stats queries. #956

vamsimanohar commented Oct 23, 2022 •

edited

Loading

codecov-commenter commented Oct 23, 2022 •

edited by codecov bot

Loading

dai-chen Nov 2, 2022

vamsimanohar Nov 3, 2022

penghuo Nov 2, 2022

vamsimanohar Nov 2, 2022

Prometheus select metric and stats queries. #956

Prometheus select metric and stats queries. #956

Conversation

vamsimanohar commented Oct 23, 2022 • edited Loading

Description

Issues Resolved

Check List

codecov-commenter commented Oct 23, 2022 • edited by codecov bot Loading

Codecov Report

dai-chen Nov 2, 2022

Choose a reason for hiding this comment

vamsimanohar Nov 3, 2022

Choose a reason for hiding this comment

penghuo Nov 2, 2022

Choose a reason for hiding this comment

vamsimanohar Nov 2, 2022

Choose a reason for hiding this comment

vamsimanohar commented Oct 23, 2022 •

edited

Loading

codecov-commenter commented Oct 23, 2022 •

edited by codecov bot

Loading