-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
exporter/loki
: handle multi-tenant use-cases
#12415
exporter/loki
: handle multi-tenant use-cases
#12415
Conversation
@dashpole, are you available to review this one? |
} | ||
|
||
func (c *Config) validate() error { | ||
if _, err := url.Parse(c.Endpoint); c.Endpoint == "" || err != nil { | ||
return fmt.Errorf("\"endpoint\" must be a valid URL") | ||
} | ||
|
||
if c.Tenant != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens if both Tenant
and TenantID
are used at the same time?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tenant takes precedence, but now that you mention it, I think it should actually at least generate a warning or fail during the Validate step.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like failing. Maybe a little harsh, but mixing deprecated fields and their replacement fields feels like something we should help our users avoid.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR updated
@@ -0,0 +1,4 @@ | |||
change_type: enhancement | |||
component: exporter/loki |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
component: exporter/loki | |
component: lokiexporter |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There actually was a discussion in the past on which format folks preferred, and having the component type followed by the component's name (without the type) was chosen:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh, we should start enforcing that then, the changelog is almost exclusively the other method haha
Fixes #3121 Signed-off-by: Juraci Paixão Kröhling <juraci@kroehling.de>
Signed-off-by: Juraci Paixão Kröhling <juraci@kroehling.de>
Signed-off-by: Juraci Paixão Kröhling <juraci@kroehling.de>
Signed-off-by: Juraci Paixão Kröhling <juraci@kroehling.de>
d3e1dab
to
f9b168f
Compare
This PR deprecates the existing tenant_id field in favor of a new tenant object, with a "source" and "value". The tenant information now may come from a resource attribute, from the context, or as a static value. Current setups using tenant_id are converted internally to "static".
This has been tested with a configuration similar to this:
Configuring a different tenant and a different data source on Grafana confirmed that only logs with the new tenant attribute show up there, now showing up on the first data source.
Fixes #3121
Signed-off-by: Juraci Paixão Kröhling juraci@kroehling.de