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

otlp: Remove OTel feature gate registration from copied translation package #13932

Merged
merged 2 commits into from May 10, 2024

Conversation

Aneurysm9
Copy link
Contributor

This change removes OTel Collector feature gates registered in copied translation packages. These feature gates are not used in Prometheus and create duplicate registration conflicts if both the OTel and copied translation packages are imported. Due to the state of the individual feature gates and their uses this change has no effect on the behavior of the translation packages.

Copy link
Member

@ArthurSens ArthurSens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a fair change in my point of view, but I'll wait for others involved with OTLP in prometheus to give their opinions as well (it might take a few days because grafana is doing their offsite at the moment)

cc @jesusvazquez , @aknuds1 @gouthamve

@aknuds1
Copy link
Contributor

aknuds1 commented Apr 18, 2024

Yes, we're coming back to this after off-site 🙂

Copy link
Member

@jesusvazquez jesusvazquez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Anthony for bringing this up and solving the feature gates issue!!

We just opened up #13991 stating that prometheus will now own this bits of code and as part of the PR we are removing the README steps and the otel contrib references and updating the licensing.

I hope you don't mind but since this PR is already drifting away @aknuds1 and I think it would make sense to rebase this PR once the other PR is merged.

@jesusvazquez
Copy link
Member

@Aneurysm9 This PR is now unblocked, please rebase your PR

Signed-off-by: Anthony J Mirabella <a9@aneurysm9.com>
Signed-off-by: Anthony J Mirabella <a9@aneurysm9.com>
Signed-off-by: Jesus Vazquez <jesusvzpg@gmail.com>
@jesusvazquez
Copy link
Member

@Aneurysm9 thanks for your PR, I took the liberty of rebasing it to get the latest changes from main.

@jesusvazquez jesusvazquez merged commit 3b8b577 into prometheus:main May 10, 2024
24 of 25 checks passed
kushalShukla-web pushed a commit to kushalShukla-web/prometheus that referenced this pull request May 13, 2024
…ackage (prometheus#13932)

Signed-off-by: Anthony J Mirabella <a9@aneurysm9.com>
Signed-off-by: Jesus Vazquez <jesusvzpg@gmail.com>
Signed-off-by: kushagra Shukla <kushalshukla110@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants