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

Support for Jakarta JMS specification #1353

Merged
merged 1 commit into from
Jan 25, 2023
Merged

Support for Jakarta JMS specification #1353

merged 1 commit into from
Jan 25, 2023

Conversation

reta
Copy link
Contributor

@reta reta commented Dec 9, 2022

Support for Jakarta JMS specification 3.0+. Largely adapted from the existing JMS support.
Closes #1352.

@jcchavezs jcchavezs requested a review from shakuzen December 14, 2022 20:39
@raverone
Copy link

Hi :) Any ideas when this will be merged?

@reta
Copy link
Contributor Author

reta commented Jan 11, 2023

@shakuzen could you please take a look? thank you

@jcchavezs jcchavezs requested a review from jeqo January 13, 2023 23:10
@jcchavezs
Copy link
Contributor

Ping @jeqo

shakuzen
shakuzen approved these changes Jan 16, 2023
View reviewed changes
Copy link
Member

@shakuzen shakuzen left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks @reta for taking on this work. Maybe we need to update the copyright year now that it is 2023? Sorry for the late review.

@raverone
Copy link

raverone commented Jan 16, 2023 •
edited
Loading

Is it okay now to merge the PR? 😅

@reta
Copy link
Contributor Author

reta commented Jan 16, 2023

LGTM. Thanks @reta for taking on this work. Maybe we need to update the copyright year now that it is 2023? Sorry for the late review.

Thanks a lot for review, @shakuzen, technically the copyright year is correct - the change was done in 2022 :)

@jcchavezs
Copy link
Contributor

jcchavezs commented Jan 17, 2023 via email

@raverone
Copy link

Hi, exuse me, but when this PR could be merged? And when new Brave version with this changes will be released?

@jcchavezs
Copy link
Contributor

jcchavezs commented Jan 24, 2023 via email

@jcchavezs jcchavezs merged commit fc72070 into openzipkin:master Jan 25, 2023
@shakuzen shakuzen mentioned this pull request Jan 26, 2023
Merged
@jcchavezs
Copy link
Contributor

jcchavezs commented Jan 27, 2023 via email

@reta
Copy link
Contributor Author

reta commented Jan 27, 2023

Thanks a lot @jcchavezs !

@raverone
Copy link

raverone commented Feb 1, 2023

Hi, thanks for merging this PR.
But the release 5.15.0 is not available in mvnrepository.com.
Is it intentional?
If yes, then could you please tell when it will be available?

@reta
Copy link
Contributor Author

reta commented Feb 1, 2023

If yes, then could you please tell when it will be available?

@raverone the deploy action failed [1] and you are right, artifacts are not published, @jeqo @jcchavezs could you folks please help?

[1] https://github.com/openzipkin/brave/actions/runs/4014424826/jobs/6995827578

@jcchavezs
Copy link
Contributor

jcchavezs commented Feb 1, 2023 via email

@shakuzen
Copy link
Member

shakuzen commented Feb 1, 2023

Indeed the release job failed, but it looks like artifacts were published to central a few hours ago. mvnrepository.com is not the canonical source of what is in maven central, and it tends to take some time for it to sync changes from maven central. You can search maven central directly and see the artifacts there for 5.15.0: https://central.sonatype.com/search?q=io.zipkin.brave

@reta
Copy link
Contributor Author

reta commented Feb 1, 2023

Indeed the release job failed, but it looks like artifacts were published to central a few hours ago. mvnrepository.com is not the canonical source of what is in maven central, and it tends to take some time for it to sync changes from maven central. You can search maven central directly and see the artifacts there for 5.15.0: https://central.sonatype.com/search?q=io.zipkin.brave

Thanks @shakuzen , I confirm that now artifacts are available.

@raverone
Copy link

raverone commented Feb 1, 2023

Yes, thanks guys, the new brave-instrumentation-jakarta-jms is now available, I just included it in my project! 🥳 🙌
Although there still might be one tiny issue: looks like this new artifact is not included in the brave-bom. I was not able to use it without specifying a version. Could you please check this?

@reta
Copy link
Contributor Author

reta commented Feb 1, 2023

Yes, thanks guys, the new brave-instrumentation-jakarta-jms is now available, I just included it in my project! 🥳 🙌 Although there still might be one tiny issue: looks like this new artifact is not included in the brave-bom. I was not able to use it without specifying a version. Could you please check this?

@raverone here we are #1361

@raverone
Copy link

raverone commented Feb 2, 2023

Thanks you all guys, you are the best! 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shakuzen shakuzen shakuzen approved these changes

@jcchavezs jcchavezs jcchavezs approved these changes

@jeqo jeqo Awaiting requested review from jeqo

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Feature] Add support for instrumenting Jakarta JMS
4 participants
@reta @raverone @jcchavezs @shakuzen