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

Provide Java 9 Module names to all instrumentation projects #9124

Closed
rjbaucells opened this issue Aug 2, 2023 · 10 comments
Closed

Provide Java 9 Module names to all instrumentation projects #9124

rjbaucells opened this issue Aug 2, 2023 · 10 comments
Labels
enhancement New feature or request

Comments

@rjbaucells
Copy link
Contributor

Modern Java applications (JDK 9+) should use modules for encapsulation and easy dependency management. The first step libraries should take (without breaking compatibility with old JDK versions) is providing a module name in the META-INF\MANIFEST.MF file. The level of effort is low and will remove unnecessary warning messages in applications consuming it. e.g.:

[WARNING] ***************************************************************************************************************************************************************************
[WARNING] * Required filename-based automodules detected: [opentelemetry-logback-appender-1.0-1.28.0-alpha.jar]. Please don't publish this project to a public artifact repository! *
[WARNING] ***************************************************************************************************************************************************************************

Using the MANIFEST.MF Automatic-Module-Name header will remove the above warning from consumer applications: e.g.:

Automatic-Module-Name: io.opentelemetry.instrumentation.logback

OpenTelemetry API and SDK already provide this configuration in the MANIFEST.MF files.

I can provide PR for adding such names once the naming convention is agreed upon.

@rjbaucells rjbaucells added the enhancement New feature or request label Aug 2, 2023
@rjbaucells rjbaucells changed the title Provide Java 9 Module names to all integration projects Provide Java 9 Module names to all instrumentation projects Aug 2, 2023
@trask
Copy link
Member

trask commented Aug 2, 2023

hi @rjbaucells! this sounds good, can you review open-telemetry/opentelemetry-java#781 and propose something similar in this repo?

@rjbaucells
Copy link
Contributor Author

Sure, I can do a similar PR in this repo. Now, what should be the module naming convention?

  • io.opentelemetry.instrumentation.log4j.appender
  • io.opentelemetry.instrumentation.logback.appender
  • ...

@trask
Copy link
Member

trask commented Aug 2, 2023

this looks good to me 👍

@trask
Copy link
Member

trask commented Aug 2, 2023

we might want to include the "base version" as well, since we include that in the artifact and package names, so that we can change the supported base version in the future without introducing breaking changes

@rjbaucells
Copy link
Contributor Author

The module name should not contain version, it should follow same guidelines as package name and must be the same from version to version. Two modules cannot export the same package name (JVM will not load the application).

@trask
Copy link
Member

trask commented Aug 2, 2023

we use "base version" to refer to the lowest version of the instrumented library that the instrumenation supports, e.g.

this "base version" is also encoded in the artifact name, e.g.

there's also some more context in #932 (comment)

@rjbaucells
Copy link
Contributor Author

Adding specific examples:

  • instrumentation\log4j\log4j-appender-1.2
  • instrumentation\log4j\log4j-appender-2.17

Could share the same module name io.opentelemetry.instrumentation.log4j.appender since they cannot be loaded together in the same JVM process (both log4j versions export the same package name org.apache.logging.log4j.*). Java module system enforces it.

I can add the same naming convention as the package name... but I think it will bring more problems in the future. Changing a dependency version requires the update of the module-info.java files.

@mateuszrzeszutek
Copy link
Member

The module name should not contain version, it should follow same guidelines as package name and must be the same from version to version. Two modules cannot export the same package name (JVM will not load the application).

The "base version" is not the version of the instrumentation library; it's the minimum supported version of the instrumented library, and we normally encode it as part of the package/artifact name precisely to avoid conflicts. I strongly think that the Automatic-Module-Name should correspond with the gradle module name (or, instrumentation name) and include the base version.

@rjbaucells
Copy link
Contributor Author

OK, I will use the same convention as in the package name.

@laurit
Copy link
Contributor

laurit commented Jan 24, 2025

Resolved with #9140

@laurit laurit closed this as completed Jan 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants