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

Fix tracing protocol configuration to only allow grpc #36166

Merged
merged 1 commit into from Jan 14, 2024

Conversation

siewp
Copy link

@siewp siewp commented Sep 26, 2023

Fixes #36084

@quarkus-bot
Copy link

quarkus-bot bot commented Sep 26, 2023

/cc @brunobat (opentelemetry), @radcortez (opentelemetry)

@quarkus-bot

This comment has been minimized.

@gsmet gsmet requested a review from brunobat October 3, 2023 14:46
@gsmet
Copy link
Member

gsmet commented Oct 3, 2023

@brunobat this makes sense to me given the exception we throw but I would prefer if you had a look.

@brunobat
Copy link
Contributor

brunobat commented Oct 9, 2023

@siewp are you going to continue with the PR?

@siewp
Copy link
Author

siewp commented Oct 9, 2023

@siewp are you going to continue with the PR?

Yes, sorry. Had no time until today. I will continue with it later.

@siewp siewp requested a review from brunobat October 11, 2023 05:39
Copy link
Contributor

@brunobat brunobat left a comment

Choose a reason for hiding this comment

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

Thanks @siewp!
@gsmet this should be good to go to the 3.2 branch.

@brunobat
Copy link
Contributor

@siewp can you please squash the into a single commit?

@siewp siewp force-pushed the fix-otel-traces-protocol-config branch from fa6944c to 8d3cde5 Compare October 11, 2023 10:38
Adjust exception message for unsupported tracing exporter protocols

Add test for unsupported otlp exporter protocol config
@siewp siewp force-pushed the fix-otel-traces-protocol-config branch from 8d3cde5 to 7b5f270 Compare October 11, 2023 10:45
@siewp
Copy link
Author

siewp commented Oct 11, 2023

@brunobat Done!

@quarkus-bot
Copy link

quarkus-bot bot commented Oct 11, 2023

Failing Jobs - Building 7b5f270

Status Name Step Failures Logs Raw logs Build scan
✔️ Maven Tests - JDK 11
Maven Tests - JDK 11 Windows Build ⚠️ Check → Logs Raw logs

@siewp
Copy link
Author

siewp commented Nov 9, 2023

@gsmet Is there anything else that needs to be done to merge this PR?

@aloubyansky
Copy link
Member

@brunobat is the equivalent change present in main? Is it still good to merge in 3.2? Thanks!

@brunobat
Copy link
Contributor

brunobat commented Jan 12, 2024

The code in main has diverged after v3.3 (removed backport triage for 3.5), however this PR still makes sense to v3.2 @aloubyansky. CC @gsmet
I'm a bit surprised this is not merged yet.

@gsmet
Copy link
Member

gsmet commented Jan 12, 2024

Pushing things to 3.2 only is not very usual, that's why this somehow felt through the cracks.

@aloubyansky @rsvoboda this one would probably require you having a look.

@gsmet gsmet added this to the 3.2.10.Final milestone Jan 12, 2024
@gsmet
Copy link
Member

gsmet commented Jan 12, 2024

I added the 3.2.10.Final milestone so that we keep it on the radar for consideration for 3.2.

@rsvoboda
Copy link
Member

rsvoboda commented Jan 12, 2024

We had triage with Alexey, that's why he added the comment here. There wasn't any PR to main from @siewp

This was found when looking into PRs created directly against 3.2 (gh pr list --base 3.2)

@brunobat
Copy link
Contributor

yes, main had diverged significantly at that point, not needing this pr. This is because we now have totally different exporters.

@aloubyansky aloubyansky merged commit efe8a1d into quarkusio:3.2 Jan 14, 2024
44 of 45 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants