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.

Sign up for GitHub

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

Jump to bottom

Bump to OTel 1.28 and SR reactive-messaging to 4.9.0 #34851

Merged
merged 1 commit into from
Aug 2, 2023
Merged

Bump to OTel 1.28 and SR reactive-messaging to 4.9.0 #34851

merged 1 commit into from
Aug 2, 2023

Conversation

brunobat
Copy link
Contributor

@brunobat brunobat commented Jul 19, 2023 •
edited
Loading

Depends on the release of SM Reactive messaging after this PR: smallrye/smallrye-reactive-messaging#2227

@quarkus-bot quarkus-bot bot added area/dependencies Pull requests that update a dependency file area/tracing labels Jul 19, 2023
@quarkus-bot
Copy link

quarkus-bot bot commented Jul 19, 2023

/cc @radcortez (opentelemetry)

@geoand
Copy link
Contributor

geoand commented Jul 19, 2023

SM Reactive messaging also has the need for changes but they don't seem to break Quarkus. Will create a PR there as well.

cc @cescoffier

geoand
geoand reviewed Jul 19, 2023
View reviewed changes
Copy link
Contributor

@geoand geoand left a comment

Choose a reason for hiding this comment

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

Awesome!

I only added a small comment.
I'll leave the actual review to @radcortez since I don't have much of the necessary context to do a meaningful review

...pentelemetry/runtime/src/main/java/io/quarkus/opentelemetry/runtime/graal/Substitutions.java Outdated Show resolved Hide resolved
@brunobat brunobat force-pushed the bump-otel-1-28 branch 2 times, most recently from 42b01a4 to abe6e7a Compare July 19, 2023 13:08
@brunobat brunobat mentioned this pull request Jul 19, 2023
Merged
@geoand geoand mentioned this pull request Jul 19, 2023
Closed
@brunobat brunobat marked this pull request as draft July 19, 2023 16:03
@brunobat
Copy link
Contributor Author

This needs more work on code starts and probably native.
Requires the new version of reactive messaging after all.

@geoand
Copy link
Contributor

geoand commented Jul 19, 2023

Ah, bummer...

@quarkus-bot

This comment has been minimized.

Sign in to view
@geoand
Copy link
Contributor

geoand commented Jul 28, 2023

Do we have any chance to get this one in 3.3?
Asking because it would nicely complement the story given all the telemetry work done between 3.2 and 3.3

@brunobat
Copy link
Contributor Author

@geoand it really depends on the SR Reactive messaging release... CC @cescoffier @ozangunalp

@ozangunalp
Copy link
Contributor

@brunobat RM 4.9.0 released so you can bump it in this PR

@brunobat
Copy link
Contributor Author

Thanks @ozangunalp !

@brunobat brunobat marked this pull request as ready for review July 31, 2023 08:43
@brunobat brunobat requested a review from alesj July 31, 2023 08:43
@gastaldi gastaldi added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Jul 31, 2023
@brunobat brunobat changed the title Bump to OTel 1.28 Bump to OTel 1.28 and SR reactive-messaging to 4.9.0 Jul 31, 2023
@brunobat
Copy link
Contributor Author

Build failures seem unrelated. Re-running failed jobs...

@quarkus-bot

This comment has been minimized.

Sign in to view
@quarkus-bot

This comment has been minimized.

Sign in to view
@geoand
Copy link
Contributor

geoand commented Aug 1, 2023

This time the failures do seem related

@brunobat
Bump to OTel 1.28
3033857 
Fix for RM Kafka HR test:
Before RM 4.9.0 Blocking method completion wasn't handled properly. Which lead to this test to be able to abuse and use HR SessionFactory and Panache Reactive in the same message context, one after the other.
This change keeps Panache Reactive using WithTransaction annotation. The Blocking method is kept to verify that message dispatch continues on duplicated context.
@quarkus-bot

This comment has been minimized.

Sign in to view
@brunobat
Copy link
Contributor Author

brunobat commented Aug 1, 2023

Re-running JDK 20 failure because it works on my machine.

@quarkus-bot
Copy link

quarkus-bot bot commented Aug 2, 2023

Failing Jobs - Building 3033857

Status Name Step Failures Logs Raw logs
✔️ Maven Tests - JDK 11
✖ Maven Tests - JDK 11 Windows Build Failures Logs Raw logs

Full information is available in the Build summary check run.

Failures

⚙️ Maven Tests - JDK 11 Windows #

- Failing: integration-tests/maven

📦 integration-tests/maven

✖ io.quarkus.maven.it.DevMojoIT.testResourcesFromClasspath line 1234 - More details - Source on GitHub

org.awaitility.core.ConditionTimeoutException: Condition with io.quarkus.maven.it.DevMojoIT was not fulfilled within 20 seconds.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.CallableCondition.await(CallableCondition.java:78)

✖ io.quarkus.maven.it.DevMojoIT.testResourcesFromClasspath line 1234 - More details - Source on GitHub

org.awaitility.core.ConditionTimeoutException: Condition with io.quarkus.maven.it.DevMojoIT was not fulfilled within 20 seconds.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.CallableCondition.await(CallableCondition.java:78)

@geoand geoand merged commit fa469e3 into quarkusio:main Aug 2, 2023
47 of 48 checks passed
@quarkus-bot quarkus-bot bot removed the triage/waiting-for-ci Ready to merge when CI successfully finishes label Aug 2, 2023
@quarkus-bot quarkus-bot bot added this to the 3.3 - main milestone Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@geoand geoand geoand left review comments

@radcortez radcortez radcortez approved these changes

@alesj alesj Awaiting requested review from alesj

Assignees
No one assigned
Labels
area/dependencies Pull requests that update a dependency file area/tracing
Projects
None yet
Milestone
3.3.0.CR1
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@brunobat @geoand @ozangunalp @radcortez @gastaldi