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

Improve staleness handling #398

Closed
brian-brazil opened this Issue Jul 15, 2014 · 29 comments

Comments

Projects
None yet
@brian-brazil
Copy link
Member

brian-brazil commented Jul 15, 2014

Currently prometheus considers a timeseries stale when it's got no data points more recent than 5m (configurable).

This causes dead instances' data to hang around for longer than it should, makes less often scrape intervals difficult, makes advanced self-referential use cases more difficult and prevents the timestamp propagating from the push gateway.

I propose that we consider a timeseries stale if it wasn't present in the most recent scrape of the target we get it from.

@discordianfish

This comment has been minimized.

Copy link
Member

discordianfish commented Jul 15, 2014

👍 It seems more reasonable to me to assume that a metric ceased to exists once it's gone from the source. Instead of looking at the metrics itself, the decision whether some metric is 'gone' should be based on the actual scraping results.

I'm personally just affected by the pushgateway issues: My batch job is running every 10 minutes and it pushes a metric with timestamp to the pushgateway. Prometheus scrapes it and can reach it just fine, so one would expect that it persists that data point. But since there are more than 5 minutes between the date points, it considers them as stale.

@juliusv

This comment has been minimized.

Copy link
Member

juliusv commented Jul 15, 2014

👍 in general, but this is hard to implement given the current design. Right now we have no way of tracking which timeseries we got from a target during a previous scrape which are now no longer present (to mark them stale). The scraper sends all scraped samples into a channel which on the other end just get appended to storage.

If you have a good idea how to redesign the current scraping/storage integration to allow for this (efficiently), let me know!

@juliusv

This comment has been minimized.

Copy link
Member

juliusv commented Jul 15, 2014

@discordianfish Regarding your issue with the pushgateway: the expectation is that you normally wouldn't explicitly assign timestamps to pushed samples. That is only for power users. Just send the sample value, and Prometheus will attach a current timestamp to the pushed sample value on every scrape. Of course you need to get used to the semantics then that the timestamp is from the last scrape and not the last push, but this is the expected use case.

@beorn7

This comment has been minimized.

Copy link
Member

beorn7 commented Jul 15, 2014

@discordianfish Yup. The timestamp field is in most cases not what you want. The typical use-case where you want to report something like the time of completion of a batch job via the pushgateway, you would create a metric last_completion_time_seconds and put the Unix time into it as a value. The timestamp in the exchange format really means "scrape time", and you really need to know what you do if you want to manipulate that.

@discordianfish

This comment has been minimized.

Copy link
Member

discordianfish commented Jul 15, 2014

Well, you can't tell whether a metric is up-to-date but just '42' each time or if the thing pushing to the pushgateway stopped working and the '42' is just the latest result. I thought I could just add timestamps for that, raise the stalenessDelta and just alert if the metric is gone. But something like last_completion_time_seconds will do the job as well.

@brian-brazil

This comment has been minimized.

Copy link
Member Author

brian-brazil commented Oct 21, 2015

I was just thinking there from the rate discussion, and have a sketch of a solution.

The two basic cases we want to solve are:

  1. If a pushgateway exports a sample with a timestamp, it should be considered fresh for as long as it exported.
  2. If a target scrape fails or a target is removed, we no longer want to consider it's time series fresh. This is the sum(some_gauge) case.

In addition we want something that'll produce the same results now as back in time, and across Prometheus restarts.

My idea is to add two new values that a sample can have: "fresh" and "stale". These would be persisted like normal samples, but not directly exposed to users.

When we get a scrape with timestamps set on a sample we don't have, we'd add a sample for the exported value, and a 2nd one with the "fresh" value and the scrape time. If we get a sample we already have, then we add a new sample with the "fresh" value and scrape time. When querying for a given time with an instant selector, if we get a "fresh" then we walk back until we get the actual sample. For a range selector we'd ignore that sample.

When a scrape fails, a evaluation no longer produces certain timeseries or a target is removed we'd add in a "stale" on all affected time-series. For an instant selector if the first sample we hit is stale, we stop and return nothing. For a range selector we'd ignore the stale samples, and return the other samples as usual (irate needs special handling here as it's more instant in semantics - could we change it to an instant selector?, and we'll need to be careful with SD blips).

