opentelemetry-collector-contrib icon indicating copy to clipboard operation
opentelemetry-collector-contrib copied to clipboard

[receiver/prometheus] Split target allocator into internal package

Open mx-psi opened this issue 1 year ago • 2 comments

Component(s)

receiver/prometheus

Describe the issue you're reporting

The target allocator from the Prometheus receiver is a part of the codebase that not all contributors may be as familiar with as other parts. In the OpenTelemetry Operator, the target allocator is owned by a separate group of maintainers.

After discussion with the current set of codeowners of the receiver, this issue proposes splitting off this part of the codebase into an internal package (or a separate module under the top-level internal/ folder if there is a possibility of reuse), so that we can find new owners for this part of the codebase and ensure we can appropriately handle PRs and issues related to this part of the code.

cc @dashpole @Aneurysm9

mx-psi avatar May 21 '24 12:05 mx-psi

Pinging code owners:

  • receiver/prometheus: @Aneurysm9 @dashpole

See Adding Labels via Comments if you do not have permissions to add labels yourself.

github-actions[bot] avatar May 21 '24 12:05 github-actions[bot]

I support this change

dashpole avatar May 21 '24 13:05 dashpole

I agree in spirit. The target allocator integration should be owned by the codeowners of the target allocator itself, which in principle includes some combination of @open-telemetry/operator-ta-maintainers and @open-telemetry/operator-maintainers.

I don't know if I like the idea of making it a separate package. It would only have a single consumer, with no real prospects of having more in the future, and would probably be heavily coupled to the prometheus receiver internals anyway. Do we have the option of moving to to a separate Go file, and having different codeowners just for that file?

swiatekm avatar May 23 '24 17:05 swiatekm

I don't know if I like the idea of making it a separate package. It would only have a single consumer, with no real prospects of having more in the future, and would probably be heavily coupled to the prometheus receiver internals anyway. Do we have the option of moving to to a separate Go file, and having different codeowners just for that file?

A separate package could just be a receiver/prometheusreceiver/internal/targetallocator folder, no need for it to be exposed to other modules if there's a single consumer. A single file is also fine (although I suppose at least two files are needed, one of them for testing)

mx-psi avatar May 23 '24 21:05 mx-psi

In my current PoC, it is 2 files + 2 test files, with the configuration being split into a config.go file. I'll push what I have tomorrow if I can

dashpole avatar May 24 '24 00:05 dashpole

https://github.com/open-telemetry/opentelemetry-collector-contrib/pull/33223

dashpole avatar May 24 '24 21:05 dashpole

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.

Pinging code owners:

  • receiver/prometheus: @Aneurysm9 @dashpole

See Adding Labels via Comments if you do not have permissions to add labels yourself.

github-actions[bot] avatar Jul 24 '24 03:07 github-actions[bot]