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

Enable instrumentation of Spring EJB clients #11104

Merged
merged 9 commits into from May 10, 2024

Conversation

zackman0010
Copy link
Contributor

Spring's XML configuration allows connecting to a remote EJB through the remote-slsb XML element. This connection utilizes a class in the package org.springframework.ejb.access. This PR instruments the required class.

@zackman0010 zackman0010 requested a review from a team as a code owner April 11, 2024 15:49
Copy link
Member

@trask trask left a comment

Choose a reason for hiding this comment

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

thanks! is it possible to add a test similar to SpringRmiTest.groovy for this?

@zackman0010
Copy link
Contributor Author

Sure, I'll see what I can do!

@zackman0010
Copy link
Contributor Author

zackman0010 commented Apr 17, 2024

@trask I was able to write a test for the remote-slsb Spring XML integration pattern. While I was at it, I also converted the Groovy test to Java for #7195.
The current CI/CD failure appears to be unrelated to my changes, every commit in main from the past day has failed with the same error. I'm currently working on the failure when testing latest dependencies.

The test worked when I ran from IntelliJ, but not when I ran from console. IntelliJ must include all the dependencies on the classpath, so the Spring 6 classes were available there.
The spring-test dependency was only added to use their JNDI mock, but that was removed in Spring 5. With the use of simple-jndi instead, spring-test was no longer required
@zackman0010
Copy link
Contributor Author

@trask Tests are now successful. Turns out the Spring class I used to mock JNDI was removed in Spring 5, so it broke the latest dependencies test with a NoClassDefFoundError. I replaced it with a different JNDI mock library, it's working now.

@zackman0010 zackman0010 requested a review from trask May 8, 2024 15:28
@laurit laurit added this to the v2.4.0 milestone May 10, 2024
@zackman0010
Copy link
Contributor Author

Thanks @laurit! I wasn't able to find a way to dynamically create an EJB at runtime, so it's cool to see that you were able to.

@trask trask merged commit 48d4c13 into open-telemetry:main May 10, 2024
53 checks passed
@zackman0010 zackman0010 deleted the feature/spring_rmi branch May 10, 2024 19:07
zeitlinger pushed a commit to zeitlinger/opentelemetry-java-instrumentation that referenced this pull request May 14, 2024
Co-authored-by: Lauri Tulmin <ltulmin@splunk.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

3 participants