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

Add jakarta namespaced jaxws instrumentation #9569

Open
philsttr opened this issue Sep 28, 2023 · 1 comment
Open

Add jakarta namespaced jaxws instrumentation #9569

philsttr opened this issue Sep 28, 2023 · 1 comment
Labels
contribution welcome Request makes sense, maintainers probably won't have time, contribution would be welcome enhancement New feature or request new instrumentation

Comments

@philsttr
Copy link
Contributor

Is your feature request related to a problem? Please describe.

Currently, only the javax namespaced jaxws is instrumented (javax.jws-api 1.1 and javax jaxws-api 2.0).

After apps migrate to the equivalent jakarta namespaced artifacts, they lose their instrumentation, since the equivalent jakarta namespaces are not currently instrumented.

Describe the solution you'd like

I would like new instrumentation for the jakarta namespaced jakarta.jws-api 3.0 and jakarta.xml.ws-api-3.0 to behave similar to the existing javax namespaced javax.jws-api 1.1 and javax jaxws-api 2.0

Describe alternatives you've considered

No alternatives, other than apps staying in the javax namespace.

Additional context

Associated maven coordinates of related artifacts

javax jakarta
javax.jws:javax.jws-api:1.1 jakarta.jws:jakarta.jws-api:3.0.0
javax.xml.ws:jaxws-api:2.0 jakarta.xml.ws:jakarta.xml.ws-api:3.0.0
@philsttr philsttr added enhancement New feature or request needs triage New issue that requires triage labels Sep 28, 2023
@mateuszrzeszutek mateuszrzeszutek added contribution welcome Request makes sense, maintainers probably won't have time, contribution would be welcome new instrumentation and removed needs triage New issue that requires triage labels Oct 9, 2023
@mateuszrzeszutek
Copy link
Member

Thanks for filing this @philsttr . We'd welcome a contribution for that.

philsttr added a commit to philsttr/opentelemetry-java-instrumentation that referenced this issue Oct 18, 2023
Previously, only jaxws metro 2.2 was instrumented, which only worked with Java EE (javax namespace).
Now, jaxws metro 3.0+ is also instrumented, which works with Jakarta EE (jakarta namespace).

Rather than copy/pasting the metro 2.2 instrumentation implementation to a new metro 3.0 module,
I made the existing module able to work with both Java EE and Jakarta EE classes.

More specifically, I
* moved the instrumentation implementation from `jaxws-2.0-metro-2.2` to `jaxws-metro-2.2` since "jaxws-2.0" is specific to Java EE.
  * Renamed `MetroServerSpanNaming` to `MetroServerSpanNameUpdater`, and made it work with both Java EE and Jakarta EE based on what is available on the runtime classpath.
* moved the Java EE specific tests from `jaxws-2.0-metro-2.2` to `jaxws-2.0-metro-2.2-testing`
* added new Jakarta EE specific tests in `jaxws-3.0-metro-2.2-testing`.
  This is basically a copy/paste of the tests from `jaxws-2.0-metro-2.2-testing`, but using Jakarta EE namespacing.
* added the `jaxws-3.0-common-testing` module for reusable Jakarta EE tests for other future jaxws instrumentation implementations.
  This is basically a copy/paste of `jaxws-2.0-common-testing`, but using Jakarta EE namespacing.

Regarding the implementation of `MetroServerSpanNameUpdater`, I originally added compile time dependencies on both `jakarta.servlet-api` and `javax.servlet-api`, and referenced both of their classes directly in the code, although guarded to make sure things would still work if they weren't on the runtime classpath.  Unfortunately, muzzle would disable the instrumentation if either was not found at runtime.  Therefore, I had to refactor the code a bit to only use reflection to access the classes.  This way muzzle won't disable the instrumentation if either is not found.

This is related to open-telemetry#9569, but specific to the metro runtime, rather than at the annotated `@WebService` level.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution welcome Request makes sense, maintainers probably won't have time, contribution would be welcome enhancement New feature or request new instrumentation
Projects
None yet
Development

No branches or pull requests

2 participants