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

ingester: remove unused loki_ingester_sent_chunks metric #817

Closed

Conversation

rfratto
Copy link
Member

@rfratto rfratto commented Jul 30, 2019

While loki_ingester_sent_chunks was given a value, by the time Prometheus would scrape it, the ingester would have already shut down and would never have a non-zero value.

loki_ingester_received_chunks stores the same value on the receiving end and lives long enough to be picked up by Prometheus.

While loki_ingester_sent_chunks was given a value, by the time
Prometheus would scrape it, the ingester would have already
shut down and would never have a non-zero value.

loki_ingester_received_chunks would store the same value
on the receiving end and lives long enough to be picked
up by Prometheus.
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 should you see if we have to update a dashboard now ?

@rfratto
Copy link
Member Author

rfratto commented Jul 30, 2019

This wasn't in any dashboards since it's brand new so nothing should have to be updated.

However, I talked about this with @tomwilkie and he mentioned that the ingester lifecycler is supposed to sleep for long enough so the metric can be scraped. Seems like that's not happening for us; I need to look into why. Closing.

@rfratto rfratto closed this Jul 30, 2019
@cyriltovena
Copy link
Contributor

@pracucci I think did this change ?

@rfratto
Copy link
Member Author

rfratto commented Jul 30, 2019

It looks like the default wait period is long enough but the server serving the metrics shuts down before anything else so any shutdown metrics would never be scraped. I'll open an issue for this

@pracucci
Copy link
Contributor

@cyriltovena In the PR #784 I've set final_sleep: 0s in the cmd/loki/loki-local-config.yaml. The rationale behind that PR was that - given that's a sample local config file - we should aim for a fast shutdown instead of forcing a 30s (default) sleep on shutdown.

I think the problem described here by @rfratto is different, assuming he's not using the loki-local-config.yaml.

@rfratto
Copy link
Member Author

rfratto commented Jul 30, 2019

@pracucci Yeah, this appears to be caused by #819 and isn't related to your change 🙂

@cyriltovena
Copy link
Contributor

thanks !

@rfratto rfratto deleted the remove-unused-ingester-metric branch July 30, 2019 19:39
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