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

Deduplication labels per tenant #3871

Open
brancz opened this issue Mar 4, 2021 · 11 comments
Open

Deduplication labels per tenant #3871

brancz opened this issue Mar 4, 2021 · 11 comments

Comments

@brancz
Copy link
Member

brancz commented Mar 4, 2021

Is your proposal related to a problem?

Different tenants may choose to label their ingesting Prometheus servers with different labels, Thanos should be flexible enough to allow for this.

Describe the solution you'd like

I'm inclined to allow it per request in order to be able to have different label schemes even within one tenant, on the other hand the same per-tenant deduplication-label would be good to have at offline deduplication time, so a per-tenant configuration could be a good idea as well.

Describe alternatives you've considered

Enforcing tenants to use the same labels for deduplication everywhere. This is not practical I believe as you can't necessarily enforce things like this, and even if it's likely that in the lifetime of running Thanos these might want to change.

@thanos-io/thanos-maintainers

@bwplotka
Copy link
Member

bwplotka commented Mar 4, 2021

Makes sense, thanks for raising. I think this screams for config object instead of flag though 🤔

@brancz
Copy link
Member Author

brancz commented Mar 5, 2021

Did I write flag anywhere?

@LeviHarrison
Copy link
Contributor

@brancz I might take a swing at this if that's OK. I just have a few questions about the implementation:

  • What I've gathered is that the proposal is to allow different deduplication labels per tenant so that when making a query encompassing multiple tenants, everything would be deduplicated properly. Is that correct?

  • So there would be a configuration that looks something like this:

replica_labels:
  - tenant1:
      - replicay
  - tenant2:
      - replica1
      - replicax

And that would be in a file, specified by a flag like --query.replica-label-file=replica_labels.yaml.

  • You also mentioned having an option to do this per-request, which I think might get a little complicated regarding URL parameters, but maybe bending the existing replicaLabels parameter with something along the lines of this:
?replicaLabels=tenant1=replicay&replicaLabels=tenant2=replica1

and then use a regex.

  • I'm a little bit new to Thanos, how would you distinguish between tenants in the querier?

@brancz
Copy link
Member Author

brancz commented Apr 8, 2021

I think I would start with the config and not per request (it could still be done additionally later). I suspect that there will be various per tenant configurations eventually, so I think I would invert the config:

tenants:
- name: tenant1
  replica_labels:
  - replicay
- name: tenant2
  replica_labels:
  - replica1
  - replicax

We would need something like what is depicted in #3822 to identify the tenant of a request (which does not exist yet).

@LeviHarrison
Copy link
Contributor

Ok, so we use the value of the THANOS_TENANT header (or something like that) in a tenant label in the query and then deduplicate the results based on the replica labels specified in the config (if there are any specified). Does that seem right?

Sidenote: is there a specific label used to distinguish tenants, or will that have to be configured also?

@stale
Copy link

stale bot commented Jun 22, 2021

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 stale bot added the stale label Jun 22, 2021
@stale
Copy link

stale bot commented Jul 8, 2021

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

@stale stale bot closed this as completed Jul 8, 2021
@yeya24 yeya24 reopened this Jul 8, 2021
@stale stale bot removed the stale label Jul 8, 2021
@stale
Copy link

stale bot commented Sep 6, 2021

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 stale bot added the stale label Sep 6, 2021
@onprem onprem removed the stale label Sep 11, 2021
@stale
Copy link

stale bot commented Jan 9, 2022

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 stale bot added the stale label Jan 9, 2022
@GiedriusS GiedriusS removed the stale label Jan 10, 2022
@stale
Copy link

stale bot commented Apr 17, 2022

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 stale bot added the stale label Apr 17, 2022
@GiedriusS GiedriusS removed the stale label Apr 17, 2022
@stale
Copy link

stale bot commented Oct 1, 2022

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 stale bot added the stale label Oct 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants