Add Extended range selectors proposal

[roidelapluie]

No description provided.

[free]

smoothed works fine in the middle of a time series (i.e. for historical queries). But the fact that it will produce different results in a recording rule vs later on (beyond the usual race conditions that may lead to a couple of missing samples every now and then at the time of rule evaluation) means that it is really only useful for charts and such.

I suppose that's fine, since it's an optional modifier, but I would consider strongly discouraging its use in rules. E.g. if you scrape instance="A" with an offset of 1 second and instance="B" with an offset of 14 seconds (on a scrape interval of 15 seconds), then increase(foo{instance="A"}) will be consistently ~30% higher than increase(foo{instance="B"}) for a foo that increases at the same rate on both instances.

Also, metric smoothed won't even produce an output in real time, since there's never a next sample at the time of evaluation.

[roidelapluie]

Yes, you are correct. In fact, rules would need a offset $scrape_interval to work as correctly as possible, which is something we already see at some locations.

But interpolating would also mean that if we have no new samples and the target is down, we would again extrapolate "wrongly" and invent data that will never happen.

In the same way, I did not want to add heuristics around staleness marker because: 1. They do not mean that counter has stopped, just that we can't get data. 2. Push use cases (like otel or other monitoring systems) will not produce staleness markers.

[free]

Leaving aside the fact that it shifts the resulting timestamps, AFAICT the offset approach is only a best-effort hack. In order for it to work reliably it would have to be something like offset <lookback_window>. An offset <scrape_interval> (or a multiple thereof; also a bit of a hack, BTW, because you'd be hardcoding the scrape interval in your query) breaks if you miss a scrape (or more). Whereas, even with a missing sample(s), the metric did actually experience an increase. So not producing an output under those conditions is very much incorrect behavior.

As for interpolation / extrapolation, I agree with you that they only make up misleading data. E.g. why should it be that a query like increase(foo[1ms]) would produce a non-zero output when foo has orders of magnitude lower resolution? How can anyone tell that the underlying metric actually increased smoothly within that 1 ms? (Vs it staying flat or experiencing the full increase all at once.)

IMHO, one should embrace the fact that the data has the resolution that it does, not whitewash it for the sake of smooth lines on a chart (while at the same time, not generating weird artifacts at the edges or around step increases, as the current implementation does).

[sathieu]

Hi @roidelapluie, I fail to see how this handle the following usecase (see prometheus/prometheus#3806): trivy operator provides a metric for vulnerabilities of a given Kubernetes pod in a given namespace by severity. For a new pod with vulnerabilities, the metric appears with the namespace,pod,severity labels and a non-zero value. How to have an alert for new vulnerabilities (probably per-namespace) with the new semantics?

[beorn7]

Thank you very much for doing this.

I generally like it. I'm not so sure about some edge cases, but we can go forward with an implementation and see how it works in practice. (It will be an experimental feature for a while, I assume, so we can iterate on it.)

I see in your Q&A that you decided to ignore staleness markers. Maybe you are right. I'm not so sure, but see above. I would still recommend to mention the fact that we plan to ignore staleness markers more prominently (where I added comments asking what to do with staleness markers).

The big remaining question is if anchored should work if there is no sample before the range. I was originally thinking it should only return anything if there is actually a sample before the range (or precisely at the beginning of the range). After reading everything, I'm not so sure anymore. I can imagine that the use case I'm often mentioning ("Give my disk usage growth over the last month" – and then I get the growth over the last hour without noticing) will mostly affect gauges, so foo - foo offset 30d will work just fine.

Essentially, I'm willing to give it a go pretty much precisely as you propose it here.

[roidelapluie]

The big remaining question is if anchored should work if there is no sample before the range. I was originally thinking it should only return anything if there is actually a sample before the range (or precisely at the beginning of the range). After reading everything, I'm not so sure anymore. I can imagine that the use case I'm often mentioning ("Give my disk usage growth over the last month" – and then I get the growth over the last hour without noticing) will mostly affect gauges, so foo - foo offset 30d will work just fine.

If you want to know how many HTTP requests you have over 1h:

sum(increase(http_requests_total[1h] anchored))

ignoring metrics without samples before the range would drop a lot of results (all newly created labels, which could be e.g. paths. so it would completely underestimate the result.

The ideal thing is CT: we insert a 0 and so the 0 for newly created labels is reported at the beginning of the range, so we do not miss anything.

[beorn7]

Thanks for addressing my comments. I'm still caught between several events at work and in my usual life, but I will finally be able to tackle my review backlog next week, which will allow me to address the open questions.

In the meantime, it would be good to get some feedback from other interested Prometheus developers. I would much prefer more approvals on this than just mine.

[beorn7]

A few comments are still unaddressed, see my follow-ups.

[beorn7]

I would like to ask for a two-word addition, but otherwise, this LGTM.

Still waiting for the addition about minimum range that you talked about.

[beorn7]

I'm fine with the current state. As a lot of work is already ongoing and none of the team members have raised concerns so far, I would recommend merging this rather sooner than later to get out of this "formally still not approved" state. (The design doc does not have to represent the final implementation. In fact, most non-trivial design docs don't. We can still iterate on implementation details.)

[bwplotka]

Thanks for this; I'm supportive, LGTM

Thanks for considering the CreatedTimestamp feature in the design!

@realschwa would like to double check additionally from the GCM standpoint?

[juliusv]

Just took a first look at this as well. Overall the approach seems well thought through and reasonable, good work! A few editorial comments...

[roidelapluie]

@juliusv I have updated the proposal based on your comments.

[juliusv]

Thanks, looks good now regarding my comments :) No idea if it makes sense to include the anchoring for instant vector selectors, but fine either way.

[roidelapluie]

Thank you all for this. Based on the three approvals, I will merge this. We will probably update this or revisit it later, based on the results we gather from the field.

[ringerc]

Regarding

What about influence from datapoints far outside the query range?

Are ranges always both left-anchored and right-anchored with the anchored keyword? My reading suggests so.

How far will an anchored range look back and/or forward for the anchoring point(s)? The delta lookback (staleness threshold)?

What happens if no anchoring point exists in the series? Will the behaviour modifications discussed for anchored series still apply when no anchoring point is found?

[frittentheke]

Am I right in reading that the observations about handling of e.g. a single missing data point (prometheus/prometheus#16891) is also covered in this proposal here?