Ability to require tenant in queries

[brancz]

Is your proposal related to a problem?

The most questions we get about Thanos are around multi tenancy, and I think it's worth considering that we could have more first class multi tenancy features. This is about the query path.

Describe the solution you'd like

Allow (optionally via a flag) requiring the THANOS_TENANT header on queries against the querier, that enforces the query towards that tenant, rejecting queries that do not specify it.

Describe alternatives you've considered

Just do it entirely outside of Thanos, but as I said in the first sentence, that does not seem like a real possibility to people outside of the maintainer group.

@thanos-io/thanos-maintainers

[kakkoyun]

One other thing to consider is to have a special configurable tenant label. So we can support multi-tenancy more "natively". Store APIs can advertise these. And depending on the level of isolation you require, you can use querier for multiple tenants at the same time or you can create dedicated queries. This special label needs to be considered by any component that exposes StoreAPI.

[brancz]

That's actually really interesting, I had not thought about that! That way we wouldn't even have to parse the PromQL query but just do it at selection time. That indeed makes it much more efficient. I love it! More secure and more efficient! :D

[roidelapluie]

I think it is much easier to use a proxy for authentication that adds a http header than that alters a promql query to add a label.

[brancz]

Yep that's the idea of the first class feature! :)

[bwplotka]

So essentially this proposes embedding prom-label-proxy. 🤔

So I don't think it's needed technically, it works just fine outside. However, agree that there is this perception and configuration complexity that makes people think Thanos is not multi-tenant. I think I would be fine to consider this.

I like the HTTP parameter idea more than fuzzy header. Parameter feels more "typed". I would only suggest looking on other aspects too: Injecting tenant isolation is one thing, but we also need other things like instrumentation (metrics, logs, traces) and limits per tenant, which might be orthogonal to what data tenant is asking for and have access to 🤔 I would vote on defining some strategy for those things.

cc @prmsrswt who is working on cross tenant views for prom-label proxy nowadays.

[roidelapluie]

I think that it is the opposite of the original request, which is about requiring a http header. It would then work with any proxy, and not prom-label-proxy.

[brancz]

As @kakkoyun mentioned, with a tighter integration we could actually push it further down into the store API request and ensure it there, without even having to parse the query.

[bwplotka]

Thoughts on contributor hours from @brancz We can make it more efficient with selectors on already parsed queries etc, no need to work around. More efficient, simpler and more secure.

Other potentially orthogonal discussion:

• Separating data from the environment. metadata, tenancy.
  • Potentially leaking abstractions: It hasn't bitten us much, but it might in the future!
  • List pros and cons
  • What about tenant? Is is infra or data? Kind of both! Meta one!
• Zone aware strict discovery, maybe just file discovery

[bwplotka]

Open question: Header vs Param?

[bwplotka]

As agreed offline, looks like header is what people consistently use for this kind of metadata - we can make sure it's versioned and somehow typed (has proto for it?) as well.

[tomasfreund]

Would it also be possible to expose the tenant/labelset information in the query UI somehow? A drop-down or multi-select perhaps?

One problem I have with Thanos is that while I sometimes want to query data across multiple clusters, I'd prefer if that was not the default and you had to select one/multiple/all tenants/labelsets explicitly to avoid accidentally evaluating the queries in all clusters.

It would also be great if the query UI supported a url like https://my-thanos-instance/{some-tenant-or-labelset-segment}/query as this would allow me to set prometheus --web.external-url to this value and get a useful source url in alertmanager alerts. When I set the parameter to the url of my Thanos instance, I get a working url but it is not filtered by the cluster external_labels and I can't add those to the query because external_labels are applied only when the data is uploaded to object storage and the query is evaluated before that happens.

Anyway, thanks everyone for all the work on this project, aside from these small issues, it's been quite easy to set up and pleasant to work with.

[brancz]

I think exposing it in the UI would be reasonable.

[stale[bot]]

Hello 👋 Looks like there was no activity on this issue for the last two months. 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 in the next two weeks, 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.

[stale[bot]]

Hello 👋 Looks like there was no activity on this issue for the last two months. 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 in the next two weeks, 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.

[stale[bot]]

Closing for now as promised, let us know if you need this to be reopened! 🤗

[stale[bot]]

Hello 👋 Looks like there was no activity on this issue for the last two months. 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 in the next two weeks, 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.

[stale[bot]]

Hello 👋 Looks like there was no activity on this issue for the last two months. 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 in the next two weeks, 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.

[shaardie]

This would be a great feature. I am a bit confused that the PR tackling this is closed https://github.com/thanos-io/thanos/pull/4141

[jasonhao518]

How is going here? looking forward for the same feature also. As grafana stack support X-Scope-OrgID header, I think it's good to use header for Thanos to make it compatible to other tools.

[stale[bot]]

Hello 👋 Looks like there was no activity on this issue for the last two months. 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 in the next two weeks, 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.

[carlosrmendes]

Any news regarding this feature? This is still relevant for me.

Open question: Header vs Param?

I would prefer query parameter

[GiedriusS]

Any news regarding this feature? This is still relevant for me.

Open question: Header vs Param?

I would prefer query parameter

Help wanted :hugs:

[NotAFile]

It looks like this has been completed in #6756, or is there anything else missing?

[GiedriusS]

GOod point @NotAFile, let's close this. I think all requests have been implemented.