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

[prometheusremotewriteexporter] multi-tenancy support based on labels #7945

Closed
foxlegend opened this issue Feb 16, 2022 · 12 comments
Closed
Labels

Comments

@foxlegend
Copy link

Is your feature request related to a problem? Please describe.

Cortex and Thanos support multitenancy using a HTTP header (such as "X-Scope-OrgID") or maybe a query parameter (I am thinking about Thanos, but I am not 100% sure about that)..
From my understanding, with the current implementation of the prometheusremotewrite exporter, we should configure an exporter per tenant (by setting the header or the endpoint url).

That means each time we want to add a tenant we should modify collector’s configuration, and we should be aware of all tenants of our infrastructure (I want to say exhaustive).

Describe the solution you'd like

A tenant may add (or through an intermediate collector) a label to the metrics representing its name (for instance tenant="123abc"), and the collector may automatically configure the header or the query param accordingly.

Describe alternatives you've considered

There are some projects such as https://github.com/blind-oracle/cortex-tenant which does the same.
I think that instead of adding a new component to the workflow the collector/prometheusremotewrite exporter may perform the same.

Additional context

I have a working implementation on my fork: main...foxlegend:prometheusremotewriteexporter-multi-tenant

It allows to specify the header to insert and the label to use as tenant name.

@foxlegend foxlegend changed the title PrometheusRemoteWriteExporter multi-tenancy support based on labels [prometheusremotewriteexporter] multi-tenancy support based on labels Feb 16, 2022
@jpkrohling
Copy link
Member

cc @Aneurysm9, @gouthamve

@Aneurysm9
Copy link
Member

This seems reasonable to me. We should probably include warnings about the risk of trusting user-provided labels for routing/billing purposes, but I assume that most people configuring this would be doing it in a context where they're operating a Cortex instance for multiple teams in their org and have the ability to hunt down and apply a clue-by-four to offenders :) Just a note that this is not real AuthN/Z is probably sufficient.

@codeboten codeboten added the comp:prometheus Prometheus related issues label Feb 25, 2022
@politician
Copy link

Possible solution posted here

@kovrus
Copy link
Member

kovrus commented Aug 11, 2022

Hi @foxlegend, can you take a look at #12892 would that address your use case?

@github-actions
Copy link
Contributor

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

@github-actions github-actions bot added the Stale label Nov 10, 2022
@kovrus
Copy link
Member

kovrus commented Nov 16, 2022

it can be closed.

@foxlegend
Copy link
Author

foxlegend commented Nov 16, 2022

I am sorry for the late reply.

In fact, I was not able to use the extension, as I expected to use a label instead of context metadata to send the metric to a specific tenant, as what https://github.com/blind-oracle/cortex-tenant proposes.

If you have any hint about how to do this, it would be really helpful.

It’s more simple for us to use a label, than populating the context metadata.
In fact, we are using a custom receiver for OpenTSDB which metrics name contains the tenant name, and then we are using metricstransformprocessor to extract the tenant name to a label, and finally export the metric to our Cortex backend.

@salvovitale
Copy link

@foxlegend any further development on this topic?

@github-actions github-actions bot removed the Stale label May 1, 2023
@Aneurysm9
Copy link
Member

Would it make sense to have a processor that can take labels from a metric and add them to context metadata? That would seem to provide a general solution to this issue that doesn't need to be tied to the PRW exporter.

@github-actions
Copy link
Contributor

github-actions bot commented Jul 3, 2023

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

@github-actions github-actions bot added the Stale label Jul 3, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Sep 1, 2023

This issue has been closed as inactive because it has been stale for 120 days with no activity.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Sep 1, 2023
@jakubsikorski
Copy link

Reopen. Any update on this? headers setter does not help if you want to set a header based on a label

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

8 participants