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

first_over_time and last_over_time #3050

Merged
merged 4 commits into from
Apr 1, 2021

Conversation

cyriltovena
Copy link
Contributor

This is an attempt to allow users to select the first or last discrete sample values from a range vector.
This way we can create gauge, the only drawback being the log need to exists at least in every range.

May be we could transfer the last or first value to the next range, but it will still be a problem for small ranges.

Still wondering how useful this is compare to min/max_over_time.

WDYT @slim-bean and @chancez you both wanted this feature, I'm still not sure.

Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
This is an attempt to allow users to select the first or last discrete sample values from a range vector.
This way we can create gauge, the only drawback being the log need to exists at least in every range.

May be we could transfer the last or first  value to the next range, but it will still be a problem for small ranges.

Still wondering how useful this is compare to min/max_over_time.

Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
@cyriltovena
Copy link
Contributor Author

@grafana/loki-team WDYT ?

@kavirajk
Copy link
Collaborator

kavirajk commented Dec 7, 2020

May be we could transfer the last or first value to the next range, but it will still be a problem for small ranges.

what kind of problems we talking about for small ranges? you mean in terms of performance?

@cyriltovena
Copy link
Contributor Author

May be we could transfer the last or first value to the next range, but it will still be a problem for small ranges.

what kind of problems we talking about for small ranges? you mean in terms of performance?

the log need to exists at least in every range

If you want to build some sort of gauge with logs, then you need to have the log exist in every range, you're selecting otherwise you'll have gaps if not no value at all.

@slim-bean slim-bean self-assigned this Dec 7, 2020
@owen-d
Copy link
Member

owen-d commented Dec 7, 2020

I'm open to it. Part of me wants us to wait until we see real use case requests for this but my suspicion is this will be a helpful addition.

@chancez
Copy link
Contributor

chancez commented Dec 7, 2020

I don't recall having a use-case for this, but perhaps it was within another related topic.

@stale
Copy link

stale bot commented Jan 9, 2021

This issue has been automatically marked as stale because it has not had any activity in the past 30 days. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale A stale issue or PR that will automatically be closed. label Jan 9, 2021
@cyriltovena
Copy link
Contributor Author

@slim-bean I guess we're going forward with this ? To me it seems to resolve a problem.

@stale stale bot removed the stale A stale issue or PR that will automatically be closed. label Jan 11, 2021
@cyriltovena cyriltovena changed the title [POC] first_over_time and last_over_time first_over_time and last_over_time Jan 13, 2021
@cyriltovena cyriltovena marked this pull request as ready for review January 13, 2021 12:26
@cyriltovena
Copy link
Contributor Author

Alright the use-case we found is to show a gauge based of a log of stream. Currently it's almost possible with max or min but that doesn't give really the last seen value.

@codecov-io
Copy link

Codecov Report

Merging #3050 (1f5c84c) into master (322e4bc) will decrease coverage by 0.03%.
The diff coverage is 66.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3050      +/-   ##
==========================================
- Coverage   63.25%   63.21%   -0.04%     
==========================================
  Files         188      188              
  Lines       16304    16314      +10     
==========================================
  Hits        10313    10313              
- Misses       5051     5062      +11     
+ Partials      940      939       -1     
Impacted Files Coverage Δ
pkg/logql/lex.go 91.07% <ø> (ø)
pkg/logql/functions.go 44.20% <60.00%> (+1.23%) ⬆️
pkg/logql/ast.go 88.12% <100.00%> (ø)
pkg/promtail/targets/file/filetarget.go 62.23% <0.00%> (-4.20%) ⬇️
pkg/canary/comparator/comparator.go 76.36% <0.00%> (-1.82%) ⬇️
pkg/logql/evaluator.go 90.23% <0.00%> (+0.35%) ⬆️
pkg/querier/queryrange/downstreamer.go 97.64% <0.00%> (+2.35%) ⬆️

@stale
Copy link

stale bot commented Feb 13, 2021

This issue has been automatically marked as stale because it has not had any activity in the past 30 days. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale A stale issue or PR that will automatically be closed. label Feb 13, 2021
@chancez
Copy link
Contributor

chancez commented Feb 15, 2021

How does this overlap with the feature of the @ modifier in PromQL? Eg: prometheus/prometheus#8425 adds first/last functions for a time range in a range query. I wonder if that concept could be used for something like this.

@stale stale bot removed the stale A stale issue or PR that will automatically be closed. label Feb 15, 2021
@stale
Copy link

stale bot commented Mar 19, 2021

This issue has been automatically marked as stale because it has not had any activity in the past 30 days. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale A stale issue or PR that will automatically be closed. label Mar 19, 2021
@stale stale bot removed the stale A stale issue or PR that will automatically be closed. label Mar 22, 2021
Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
@cyriltovena cyriltovena merged commit 4d1da2e into grafana:master Apr 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants