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

Improve JSON parser and add labels parser hints. #3080

Merged
merged 3 commits into from
Jan 5, 2021

Conversation

cyriltovena
Copy link
Contributor

This is a rework of the json parser, that now uses iteration over reflection allowing to avoid allocations.

I've also implemented a way to guess and hints what labels needs to be parsed when doing metric queries, this means we can extract only the few labels required from the log line.

benchcmp

❯ benchcmp  before.txt after.txt5
benchmark                                      old ns/op     new ns/op     delta
Benchmark_Parser/json/no_labels_hints-16       9889          3281          -66.82%
Benchmark_Parser/logfmt/no_labels_hints-16     1624          1671          +2.89%
Benchmark_Parser/logfmt/labels_hints-16        1601          790           -50.66%
benchmark                                      old allocs     new allocs     delta
Benchmark_Parser/json/no_labels_hints-16       139            56             -59.71%
Benchmark_Parser/logfmt/no_labels_hints-16     31             31             +0.00%
Benchmark_Parser/logfmt/labels_hints-16        31             5              -83.87%
benchmark                                      old bytes     new bytes     delta
Benchmark_Parser/json/no_labels_hints-16       3671          912           -75.16%
Benchmark_Parser/logfmt/no_labels_hints-16     464           464           +0.00%
Benchmark_Parser/logfmt/labels_hints-16        464           144           -68.97%

I've experienced a 2x to 4x improvement in my cluster.

Signed-off-by: Cyril Tovena cyril.tovena@gmail.com

This is a rework of the json parser, that now uses iteration over reflection allowing to avoid allocations.

I've also implemented a way to guess and hints  what labels needs to be parsed when doing metric queries, this means we can extract only the few labels required from the log line.

benchcmp

```
❯ benchcmp  before.txt after.txt5
benchmark                                      old ns/op     new ns/op     delta
Benchmark_Parser/json/no_labels_hints-16       9889          3281          -66.82%
Benchmark_Parser/logfmt/no_labels_hints-16     1624          1671          +2.89%
Benchmark_Parser/logfmt/labels_hints-16        1601          790           -50.66%
benchmark                                      old allocs     new allocs     delta
Benchmark_Parser/json/no_labels_hints-16       139            56             -59.71%
Benchmark_Parser/logfmt/no_labels_hints-16     31             31             +0.00%
Benchmark_Parser/logfmt/labels_hints-16        31             5              -83.87%
benchmark                                      old bytes     new bytes     delta
Benchmark_Parser/json/no_labels_hints-16       3671          912           -75.16%
Benchmark_Parser/logfmt/no_labels_hints-16     464           464           +0.00%
Benchmark_Parser/logfmt/labels_hints-16        464           144           -68.97%
```

I've experienced a 2x to 4x improvement in my cluster.

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

@kavirajk kavirajk left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Copy link
Member

@owen-d owen-d left a comment

Choose a reason for hiding this comment

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

A few comments, but I really like these optimizations.

pkg/logql/log/metrics_extraction.go Show resolved Hide resolved
pkg/logql/log/parser.go Outdated Show resolved Hide resolved
pkg/logql/log/labels.go Outdated Show resolved Hide resolved
pkg/logql/log/util_test.go Show resolved Hide resolved
Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
@codecov-io
Copy link

Codecov Report

Merging #3080 (3713d31) into master (3f99a07) will increase coverage by 0.03%.
The diff coverage is 70.38%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3080      +/-   ##
==========================================
+ Coverage   62.88%   62.91%   +0.03%     
==========================================
  Files         186      187       +1     
  Lines       15949    16089     +140     
==========================================
+ Hits        10030    10123      +93     
- Misses       4987     5029      +42     
- Partials      932      937       +5     
Impacted Files Coverage Δ
pkg/logql/log/label_filter.go 52.22% <0.00%> (-3.18%) ⬇️
pkg/logql/log/filter.go 75.77% <20.00%> (-1.79%) ⬇️
pkg/logql/log/parser.go 73.80% <67.41%> (-17.50%) ⬇️
pkg/logql/log/fmt.go 90.07% <87.50%> (-0.84%) ⬇️
pkg/logql/log/util.go 87.50% <87.50%> (ø)
pkg/logql/log/pipeline.go 87.03% <92.85%> (-0.20%) ⬇️
pkg/logql/log/labels.go 91.17% <100.00%> (+0.13%) ⬆️
pkg/logql/log/metrics_extraction.go 84.04% <100.00%> (+2.33%) ⬆️
pkg/querier/queryrange/downstreamer.go 95.29% <0.00%> (-2.36%) ⬇️
pkg/ingester/checkpoint.go 67.40% <0.00%> (-0.89%) ⬇️
... and 3 more

@owen-d owen-d merged commit 7682f13 into grafana:master Jan 5, 2021
cyriltovena pushed a commit to cyriltovena/loki that referenced this pull request Jun 11, 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

4 participants