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

fix(query): Fix histogram query rewrite to exclude summaries #1887

Merged
merged 1 commit into from
Nov 13, 2024

Conversation

vishramachandran
Copy link
Member

Pull Request checklist

  • The commit(s) message(s) follows the contribution guidelines ?
  • Tests for the changes have been added (for bug fixes / features) ?
  • Docs have been added / updated (for bug fixes / features) ?

Today we ingest summaries are three or more time series. For example, for a summary named foo:

  • One or more quantiles can be reported as time series with metric name foo. Today, this is ingested as a gauge in FiloDB.
  • We have a sum reported at foo_sum. This is a counter.
  • We have a foo_count. This is also a counter.
    Details in the original prom doc

Problem:
When foo_count is queried, we lookup to see if metric name foo exists. If it exists, we simply rewrite the query to foo::count which ends up querying the count column on foo. This is for backward compatibility on prometheus histograms.

In the case of summaries, the count column does not exist on foo and it leads to query failure.

Fix:
Rewrite the query only if the second lookup exists, and additionally has a column called count. This additional check will be true for histograms only.

Testing:
Existing unit tests in MultiSchemaPartitionsExecSpec cover this case.

Copy link
Contributor

@sandeep6189 sandeep6189 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 taking this up @vishramachandran

@vishramachandran vishramachandran merged commit e20b7dd into filodb:develop Nov 13, 2024
1 check passed
@vishramachandran vishramachandran deleted the summary-query-bug branch November 13, 2024 19:55
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.

2 participants