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

refactor module packages for improved organization and use in modular… #267

Closed
wants to merge 1 commit into from

Conversation

jack-berg
Copy link
Contributor

The package organization of this project is not conducive to usage with a modularized project. Problems include:

  1. The various sub modules of this project all place their classes in com.newrelic.telemetry. This poses problems with modularized projects because the same packages end up being exported from multiple dependencies, which is a no go.
  2. The telemetry-all module attempts to solve this, but falls short in my opinion, since it assumes that you want to use the telemetry-http-java11 implementation instead of telemetry-http-okhttp.
  3. The telemetry-core module produced a fat jar using the shadowJar plugin, and erroneously includes the module-info.class file from its com.google.gson dependency. This causes modularized projects that depend on it to interpret it as a modularized project that only exports com.google.gson! Not good.
  4. As currently configured, you can't have a java 11 modularized project, which depends on a java 8 project, which depends on newrelic-telemetry-sdk-java: The java 8 project has to take dependencies on the java 8 sdk modules (and NOT telemetry-all) telemetry-core, telemetry, and telemetry-http-okhttp. When the java 11 modularized project depends on the java 8 project using standard gradle plugins, it will attempt to create explicit modules for telemetry-core, telemetry, and telemetry-http-okhttp. This will cause a conflict because they all export the same com.newrelic.telemetry package.

This PR fixes all these deficiencies:

  • It renames the telemetry module to telemetry-client, to disambiguate and better reflect its classes purpose.
  • It qualifies the packages of each of the submodules.
    • The root package for telemetry-client is com.newrelic.telemetry.client
    • The root package for telemetry-core is com.newrelic.telemetry.core
    • The root package for telemetry-http-java11 is com.newrelic.telemetry.javahttp
    • The root package for telemetry-http-okhttp is com.newrelic.telemetry.okhttp
  • This allows gradle plugins to properly create explicit modules for each of these that export each's respective root package.
  • The telemetry-all module-info.java has been updated to export the new packages.
  • The telemetry-core shadowJar configuration has been updated to exclude the erroneous module-info.class.

This PR touches many files, which is the nature of package reorganization. However, it is strictly a refactor, as there are zero behavior changes.

@jack-berg
Copy link
Contributor Author

Closing this PR in favor of PR 269, which offers a less intrusive alternative.

The fix to the telemetry-core shadowJar configuration is broken out in PR 268.

@jack-berg jack-berg closed this Jan 22, 2021
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.

1 participant