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

pkg/promtail: Initialize counters to 0 when creating client #1453

Merged
merged 1 commit into from
Jan 13, 2020

Conversation

chancez
Copy link
Contributor

@chancez chancez commented Dec 23, 2019

What this PR does / why we need it:

This ensures metrics are not missing before they're incremented the first
time, as recommended by the upstream Prometheus instrumentation guidelines:
https://prometheus.io/docs/practices/instrumentation/#avoid-missing-metrics.

Currently, in the case of dropped_entries_total and dropped_bytes_total,
the metric is simply missing until the first occurrence of an entry
being dropped.

Checklist

  • Documentation added
  • Tests updated

@chancez chancez force-pushed the promtail_counters branch 2 times, most recently from a237c24 to a2dbb6c Compare December 23, 2019 21:18
Copy link
Contributor

@pracucci pracucci left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! Changes LGTM. I just left a minor comment.

pkg/promtail/client/client.go Outdated Show resolved Hide resolved
This ensures metrics are not missing before they're incremented the first
time, as recommended by the upstream Prometheus instrumentation guidelines:
https://prometheus.io/docs/practices/instrumentation/#avoid-missing-metrics.

Currently, in the case of dropped_entries_total and dropped_bytes_total,
the metric is simply missing until the first occurrence of an entry
being dropped.
@pracucci
Copy link
Contributor

@cyriltovena This PR LGTM.

@cyriltovena cyriltovena merged commit 3a2e5d4 into grafana:master Jan 13, 2020
@chancez chancez deleted the promtail_counters branch January 13, 2020 19:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants