Promtail: Add compressed files support #6708

DylanGuedes · 2022-07-18T15:20:57Z

What this PR does / why we need it:
Adds to Promtail the ability to read compressed files. It works by:

Infer which compression format to use based on the file extension
Uncompress the file with the native golang/compress packages
Iterate over uncompressed lines and send them to Loki

Its usage is the same as our current file tailing. In the example below all files under logfiles folder are parsed/processed, regardless of being compressed or not. file1 and file2 are first uncompressed with Gunzip; file3 is parsed as it is.

scrape_configs:
  - job_name: simplejob
    static_configs:
      - targets: [localhost]
        labels:
          job: compressedfolder
          __path__: /logfiles/**.*

$ ls /logfiles
     file1.tar.gz file2.tar.gz file3.txt

Which issue(s) this PR fixes:
Fixes #5956

Special notes for your reviewer:
Another approach is to infer the compression format based on the header bytes but this approach is much simpler; in the future we can improve it if we think the community needs it.

Checklist

Documentation added
Tests updated
Is this an important fix or new feature? Add an entry in the CHANGELOG.md.
Changes that require user attention or interaction to upgrade are documented in docs/sources/upgrading/_index.md

- The new Reader interface is used across source code instead of the `tailer` - `tailer` is an implementation of Reader

- Decompresser implements the file.Reader interface - Decompresser infers which protocol to use to unzip file based on its extension

grafanabot · 2022-07-19T14:09:21Z

./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki

Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell.

+           ingester	0%
+        distributor	0%
+            querier	0%
+ querier/queryrange	0%
+               iter	0%
+            storage	0%
+           chunkenc	0%
+              logql	0%
+               loki	0%

dannykopping

Overall looks awesome @DylanGuedes!
I'm a bit concerned by the potential here for resource overuse by reading the whole decompressed file into memory, from what I can see.

clients/pkg/promtail/targets/file/decompresser.go

dannykopping · 2022-07-22T07:15:28Z

clients/pkg/promtail/targets/file/decompresser.go

+// It first decompress the file as a whole using a reader and then it will iterate
+// over its chunks, separated by '\n'.
+// During each iteration, the parsed and decoded log line is then sent to the API with the current timestamp.


This is what I was kinda worried about seeing...
If the file is compressed, it could potentially store a huge amount of data which we need to entirely load into memory to iterate over.

Is there a more resource-efficient way we could do this?

Totally @dannykopping ... compression ratios >99% are common when thinking about uniformly structured logs like access or event logs. Reading a whole file into memory would totally not be an option. When suggesting implementing this along the lines of piping data via zcat into an existing tool (#5956 (comment)). And isn't that exactly what the mountReader method does? It gets the right type of reader (which is able to read a compressed data stream of various algorithms) and then hands that off to be read from line by line. But there is not slurping into memory, is there?

Great observation, thanks. I'm relying on whatever the underlying readers are doing; I'm not sure if all of them (like flatten/zip) supports doing inline uncompression without buffering, but I'm not either explicitly decompressing things as a whole with something like tar -xzvf myfile.tar.gz, so it might be supported. I'm planning on doing a few tests with this this week, one of them will be to uncompressed big files and another one will be to rotate files to see how it behaves.

We tested this together, and found that it is indeed allocating a lot of memory but the GC acts as it should, and the memory doesn't grow unbounded. Dylan also verified that the files are decompressed progressively, so at no time do we try to load the whole file into memory at once, so the overall memory usage will not spike very high (we use small buffers when reading the compressed files). We should add a note in the docs to specify that GC frequency will increase dramatically when decompressing large files, and this will cause CPU usage to rise.

clients/pkg/promtail/targets/file/decompresser.go

clients/pkg/promtail/targets/file/filetarget.go

frittentheke · 2022-07-22T21:28:27Z

clients/pkg/promtail/targets/file/decompresser.go

+// It first decompress the file as a whole using a reader and then it will iterate
+// over its chunks, separated by '\n'.
+// During each iteration, the parsed and decoded log line is then sent to the API with the current timestamp.


Totally @dannykopping ... compression ratios >99% are common when thinking about uniformly structured logs like access or event logs. Reading a whole file into memory would totally not be an option. When suggesting implementing this along the lines of piping data via zcat into an existing tool (#5956 (comment)). And isn't that exactly what the mountReader method does? It gets the right type of reader (which is able to read a compressed data stream of various algorithms) and then hands that off to be read from line by line. But there is not slurping into memory, is there?

clients/pkg/promtail/targets/file/decompresser.go

DylanGuedes · 2022-07-25T13:03:19Z

@dannykopping @frittentheke thank you both for the fantastic reviews, I'll be working on those suggestions and other ones that might come along the week and I plan to try testing a few things with this too.

Just one thing, I think even if this doesn't add decent support for compressed log rotations, we should still proceed to at least have this, as a simpler implementation that is to be improved in a follow-up PR (I can start working on it exactly after this gets merged). My argument is that as of now Loki isn't supporting even the most straightforward compression scenarios, so this would at least add some support without a lot of burdens.
For instance, I liked that with this PR you can now tell Promtail to scrape from a folder that has tons of compressed files and that you can keep appending new compressed files to it and all of them will be scraped.

…ed-support

frittentheke · 2022-07-25T13:23:34Z

For instance, I liked that with this PR you can now tell Promtail to scrape from a folder that has tons of compressed files and that you can keep appending new compressed files to it and all of them will be scraped.

Yes. Question is though, in what order those files will be picked up? Does Promtail sort them by modification date? That would then at least keep as much order as possible.

grafanabot · 2022-07-25T13:27:08Z

./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki

Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell.

+           ingester	0%
+        distributor	0%
+            querier	0%
+ querier/queryrange	0%
+               iter	0%
+            storage	0%
+           chunkenc	0%
+              logql	0%
+               loki	0%

- It tells that the file was compressed when it actually just mounted its reader - Add file extension to it

- Change message log level from Info to Debug - Enhance grammar of log message (with -> has) Co-authored-by: Danny Kopping <dannykopping@gmail.com>

grafanabot · 2022-07-25T13:38:46Z

./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki

Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell.

+           ingester	0%
+        distributor	0%
+            querier	0%
+ querier/queryrange	0%
+               iter	0%
+            storage	0%
+           chunkenc	0%
+              logql	0%
+               loki	0%

grafanabot · 2022-07-25T13:46:30Z

./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki

Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell.

+           ingester	0%
+        distributor	0%
+            querier	0%
+ querier/queryrange	0%
+               iter	0%
+            storage	0%
+           chunkenc	0%
+              logql	0%
+               loki	0%

…ed-support

grafanabot · 2022-09-02T20:23:28Z

./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki

Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell.

+           ingester	0%
+        distributor	0%
+            querier	0%
+ querier/queryrange	0%
+               iter	0%
+            storage	0%
+           chunkenc	0%
+              logql	0.4%
+               loki	0%

grafanabot · 2022-09-19T13:48:48Z

./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki

Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell.

+           ingester	0%
+        distributor	0%
+            querier	0%
+ querier/queryrange	0%
+               iter	0%
+            storage	0%
+           chunkenc	0%
+              logql	0%
+               loki	0%

grafanabot · 2022-09-21T00:17:56Z

./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki

Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell.

+           ingester	0%
+        distributor	0%
+            querier	0%
+ querier/queryrange	0%
+               iter	0%
+            storage	0%
+           chunkenc	0%
+              logql	0%
+               loki	0%

grafanabot · 2022-09-21T10:28:06Z

./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki

Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell.

+           ingester	0%
+        distributor	0%
+            querier	0%
+ querier/queryrange	0%
+               iter	0%
+            storage	0%
+           chunkenc	0%
+              logql	0%
+               loki	0%

grafanabot · 2022-09-21T12:16:39Z

./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki

Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell.

+           ingester	0%
+        distributor	0%
+            querier	0%
+ querier/queryrange	0%
+               iter	0%
+            storage	0%
+           chunkenc	0%
+              logql	0%
+               loki	0%

dannykopping

LGTM, some small doc changes and I think we should change that buffer size - but after that, merge at will!

dannykopping · 2022-09-21T14:22:17Z

clients/pkg/promtail/targets/file/decompresser.go

+// It first decompress the file as a whole using a reader and then it will iterate
+// over its chunks, separated by '\n'.
+// During each iteration, the parsed and decoded log line is then sent to the API with the current timestamp.


We tested this together, and found that it is indeed allocating a lot of memory but the GC acts as it should, and the memory doesn't grow unbounded. Dylan also verified that the files are decompressed progressively, so at no time do we try to load the whole file into memory at once, so the overall memory usage will not spike very high (we use small buffers when reading the compressed files). We should add a note in the docs to specify that GC frequency will increase dramatically when decompressing large files, and this will cause CPU usage to rise.

clients/pkg/promtail/targets/file/decompresser.go

docs/sources/clients/promtail/_index.md

clients/pkg/promtail/targets/file/decompresser_test.go

Co-authored-by: Danny Kopping <dannykopping@gmail.com>

grafanabot · 2022-09-27T13:10:17Z

./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki

Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell.

+           ingester	0%
+        distributor	0%
+            querier	0%
+ querier/queryrange	0%
+               iter	0%
+            storage	0%
+           chunkenc	0%
+              logql	0%
+               loki	0%

DylanGuedes · 2022-09-27T13:15:09Z

Benchmark results with 3MB buffer size for decompression:

Running tool: /usr/local/go/bin/go test -benchmem -run=^$ -tags integration,requires_docker -bench ^BenchmarkReadlines$ github.com/grafana/loki/clients/pkg/promtail/targets/file

goos: linux
goarch: amd64
pkg: github.com/grafana/loki/clients/pkg/promtail/targets/file
cpu: Intel(R) Core(TM) i7-10750H CPU @ 2.60GHz
BenchmarkReadlines/2000_lines_of_log_.tar.gz_compressed-12         	     768	   1559020 ns/op	 3063606 B/op	     239 allocs/op
BenchmarkReadlines/100000_lines_of_log_.gz_compressed-12           	      18	  63992058 ns/op	 5666200 B/op	   21157 allocs/op
PASS
ok  	github.com/grafana/loki/clients/pkg/promtail/targets/file	3.221s

DylanGuedes · 2022-09-27T13:16:30Z

Benchmark results with 4096 buffer size:

Running tool: /usr/local/go/bin/go test -benchmem -run=^$ -tags integration,requires_docker -bench ^BenchmarkReadlines$ github.com/grafana/loki/clients/pkg/promtail/targets/file

goos: linux
goarch: amd64
pkg: github.com/grafana/loki/clients/pkg/promtail/targets/file
cpu: Intel(R) Core(TM) i7-10750H CPU @ 2.60GHz
BenchmarkReadlines/2000_lines_of_log_.tar.gz_compressed-12         	     933	   1261038 ns/op	   60287 B/op	     236 allocs/op
BenchmarkReadlines/100000_lines_of_log_.gz_compressed-12           	      19	  58614711 ns/op	 2536763 B/op	   20279 allocs/op
PASS
ok  	github.com/grafana/loki/clients/pkg/promtail/targets/file	3.167s

DylanGuedes · 2022-09-27T14:53:17Z

@dannykopping Merging as I addressed all your great suggestions. I'll be working on the zip compression on a follow-up PR 😄

**What this PR does / why we need it**: Adds to Promtail the ability to read compressed files. It works by: 1. Infer which compression format to use based on the file extension 2. Uncompress the file with the native `golang/compress` packages 3. Iterate over uncompressed lines and send them to Loki Its usage is the same as our current file tailing. **Which issue(s) this PR fixes**: Fixes grafana#5956 Co-authored-by: Danny Kopping <dannykopping@gmail.com>

DylanGuedes requested a review from a team as a code owner July 18, 2022 15:20

pull-request-size bot added the size/L label Jul 18, 2022

DylanGuedes changed the title ~~Add compressed files support for Promtail~~ Add compressed files support to Promtail Jul 18, 2022

DylanGuedes changed the title ~~Add compressed files support to Promtail~~ Promtail: Add compressed files support to Promtail Jul 18, 2022

DylanGuedes changed the title ~~Promtail: Add compressed files support to Promtail~~ Promtail: Add compressed files support Jul 18, 2022

DylanGuedes force-pushed the promtail-compressed-support branch 3 times, most recently from eeffa05 to d18a0a1 Compare July 18, 2022 18:09

DylanGuedes added 5 commits July 19, 2022 10:19

Implement new Reader interface.

3c8ad6a

- The new Reader interface is used across source code instead of the `tailer` - `tailer` is an implementation of Reader

Implement Decompresser.

01800fd

- Decompresser implements the file.Reader interface - Decompresser infers which protocol to use to unzip file based on its extension

Add relevant docstrings.

9bffe70

Fix positions state reuse.

4cf7c1c

Add CHANGELOG entry.

973d66f

DylanGuedes force-pushed the promtail-compressed-support branch from d18a0a1 to fd6162d Compare July 19, 2022 13:20

Fix calls to .tailers to be .readers instead.

f94e7e2

DylanGuedes force-pushed the promtail-compressed-support branch from fd6162d to f94e7e2 Compare July 19, 2022 13:44

dannykopping reviewed Jul 22, 2022

View reviewed changes

frittentheke reviewed Jul 22, 2022

View reviewed changes

Merge branch 'main' of github.com:grafana/loki into promtail-compress…

86107fe

…ed-support

Rename decompresser as decompressor.

c6ec76e

DylanGuedes and others added 2 commits July 25, 2022 10:28

Rephrase misreading decompression log message.

58b5634

- It tells that the file was compressed when it actually just mounted its reader - Add file extension to it

Apply suggestions from code review

77d7fe8

- Change message log level from Info to Debug - Enhance grammar of log message (with -> has) Co-authored-by: Danny Kopping <dannykopping@gmail.com>

Add to error text all supported extensions.

e80a2f8

Doesn't ignore scanning errors != io.EOF.

1168d7a

DylanGuedes added 4 commits September 2, 2022 16:06

Merge branch 'main' of github.com:grafana/loki into promtail-compress…

794dc33

…ed-support

Remove dupped changelog lines.

d99b09e

Remove dupped log line.

a2c86db

Return pointer for noopclient.

a312cb5

Start counting lines at 1.

7f37bea

Drop support for .zip

fd1041c

pull-request-size bot added size/XXL and removed size/XL labels Sep 21, 2022

Add docs section.

edc3b01

github-actions bot added the type/docs Issues related to technical documentation; the Docs Squad uses this label across many repositories label Sep 21, 2022

Test correctness of the gunzip reader.

ba0483e

dannykopping approved these changes Sep 21, 2022

View reviewed changes

DylanGuedes and others added 2 commits September 27, 2022 10:04

Apply suggestions from code review

c49af30

Co-authored-by: Danny Kopping <dannykopping@gmail.com>

Link Loki limits.

30b6934

DylanGuedes added 2 commits September 27, 2022 10:10

Explain why Promtail doesn't support log rotations.

7621ef9

Explain why test case scenario is necessary.

fd71e02

DylanGuedes added 2 commits September 27, 2022 10:17

Add TODOs for .zip file.

dc57333

Add clarification to decompressing process.

84bfeea

DylanGuedes merged commit 73bea7e into grafana:main Sep 27, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Promtail: Add compressed files support #6708

Promtail: Add compressed files support #6708

DylanGuedes commented Jul 18, 2022 •

edited

Loading

grafanabot commented Jul 19, 2022

dannykopping left a comment

dannykopping Jul 22, 2022

frittentheke Jul 22, 2022

DylanGuedes Jul 25, 2022

dannykopping Sep 21, 2022

frittentheke Jul 22, 2022

DylanGuedes commented Jul 25, 2022

frittentheke commented Jul 25, 2022

grafanabot commented Jul 25, 2022

grafanabot commented Jul 25, 2022

grafanabot commented Jul 25, 2022

grafanabot commented Sep 2, 2022

grafanabot commented Sep 19, 2022

grafanabot commented Sep 21, 2022

grafanabot commented Sep 21, 2022

grafanabot commented Sep 21, 2022

dannykopping left a comment

dannykopping Sep 21, 2022

grafanabot commented Sep 27, 2022

DylanGuedes commented Sep 27, 2022

DylanGuedes commented Sep 27, 2022

DylanGuedes commented Sep 27, 2022

Promtail: Add compressed files support #6708

Promtail: Add compressed files support #6708

Conversation

DylanGuedes commented Jul 18, 2022 • edited Loading

grafanabot commented Jul 19, 2022

dannykopping left a comment

Choose a reason for hiding this comment

dannykopping Jul 22, 2022

Choose a reason for hiding this comment

frittentheke Jul 22, 2022

Choose a reason for hiding this comment

DylanGuedes Jul 25, 2022

Choose a reason for hiding this comment

dannykopping Sep 21, 2022

Choose a reason for hiding this comment

frittentheke Jul 22, 2022

Choose a reason for hiding this comment

DylanGuedes commented Jul 25, 2022

frittentheke commented Jul 25, 2022

grafanabot commented Jul 25, 2022

grafanabot commented Jul 25, 2022

grafanabot commented Jul 25, 2022

grafanabot commented Sep 2, 2022

grafanabot commented Sep 19, 2022

grafanabot commented Sep 21, 2022

grafanabot commented Sep 21, 2022

grafanabot commented Sep 21, 2022

dannykopping left a comment

Choose a reason for hiding this comment

dannykopping Sep 21, 2022

Choose a reason for hiding this comment

grafanabot commented Sep 27, 2022

DylanGuedes commented Sep 27, 2022

DylanGuedes commented Sep 27, 2022

DylanGuedes commented Sep 27, 2022

DylanGuedes commented Jul 18, 2022 •

edited

Loading