There's various corner cases and details to be worked out, but this seems practical and has the right semantics.

@beorn7

This comment has been minimized.

Copy link
Member

beorn7 commented Oct 23, 2015

When a scrape fails, a evaluation no longer produces certain timeseries or a target is removed we'd add in a "stale" on all affected time-series.

I believe the actual challenge in staleness handling is to identify which time-series are "affected" in the above sense. To do that, we needed to track which target exported which time-series in their respective last scrape. All the updates and lookups required are not trivial to get right and fast, especially in a highly concurrent, high throughput scenario.

@brian-brazil

This comment has been minimized.

Copy link
Member Author

brian-brazil commented Sep 29, 2016

That has similar issues to the difference in values, as the client needs to track scrapes.

@ghost

This comment has been minimized.

Copy link

ghost commented Oct 25, 2016

@brian-brazil

Where is the "staleness configuration" for a timeseries? You mention that it's 5m by default and configurable. I have a system that, currently, can only be scrapped every 4 hours and I want my graphs to have valid data-points for up to that time without gaps.

@beorn7

This comment has been minimized.

Copy link
Member

beorn7 commented Oct 25, 2016

@szxnyc The flag is `--query.staleness-delta``.

@brian-brazil

This comment has been minimized.

Copy link
Member Author

brian-brazil commented Apr 4, 2017

@brian-brazil

This comment has been minimized.

Copy link
Member Author

brian-brazil commented May 16, 2017

The base form of this is in place for scrapes in the dev-2.0 branch if ye'd like to try it out.

@AndreaGiardini

This comment has been minimized.

Copy link
Contributor

AndreaGiardini commented May 17, 2017

@brian-brazil Can you build a new alpha release in order to test it?

@brian-brazil

This comment has been minimized.

Copy link
Member Author

brian-brazil commented May 17, 2017

I believe there's other changes pending before the next alpha. This is only half of staleness handling, recording rules need the same logic added too.

@fabxc

This comment has been minimized.

Copy link
Member

fabxc commented May 17, 2017

@AndreaGiardini sync'd with @brian-brazil again and we concluded the amount of fixes and new experimental features justifies another alpha.
Release is already in progress.

@WIZARD-CXY

This comment has been minimized.

Copy link

WIZARD-CXY commented May 19, 2017

looking forward to the new release

@brian-brazil

This comment has been minimized.

Copy link
Member Author

brian-brazil commented May 23, 2017

Remaining core staleness work is now all out for review.

@brian-brazil

This comment has been minimized.

Copy link
Member Author

brian-brazil commented Jun 6, 2017

This is all implemented in 2.0 with the pgw changes just awaiting a release. Still need docs, but that'll happen with the rest of 2.0 docs.

simonpasquier pushed a commit to simonpasquier/prometheus that referenced this issue Oct 12, 2017

Merge pull request prometheus#398 from prometheus/remove-title-case
Remove title case in exporter and clientlib guidelines.
@alok87

This comment has been minimized.

Copy link

alok87 commented Dec 20, 2017

@brian-brazil I used prometheus/cloudwatch_exporter with below config to collect data for SQS metric. This data takes more than 10mins to refresh (localhost:9106/metrics). Please suggest how can i avoid this.

---
region: ap-south-1
metrics:
- aws_namespace: AWS/SQS
  aws_metric_name: ApproximateNumberOfMessagesVisible
  aws_dimensions: [QueueName]
  aws_statistics: [Average]

Data did not get updated even after more than 10mins(14mins approx)
screen shot 2017-12-20 at 7 10 17 pm
screen shot 2017-12-20 at 7 09 59 pm

@alok87

This comment has been minimized.

Copy link

alok87 commented Dec 20, 2017

The above was because of the cloudwatch gets updated with latest data every 5mins. Also i changed the delay_seconds to 10seconds in cloudwatch_exporter. Now the data gets refreshed in max 5mins.

@lock

This comment has been minimized.

Copy link

lock bot commented Mar 23, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked and limited conversation to collaborators Mar 23, 2019

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.