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

Fixes LogQL grouping #2346

Merged
merged 3 commits into from
Jul 13, 2020
Merged

Fixes LogQL grouping #2346

merged 3 commits into from
Jul 13, 2020

Conversation

cyriltovena
Copy link
Contributor

@cyriltovena cyriltovena commented Jul 13, 2020

This fixes #2334

A sort was missing on the group of labels (e.g by(foo,bar,buzz)) causing data to be all over the place. I've added multiple tests reproducing the error and confirmed this is fixed.

Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
@cyriltovena cyriltovena changed the title Fixes Logql groupping Fixes Logql grouping Jul 13, 2020
@cyriltovena cyriltovena changed the title Fixes Logql grouping Fixes LogQL grouping Jul 13, 2020
@codecov-commenter
Copy link

Codecov Report

Merging #2346 into master will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2346      +/-   ##
==========================================
+ Coverage   61.50%   61.52%   +0.01%     
==========================================
  Files         160      160              
  Lines       13422    13423       +1     
==========================================
+ Hits         8255     8258       +3     
+ Misses       4546     4544       -2     
  Partials      621      621              
Impacted Files Coverage Δ
pkg/logql/evaluator.go 92.88% <100.00%> (+0.01%) ⬆️
pkg/querier/queryrange/downstreamer.go 97.93% <0.00%> (+2.06%) ⬆️

@owen-d
Copy link
Member

owen-d commented Jul 13, 2020

nice catch

@owen-d owen-d merged commit ce76ad0 into grafana:master Jul 13, 2020
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.

'sum by' inconsistent when grouped by multiple labels (Loki as Prometheus data source)
3 participants