[Promtail] drop lines with malformed json #6099

cstyan · 2022-05-04T23:53:05Z

In this PR I've added a new config field to the JSON pipeline stage which allows for dropping of log lines that cannot properly be unmarshaled to JSON objects (malformed json or just not json at all). In the future we could extend this to other log formats with new pipeline stages such as logfmt.

The commits are relatively standalone, first I refactored the json pipeline stage to match the new(ish) pipeline stage interface with a Run function, then implemented the new drop malformed json option.

Signed-off-by: Callum Styan <[email protected]>

valid json strings Signed-off-by: Callum Styan <[email protected]>

Signed-off-by: Callum Styan <[email protected]>

grafanabot · 2022-05-05T18:52:45Z

./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%

Signed-off-by: Callum Styan <[email protected]>

grafanabot · 2022-05-17T00:17:09Z

./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%

clients/pkg/logentry/stages/json.go

clients/pkg/logentry/stages/json_test.go

owen-d

Approving, but please address @chaudum's feedback. Nice work.

vlad-diachenko

Looks great! also, I think that @chaudum 's suggestion to use testify lib for assertions will simplify the test for reading...

clients/pkg/logentry/stages/json_test.go

Signed-off-by: Callum Styan <[email protected]>

t.Errorf Signed-off-by: Callum Styan <[email protected]>

cstyan · 2022-05-19T19:11:20Z

Thanks for the reviews guys! I fixed some of the comments in one commit and included a second that refactors all of the json_test.go tests to use testify assert consistently.

grafanabot · 2022-05-19T19:59:41Z

./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.3%
+            querier	0%
+ querier/queryrange	0%
+               iter	0%
+            storage	0%
+           chunkenc	0%
+              logql	0%
+               loki	0%

grafanabot · 2022-05-19T20:05:45Z

./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%

Signed-off-by: Callum Styan <[email protected]>

grafanabot · 2022-05-20T19:23:03Z

./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%

KMiller-Grafana

I've left comments/suggestions, but am approving this PR such that you don't need a re-review from me.

CHANGELOG.md

docs/sources/clients/promtail/stages/json.md

Co-authored-by: Karen Miller <[email protected]>

grafanabot · 2022-05-24T15:28: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.3%
+            querier	0%
+ querier/queryrange	0%
+               iter	0%
+            storage	0%
+           chunkenc	0%
+              logql	0%
+               loki	0%

Signed-off-by: Callum Styan <[email protected]>

grafanabot · 2022-05-24T18:09: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-05-24T20:46: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%

pull-request-size bot added the size/M label May 4, 2022

cstyan added 3 commits May 5, 2022 09:24

Refactor json pipeline stage to have a Run function

2b7f02b

Signed-off-by: Callum Styan <[email protected]>

Allow for dropping log lines in json pipeline stage if they are not

91a4739

valid json strings Signed-off-by: Callum Styan <[email protected]>

fix linting

5e439c6

Signed-off-by: Callum Styan <[email protected]>

Clean up new malformed json test.

0fb2149

Signed-off-by: Callum Styan <[email protected]>

cstyan marked this pull request as ready for review May 17, 2022 03:33

cstyan requested a review from a team as a code owner May 17, 2022 03:33

chaudum reviewed May 17, 2022

View reviewed changes

owen-d approved these changes May 18, 2022

View reviewed changes

vlad-diachenko approved these changes May 18, 2022

View reviewed changes

vlad-diachenko reviewed May 18, 2022

View reviewed changes

clients/pkg/logentry/stages/json_test.go Outdated Show resolved Hide resolved

cstyan added 2 commits May 19, 2022 11:13

Address some review feedback.

63082b8

Signed-off-by: Callum Styan <[email protected]>

Refactor json pipeline stage test to use testify assert instead of

7bf7309

t.Errorf Signed-off-by: Callum Styan <[email protected]>

Update changelog and docs with new drop malformed option

80dd811

Signed-off-by: Callum Styan <[email protected]>

cstyan requested a review from KMiller-Grafana as a code owner May 20, 2022 19:18

pull-request-size bot added size/L and removed size/M labels May 20, 2022

github-actions bot added the area/docs label May 20, 2022

KMiller-Grafana approved these changes May 20, 2022

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

docs/sources/clients/promtail/stages/json.md Outdated Show resolved Hide resolved

chaudum approved these changes May 23, 2022

View reviewed changes

Update CHANGELOG.md

12f4089

Co-authored-by: Karen Miller <[email protected]>

cstyan added 2 commits May 24, 2022 08:41

Update docs based on Karen's comments.

22eaf46

Signed-off-by: Callum Styan <[email protected]>

Merge branch 'main' into promtail-malformed-json

c317829

slim-bean merged commit b6ac00e into grafana:main May 24, 2022

This was referenced Jun 28, 2022

dannykopping/remove cache stats dannykopping/loki#13

Closed

dannykopping/remove cache stats dannykopping/loki#14

Draft

osg-grafana added type/docs Issues related to technical documentation; the Docs Squad uses this label across many repositories and removed area/docs labels Oct 19, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Promtail] drop lines with malformed json #6099

[Promtail] drop lines with malformed json #6099

cstyan commented May 4, 2022 •

edited

Loading

grafanabot commented May 5, 2022

grafanabot commented May 17, 2022

owen-d left a comment

vlad-diachenko left a comment

cstyan commented May 19, 2022

grafanabot commented May 19, 2022

grafanabot commented May 19, 2022

grafanabot commented May 20, 2022

KMiller-Grafana left a comment

grafanabot commented May 24, 2022

grafanabot commented May 24, 2022

grafanabot commented May 24, 2022

[Promtail] drop lines with malformed json #6099

[Promtail] drop lines with malformed json #6099

Conversation

cstyan commented May 4, 2022 • edited Loading

grafanabot commented May 5, 2022

grafanabot commented May 17, 2022

owen-d left a comment

Choose a reason for hiding this comment

vlad-diachenko left a comment

Choose a reason for hiding this comment

cstyan commented May 19, 2022

grafanabot commented May 19, 2022

grafanabot commented May 19, 2022

grafanabot commented May 20, 2022

KMiller-Grafana left a comment

Choose a reason for hiding this comment

grafanabot commented May 24, 2022

grafanabot commented May 24, 2022

grafanabot commented May 24, 2022

cstyan commented May 4, 2022 •

edited

Loading