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

Promtail: Drop stage #2496

Merged
merged 5 commits into from
Aug 13, 2020
Merged

Promtail: Drop stage #2496

merged 5 commits into from
Aug 13, 2020

Conversation

slim-bean
Copy link
Collaborator

The drop stage can be used to drop logs that meet provided criteria, the initial implementation will support dropping logs based on:

  • regex matching content of the log line
  • regex matching content of the extracted map
  • exact string matching of content in the extracte map
  • dropping lines that are older than the current time by a specified amount
  • dropping lines that are longer than a specified byte count.

docs/sources/clients/promtail/stages/drop.md Outdated Show resolved Hide resolved
---
title: drop
---
# `drop` stage
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add a verb here? Or maybe change this to "Filter or drop logs"?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

for now I would like to leave this as is, which is consistent with how we title all our other stages.

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

Complex `drop` stage configurations specify multiple options in one stage or specify multiple drop stages

#### Drop logs by regex AND length
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
#### Drop logs by regex AND length
#### Drop logs by regex and length


Would drop all logs that contain the word _debug_ *AND* are longer than 1024 bytes

#### Drop logs by time OR length OR regex
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
#### Drop logs by time OR length OR regex
#### Drop logs by time or length or regex

Copy link
Contributor

Choose a reason for hiding this comment

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

Do not use capitalization for emphasis. Per the Documentation style guide, use italics for emphasis.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In this case the capitalization is less for emphasis and more to indicate that it's a binary operation, where say in a programming language you might write if x > 1 && x < 5 it's common to express this as if x >1 AND x < 5 mostly to disambiguate the logical operation from the actual word, (AND from and)

regex: ".*trace.*"
```

Would drop all logs older than 24h OR longer than 8192 bytes OR have a json `msg` field containing the word _trace_
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Would drop all logs older than 24h OR longer than 8192 bytes OR have a json `msg` field containing the word _trace_
Would drop all logs older than 24h _or_ longer than 8192 bytes _or_ have a json `msg` field containing the word _trace_.

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.

Test 👮 in duty.

Will this work if promtail timestamps the file rather than extracting the timestamp from the log line? I guess that'd defeat the purpose as they're always time.Now()

Added a few nits, but LGTM after.

pkg/logentry/stages/drop.go Show resolved Hide resolved
pkg/logentry/stages/drop.go Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

codecov-commenter commented Aug 13, 2020

Codecov Report

Merging #2496 into master will increase coverage by 0.09%.
The diff coverage is 85.50%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2496      +/-   ##
==========================================
+ Coverage   63.29%   63.39%   +0.09%     
==========================================
  Files         162      163       +1     
  Lines       14114    14246     +132     
==========================================
+ Hits         8934     9031      +97     
- Misses       4470     4504      +34     
- Partials      710      711       +1     
Impacted Files Coverage Δ
pkg/logentry/stages/stage.go 47.54% <50.00%> (+0.17%) ⬆️
pkg/logentry/stages/pipeline.go 75.00% <75.00%> (-0.41%) ⬇️
pkg/logentry/stages/match.go 90.62% <77.77%> (-2.71%) ⬇️
pkg/logentry/stages/drop.go 88.99% <88.99%> (ø)
pkg/promtail/positions/positions.go 46.49% <0.00%> (-13.16%) ⬇️
pkg/canary/comparator/comparator.go 78.43% <0.00%> (-2.36%) ⬇️
pkg/logql/evaluator.go 92.88% <0.00%> (+0.40%) ⬆️
pkg/logentry/stages/timestamp.go 89.87% <0.00%> (+2.53%) ⬆️
pkg/logentry/stages/output.go 81.81% <0.00%> (+6.06%) ⬆️

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.

Great tests, tyvm.

@slim-bean slim-bean dismissed oddlittlebird’s stale review August 13, 2020 15:29

Comments to requested changes were added, apologies for dismissing the review but we are on the clock to get a release out.

@slim-bean slim-bean merged commit e69b64f into master Aug 13, 2020
@slim-bean slim-bean deleted the drop-stage branch August 13, 2020 15:29
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