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: adding pipeline stage for dropping labels #2571

Merged
merged 4 commits into from
Sep 10, 2020

Conversation

rsteneteg
Copy link
Contributor

Adding a pipeline stage in Promtail to drop labels

Signed-off-by: Roger Steneteg rsteneteg@ea.com

What this PR does / why we need it:
Adding a pipeline stage to drop labels, our use case for this is to move unbound labels into the log line and dropping the label

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

Special notes for your reviewer:

Checklist

  • Documentation added
  • Tests updated

…o move unbound labels into the log line and dropping the label, fixes grafana#2421

Signed-off-by: Roger Steneteg <rsteneteg@ea.com>
@codecov-commenter
Copy link

codecov-commenter commented Aug 28, 2020

Codecov Report

Merging #2571 into master will decrease coverage by 0.04%.
The diff coverage is 50.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2571      +/-   ##
==========================================
- Coverage   62.85%   62.81%   -0.05%     
==========================================
  Files         169      170       +1     
  Lines       14913    14937      +24     
==========================================
+ Hits         9374     9383       +9     
- Misses       4790     4804      +14     
- Partials      749      750       +1     
Impacted Files Coverage Δ
pkg/logentry/stages/stage.go 44.61% <0.00%> (-2.93%) ⬇️
pkg/logentry/stages/labeldrop.go 60.00% <60.00%> (ø)
pkg/querier/queryrange/downstreamer.go 95.87% <0.00%> (-2.07%) ⬇️
pkg/promtail/targets/file/filetarget.go 65.14% <0.00%> (-1.72%) ⬇️
pkg/logql/evaluator.go 92.88% <0.00%> (+0.40%) ⬆️

rsteneteg and others added 2 commits August 28, 2020 14:47
Co-authored-by: Diana Payton <52059945+oddlittlebird@users.noreply.github.com>
Co-authored-by: Diana Payton <52059945+oddlittlebird@users.noreply.github.com>
@slim-bean
Copy link
Collaborator

This looks really good btw, almost had a chance to get to review this today but it might have to wait until next week we have a lot of folks on PTO. Thank you though and we will get this merged soon!! ❤️


const (
// ErrEmptyLabelDropStageConfig error returned if config is empty
ErrEmptyLabelDropStageConfig = "drop_label stage config cannot be empty"
Copy link
Contributor

Choose a reason for hiding this comment

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

drop_label and labeldrop ? There is some inconsistency here. Should we rename this to drop_labels everywhere ? WDYT ? naming is hard but also important.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks for finding that, I switched that out to labeldrop, I picked "labeldrop" over "drop_label" because the relabel action to drop labels is called labeldrop so thought it would be more consistent.

Copy link
Contributor

@cyriltovena cyriltovena left a comment

Choose a reason for hiding this comment

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

Looks good to me, I'd like @slim-bean to chime in for the name of the stage.

Signed-off-by: Roger Steneteg <rsteneteg@ea.com>
Copy link
Contributor

@cyriltovena cyriltovena left a comment

Choose a reason for hiding this comment

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

LGTM, if everyone is ok with the name let's merge this one.

@slim-bean
Copy link
Collaborator

labeldrop is consistent with the action of the Prometheus relabel config so this seems good to me

@cyriltovena cyriltovena merged commit edb5fc5 into grafana:master Sep 10, 2020
cyriltovena pushed a commit to cyriltovena/loki that referenced this pull request Jun 11, 2021
* Translate internal store errors to prometheus API errors.

Signed-off-by: Peter Štibraný <peter.stibrany@grafana.com>

* Added integration test for chunks querier and query-frontend.

Signed-off-by: Peter Štibraný <peter.stibrany@grafana.com>

* Account for one extra query.

Signed-off-by: Peter Štibraný <peter.stibrany@grafana.com>

* Fix test after rebase.

Signed-off-by: Peter Štibraný <peter.stibrany@grafana.com>
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.

Pipeline stages should support dropping labels
5 participants