querier: Limit LabelNames; LabelValues to certain time period.

[bwplotka]

Something that Cortex is doing is that LabelNames;LabelValues by default are limited to active series only on ingesters (so let's say in Thanos world to Thanos receiver or Prometheus for latest 2h). This is due to the fact that when you query you are usually interested in autocompletion for fresh metrics.

We are now exploring if we can apply the same logic for Thanos e.g limit LabelNames and LabelValues and Series (by default) on Querier QueryAPI to 2h.

IF we don't do it, StoreGateway will have to load portions of the blocks in memory for full-time retention, for all store APIs, which is something we wanted to avoid. It was briefly mentioned here: #39 Also currently this is limited to raw data only, so it's already scoped for certain setups with retention for older blocks.

[ppanyukov]

Do I understand correctly this does not affect the query execution and any memory limits for that?

[bwplotka]

Which memory limits you have in mind?

[ppanyukov]

The limits on memory taken during query execution, as this PR attempts to address.

#1747

[bwplotka]

Yes, I still don't see how limits improve those methods. This issue is required to be able to have this: #1813

[pracucci]

I would be glad to work on this. Few questions:

• Should the period be configurable?
• Should we introduce a breaking change enforcing a default of 2h, or should we keep an unlimited default just introducing an option to slim it down?

We are now exploring if we can apply the same logic for Thanos e.g limit LabelNames and LabelValues and Series (by default) on Querier QueryAPI to 2h.

FYI, Cortex doesn't enforce any short default for the Series. It currently uses Prometheus defaults, which is the entire history. Do we want to change this in Thanos, and enforce a default of 2h for Series as well?

[bwplotka]

Nice!

Should the period be configurable?

yes

Should we introduce a breaking change enforcing a default of 2h, or should we keep an unlimited default just introducing an option to slim it down?

We can introduce a breaking change if needed, but we need to be explicit in changelog. The question is, should we?

So what makes me think we should have unlimited default, for now: is the fact that if user goes to the dashboard with some templates using those APIs and asks for data month ago it will get label values for active series. Let's say he had eu4-production cluster month ago and now not, he won't be able to see it in his Grafana variables, right? (: I think it's should be a Thanos user choice.

I think simultaneously we should start a discussion about:

• templates https://grafana.com/docs/features/datasources/prometheus/#templating being based on time
• LabelNames/LabelValues being based on time

In terms of series those are already based on time, but we should in similar way define a default if not time range is specified, based on some flag we define: "just introducing an option to slim it down?"

[brancz]

I personally think this should actually even be a query parameter on the API endpoints. I believe there are even issues about this on Prometheus. A Prometheus with many blocks is just as affected by this as Thanos/Cortex are. I feel by default those endpoints should only respond with the active label names/values everywhere.

[davkal]

This is due to the fact that when you query you are usually interested in autocompletion for fresh metrics.

I'd challenge this fact, and I've been bitten by this in post-mortems where I set the time picker in Grafana to the time of incident and then tried to get typeahead suggestions for instances that at the time of querying were long gone.
Currently Cortex's series API ignores start and end, so I don't get the behavior I expect.
The downside would be though that if those non-recent queries would take long (e.g. >2s) then it's kind of pointless anyway for tab completion.

[bwplotka]

I agree with the functionality and latency aspect. As I mentioned:

So what makes me think we should have unlimited default, for now: is the fact that if the user goes to the dashboard with some templates using those APIs and asks for data month ago it will get label values for active series. Let's say he had eu4-production cluster month ago and now not, he won't be able to see it in his Grafana variables, right?

In terms of Thanos there is a clear improvement to be done for series http://github.com/thanos-io/thanos/issues/1822

Overall I think we should be able to return those in a reasonable time from old storage.

@davkal so if Prometheus upstream would have labelNames and labelValues with parameters start and end time, I guess it's trivial to make sure Grafana passes those in e.g dashboards templates? Also, are those APis used in Explore UI as well?

[davkal]

so if Prometheus upstream would have labelNames and labelValues with parameters start and end time, I guess it's trivial to make sure Grafana passes those in e.g dashboards templates? Also, are those APis used in Explore UI as well?

Theoretically yes. But so far it's been more nuanced:

• We're using the label API for the initial request to populate the metrics selector and metrics suggestions in the query field. So far that's unbounded and in Prometheus would return all lifetime labels AFAIK.
• For the label completion we're using the series API because it allows us facetting: returning subsets of possible label spaces as opposed to the whole label space. The series API supports time parameters and we make use of that.
• We have not used the series API for the initial metrics selector requests because an empty series API request would in effect return all series and historically had poor performance (and returned much more data than needed).

When time parameters are supported by the API, it's trivial to plug them in for both dashboards and Explore.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[brancz]

I believe this is still something we want to work on.

[stale]

This issue/PR has been automatically marked as stale because it has not had recent activity. Please comment on status otherwise the issue will be closed in a week. Thank you for your contributions.

[bwplotka]

Still valid.

[bwplotka]

TODO: Propose a new API to Prometheus. cc @rnaveiras

[bwplotka]

Proposed: prometheus/prometheus#6865

[stale]

This issue/PR has been automatically marked as stale because it has not had recent activity. Please comment on status otherwise the issue will be closed in a week. Thank you for your contributions.

[bwplotka]

Still needed (:

[stale]

Hello 👋 Looks like there was no activity on this issue for last 30 days.
Do you mind updating us on the status? Is this still reproducible or needed? If yes, just comment on this PR or push a commit. Thanks! 🤗
If there will be no activity for next week, this issue will be closed (we can always reopen an issue if we need!). Alternatively, use remind command if you wish to be reminded at some point in future.

[bwplotka]

Blocked on Prometheus issue.

…

On Fri, 1 May 2020 at 18:42, stale[bot] ***@***.***> wrote: Hello 👋 Looks like there was no activity on this issue for last 30 days. *Do you mind updating us on the status?* Is this still reproducible or needed? If yes, just comment on this PR or push a commit. Thanks! 🤗 If there will be no activity for next week, this issue will be closed (we can always reopen an issue if we need!). Alternatively, use remind command <https://probot.github.io/apps/reminders/> if you wish to be reminded at some point in future. — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#1811 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABVA3O6DPOZIIOOI2WARVQDRPMCYPANCNFSM4JSUGFDA> .

[stale]

Hello 👋 Looks like there was no activity on this issue for last 30 days.
Do you mind updating us on the status? Is this still reproducible or needed? If yes, just comment on this PR or push a commit. Thanks! 🤗
If there will be no activity for next week, this issue will be closed (we can always reopen an issue if we need!). Alternatively, use remind command if you wish to be reminded at some point in future.

[bwplotka]

This is done on Prometheus, so we can do here as well.

…

On Mon, 1 Jun 2020 at 10:05, stale[bot] ***@***.***> wrote: Hello 👋 Looks like there was no activity on this issue for last 30 days. *Do you mind updating us on the status?* Is this still reproducible or needed? If yes, just comment on this PR or push a commit. Thanks! 🤗 If there will be no activity for next week, this issue will be closed (we can always reopen an issue if we need!). Alternatively, use remind command <https://probot.github.io/apps/reminders/> if you wish to be reminded at some point in future. — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#1811 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABVA3O2MXHBHZ5EJE2UPFWTRUNVNNANCNFSM4JSUGFDA> .

[stale]

Hello 👋 Looks like there was no activity on this issue for last 30 days.
Do you mind updating us on the status? Is this still reproducible or needed? If yes, just comment on this PR or push a commit. Thanks! 🤗
If there will be no activity for next week, this issue will be closed (we can always reopen an issue if we need!). Alternatively, use remind command if you wish to be reminded at some point in future.

[bwplotka]

This was done on Prometheus side, so we can start using it! Help wanted

[stale]

Hello 👋 Looks like there was no activity on this issue for last 30 days.
Do you mind updating us on the status? Is this still reproducible or needed? If yes, just comment on this PR or push a commit. Thanks! 🤗
If there will be no activity for next week, this issue will be closed (we can always reopen an issue if we need!). Alternatively, use remind command if you wish to be reminded at some point in future.

[bwplotka]

The params were added but limit not introduced yet @kakkoyun is working on it.