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: Remove promtail_log_entries_bytes_bucket histogram #5377

Merged
merged 1 commit into from
Feb 17, 2022

Conversation

slim-bean
Copy link
Collaborator

@slim-bean slim-bean commented Feb 11, 2022

What this PR does / why we need it:

This histogram reports a distribution of the size (bytes) of every log line processed by promtail, and does so with a label for each file being tailed. There are 8 buckets in the histogram which can quickly lead to a huge amount of exported metrics.

In our experiences, we have found little use for this metric, not enough to justify the cost of the number of series it creates so this PR removes it.

Checklist

  • Documentation added
  • Tests updated
  • Add an entry in the CHANGELOG.md about the changes.

Copy link
Contributor

@dannykopping dannykopping left a comment

Choose a reason for hiding this comment

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

Overall LGTM, but I wonder if we shouldn't follow a "first-deprecate-then-remove" approach - since some folks might be relying on this metric.
What do you think?

Copy link
Contributor

@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

@slim-bean
Copy link
Collaborator Author

Overall LGTM, but I wonder if we shouldn't follow a "first-deprecate-then-remove" approach - since some folks might be relying on this metric. What do you think?

This is a great question to ask, but in this case I'd prefer to just stop the bleeding. We've had user and customer complaints about the significant number of series this generates. Internally in our ops infra it's responsible for 750,000 active series, putting it at the 14th highest cardinality metric in our infra.

Combine this with myself having never actually used this histogram, and I don't know of any dashboards we have using it either. I think this is a pretty good example of something we could trim that would be a great gain for everyone with really minimal impact.

I am sorry for anyone using this and if that's you, please open an issue and let's see if there are alternatives. But in the meantime I'd like to have our next release just drop this metric and save everyone some active series.

@slim-bean slim-bean merged commit 91b0bcf into main Feb 17, 2022
@slim-bean slim-bean deleted the promtail-histogram branch February 17, 2022 12:57
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.

3 participants