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

Add support for histogram quantiles #1533

Merged
merged 22 commits into from
Jan 10, 2023
Merged

Add support for histogram quantiles #1533

merged 22 commits into from
Jan 10, 2023

Conversation

srikanthccv
Copy link
Member

@srikanthccv srikanthccv commented Aug 24, 2022

Fixes https://github.com/SigNoz/engineering-pod/issues/618

This commit adds support for plotting/calculating the percentile based on the histogram buckets data.

There are two custom functions added

  • histogramQuantile

histogramQuantile is enabled by adding a custom executable function in the ClickHouse using UDF. The histogramQuantile takes bucket bounds, cumulative counts and quantile as args and returns the float value. The sample usage would be like the following.

c6766f6fb6fa :) select histogramQuantile([1, 2, 4, 8, 16, 32, 64, inf], [1, 10, 12, 15, 18, 20, 25, 25], 0.9);

SELECT histogramQuantile([1, 2, 4, 8, 16, 32, 64, inf], [1, 10, 12, 15, 18, 20, 25, 25], 0.9)

Query id: c8e71e09-5f5c-4732-915a-9b2b24f9f4e1

┌─histogramQuantile([1, 2, 4, 8, 16, 32, 64, inf], [1, 10, 12, 15, 18, 20, 25, 25], 0.9)─┐
│                                                                                     48 │
└────────────────────────────────────────────────────────────────────────────────────────┘

1 rows in set. Elapsed: 0.005 sec.

The executable should be compiled for the target arch to get it to work without any issues.

@request-info
Copy link

request-info bot commented Aug 24, 2022

We would appreciate it if you could provide us with more info about this issue/pr!

@sonarcloud
Copy link

sonarcloud bot commented Aug 25, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 3 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@github-actions
Copy link

Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id>

1 similar comment
@github-actions
Copy link

Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id>

palashgdev
palashgdev previously approved these changes Aug 30, 2022
frontend/src/types/common/dashboard.ts Show resolved Hide resolved
@ankitnayan
Copy link
Collaborator

@srikanthccv we should check the performance comparison of the new implemented running difference vs the native

@github-actions
Copy link

github-actions bot commented Jan 4, 2023

Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id>

@github-actions
Copy link

github-actions bot commented Jan 4, 2023

Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id>

@github-actions
Copy link

github-actions bot commented Jan 6, 2023

Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id>

1 similar comment
@github-actions
Copy link

github-actions bot commented Jan 6, 2023

Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id>

@srikanthccv
Copy link
Member Author

srikanthccv commented Jan 6, 2023

Here is an example of P99 for the hotrod services using this.

PromQL vs Query Builder
Screenshot 2023-01-07 at 7 37 56 PM

@ankitnayan ankitnayan requested review from makeavish and removed request for ankitnayan and pranshuchittora January 8, 2023 19:46
@github-actions
Copy link

github-actions bot commented Jan 9, 2023

Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id>

@srikanthccv
Copy link
Member Author

@prashant-shahi I was thinking we will maintain a separate repo signoz-udfs and use init containers to pull those changes to make them available for UDF. What do you think?

@ankitnayan
Copy link
Collaborator

ankitnayan commented Jan 9, 2023

@prashant-shahi I was thinking we will maintain a separate repo signoz-udfs and use init containers to pull those changes to make them available for UDF. What do you think?

How will this work for those who use external clickhouse clusters? The init containers should make the udfs available on hosted or external clickhouse clusters, right?

@github-actions
Copy link

Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id>

@ankitnayan ankitnayan self-requested a review January 10, 2023 07:26
@ankitnayan
Copy link
Collaborator

LGTM but more eyes can be more helpful. Feel free to merge if you are confident

@ankitnayan
Copy link
Collaborator

Here is an example of P99 for the hotrod services using this.

@srikanthccv I see some minor differences in the chart. Though they seem to be negligible, could you figure out any specific reason for that? We can keep a check on it

@srikanthccv
Copy link
Member Author

The smoothness of the curve in the Prometheus chart varies depending on the time range [x], which is customisable and usually 5min. In our case, the runningDifference works on the subsequent points giving slightly different smoothness.

@srikanthccv
Copy link
Member Author

Adding the [2m] chart, which is appropriate equivalent

Screenshot 2023-01-10 at 2 25 19 PM

@ankitnayan
Copy link
Collaborator

Adding the [2m] chart, which is appropriate equivalent

Screenshot 2023-01-10 at 2 25 19 PM

Almost perfect. Nice work

@github-actions
Copy link

Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id>

@sonarcloud
Copy link

sonarcloud bot commented Jan 10, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 3 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

Copy link
Member

@prashant-shahi prashant-shahi left a comment

Choose a reason for hiding this comment

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

:shipit: LGTM

Looks good one level down to operations as well:
image

@srikanthccv srikanthccv merged commit 44360ec into develop Jan 10, 2023
@srikanthccv srikanthccv deleted the issue-618 branch January 10, 2023 16:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants