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

feat: improve Prometheus compliance #1022

Merged
merged 6 commits into from
Feb 20, 2023

Conversation

waynexia
Copy link
Member

@waynexia waynexia commented Feb 16, 2023

I hereby agree to the terms of the GreptimeDB CLA

What's changed and what's your intention?

  • Support Prometheus's expression query result format.
    (Our metadata are dropped during query execution. We can only infer semantic type from the data type.)
  • add time index column to group by columns
  • filter out NaN in normalize
  • remove NULL in instant manipulator
  • accept form data as HTTP params
  • correct API URL, from v1/range_query to api/v1/query_range
  • accept second literal as step param (like 5)

Checklist

  • I have written the necessary rustdoc comments.
  • I have added the necessary unit tests and integration tests.

Refer to a related PR or issue link (optional)

Closes #1020

Signed-off-by: Ruihang Xia <[email protected]>
Signed-off-by: Ruihang Xia <[email protected]>
Signed-off-by: Ruihang Xia <[email protected]>
@codecov
Copy link

codecov bot commented Feb 16, 2023

Codecov Report

Merging #1022 (79128dd) into develop (a8c2b35) will decrease coverage by 0.41%.
The diff coverage is 53.43%.

@@             Coverage Diff             @@
##           develop    #1022      +/-   ##
===========================================
- Coverage    86.23%   85.82%   -0.41%     
===========================================
  Files          436      441       +5     
  Lines        63279    64376    +1097     
===========================================
+ Hits         54568    55251     +683     
- Misses        8711     9125     +414     
Flag Coverage Δ
rust 85.82% <53.43%> (-0.41%) ⬇️

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

Impacted Files Coverage Δ
src/servers/src/promql.rs 35.31% <18.58%> (-15.40%) ⬇️
src/promql/src/extension_plan/normalize.rs 92.11% <90.90%> (-0.10%) ⬇️
src/promql/src/planner.rs 86.91% <96.77%> (+0.40%) ⬆️
...rc/promql/src/extension_plan/instant_manipulate.rs 95.94% <100.00%> (-0.40%) ⬇️
src/query/src/parser.rs 97.81% <100.00%> (+0.04%) ⬆️
src/object-store/src/test_util.rs 0.00% <0.00%> (-100.00%) ⬇️
src/storage/src/compaction/scheduler.rs 8.82% <0.00%> (-80.18%) ⬇️
src/storage/src/compaction/picker.rs 23.07% <0.00%> (-19.24%) ⬇️
src/datatypes/src/vectors/operations.rs 81.35% <0.00%> (-18.65%) ⬇️
src/common/substrait/src/df_expr.rs 30.81% <0.00%> (-16.56%) ⬇️
... and 84 more

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

filter out NaN in normalize
remove NULL in instant manipulator
accept form data as HTTP params
correct API URL
accept second literal as step param
Signed-off-by: Ruihang Xia <[email protected]>
Copy link
Contributor

@killme2008 killme2008 left a comment

Choose a reason for hiding this comment

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

LGTM

Signed-off-by: Ruihang Xia <[email protected]>
@waynexia waynexia changed the title feat: Prometheus HTTP API result format conversion feat: improve Prometheus compliance Feb 20, 2023
Copy link
Contributor

@evenyag evenyag left a comment

Choose a reason for hiding this comment

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

🙏

@waynexia waynexia enabled auto-merge (squash) February 20, 2023 07:12
@waynexia waynexia merged commit 68b2319 into GreptimeTeam:develop Feb 20, 2023
@waynexia waynexia deleted the prom-api-format branch February 20, 2023 07:30
paomian pushed a commit to paomian/greptimedb that referenced this pull request Oct 19, 2023
* initial impl

Signed-off-by: Ruihang Xia <[email protected]>

* minor (useless) refactor

Signed-off-by: Ruihang Xia <[email protected]>

* retrieve metric name

Signed-off-by: Ruihang Xia <[email protected]>

* add time index column to group by columns
filter out NaN in normalize
remove NULL in instant manipulator
accept form data as HTTP params
correct API URL
accept second literal as step param

* happy clippy

Signed-off-by: Ruihang Xia <[email protected]>

* update test result

Signed-off-by: Ruihang Xia <[email protected]>

---------

Signed-off-by: Ruihang Xia <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RecordBatch to Prometheus JSON format conversion
3 participants