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

[v2] How is tenancy handled / configured? Should it be part of storage config? #6108

Open
yurishkuro opened this issue Oct 22, 2024 · 4 comments
Labels
area/storage good first issue Good for beginners help wanted Features that maintainers are willing to accept but do not have cycles to implement v2

Comments

@yurishkuro
Copy link
Member

yurishkuro commented Oct 22, 2024

This might just need some verification / findings, with code links.

@yurishkuro yurishkuro converted this from a draft issue Oct 22, 2024
@yurishkuro yurishkuro added help wanted Features that maintainers are willing to accept but do not have cycles to implement good first issue Good for beginners labels Oct 22, 2024
@yurishkuro yurishkuro changed the title How is tenancy handled / configured in v2? Should it be part of storage config? [v2] How is tenancy handled / configured? Should it be part of storage config? Oct 22, 2024
@shashank-priyadarshi
Copy link

@yurishkuro can I please take this up?

@yurishkuro
Copy link
Member Author

You can try

@mahadzaryab1
Copy link
Collaborator

@yurishkuro do you have any more context on this issue? what are we looking to achieve? what do we want to deliver here?

@yurishkuro
Copy link
Member Author

yurishkuro commented Oct 29, 2024

Here's what I am seeing:

$ rg tenancy cmd/jaeger
cmd/jaeger/internal/extension/jaegerquery/server.go
23:	"github.com/jaegertracing/jaeger/pkg/tenancy"
86:	tm := tenancy.NewManager(&s.config.Tenancy)

Seems query extension is using tenancy properly. But nothing else in v2 does?

Meanwhile, in v1 tenancy has top-level CLI flags:

cmd/query/app/flags.go
73:	Tenancy tenancy.Options `mapstructure:"multi_tenancy"`

cmd/collector/app/flags/flags.go
171:	Tenancy tenancy.Options

Finally, somewhat of a weird thing, grpc storage is the only one that also has tenancy config:

plugin/storage/grpc/config.go
16:	Tenancy                      tenancy.Options `mapstructure:"multi_tenancy"`

So some obvious questions:

  • the handlers in v1 collector are utilizing tenancy, but not in v2, how would we do that?
  • the grpc storage having dedicated tenancy flags makes no sense, we might need to fix in v1 too
  • ultimately tenancy is closely related to storage. In v2 storage management is concentrated in storage extension. Should that be the only place in v2 where tenancy is configurable and then passed around into other components?
  • even with above, how would tenancy headers be parsed by the receivers? In case of gRPC receiver the metadata might still be present by the time control and Context reaches the storage exporter (except batching will mess it up), but for HTTP receivers tenancy header will be lost, unless OTEL has special mechanisms to capture it into Context

Stepping back, multi-tenancy may not work in v2 at all without support from OTEL. There is a relevant discussion in this PR open-telemetry/opentelemetry-collector#2495

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/storage good first issue Good for beginners help wanted Features that maintainers are willing to accept but do not have cycles to implement v2
Projects
Status: TODO
Development

No branches or pull requests

3 participants