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

Migrate to Jakarta Equivalents #959

Merged
merged 9 commits into from
Aug 9, 2022
Merged

Conversation

mglazer
Copy link
Contributor

@mglazer mglazer commented Aug 8, 2022

Before this PR

There was no jakarta equivalent for the libraries, this would prevent tracing-java from being used in modern services which upgrade to the Jakarta equivalents.

After this PR

==COMMIT_MSG==
Introduced jakarta equivalents of all modules which had a javax dependency

Additionally had to do a few things:

  1. Migrated from spotify's ADT to derive4j which generates types without a legacy javax.annotation.Generated type
  2. Fixed a few tests
  3. Upgraded newer libraries to Junit5
  4. Legacy tests were deleted because keeping the versions pinned to the correct transitives was an exercise in futility.
    ==COMMIT_MSG==

Possible downsides?

Mike Glazer added 2 commits August 2, 2022 16:43
Main changes that were outside of the simple scope of this change:

* Migrated from spotify's data library to derive4j. The Spotify
  code generated code with `javax.annotation.Generated` which is
  no longer the correct package name for modern libraries. Derive4j is
  modern.
* Migrated some tests to Junit5 and a newer version of Dropwizard which
  supports the Jakarta prefixes.
@changelog-app
Copy link

changelog-app bot commented Aug 8, 2022

Generate changelog in changelog/@unreleased

Type
See change types. Select one:

  • Feature
  • Improvement
  • Fix
  • Break
  • Deprecation
  • Manual task
  • Migration

Description

Introduced jakarta equivalents of all modules which had a javax dependency

Additionally had to do a few things:

  1. Migrated from spotify's ADT to derive4j which generates types without a legacy javax.annotation.Generated type
  2. Fixed a few tests
  3. Upgraded newer libraries to Junit5

Check the box to generate changelog(s)

  • Generate changelog entry

settings.gradle Outdated Show resolved Hide resolved
…er/migrate-jakarta

* 'develop' of github.com:palantir/tracing-java:
  Excavator:  Upgrades Baseline to the latest version (#958)
  Excavator:  Upgrade gradle wrapper to the latest version (#957)
  Excavator:  Update gradle-jdks infrastructure plugins (#956)
  Excavator:  Upgrade dependencies (#955)
  Excavator:  Upgrades Baseline to the latest version (#953)
  Excavator:  Format Java files (#954)
build.gradle Outdated Show resolved Hide resolved
tracing-servlet/build.gradle Outdated Show resolved Hide resolved
tracing-jaxrs/build.gradle Outdated Show resolved Hide resolved
tracing-jersey-jakarta/build.gradle Outdated Show resolved Hide resolved
tracing-jersey/build.gradle Outdated Show resolved Hide resolved
Copy link
Contributor

@schlosna schlosna left a comment

Choose a reason for hiding this comment

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

dependencies {
api project(":tracing")
api "jakarta.ws.rs:jakarta.ws.rs-api"
api "javax.ws.rs:javax.ws.rs-api"
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we @Deprecate everything in the non-Jakarta modules to flag these for migration?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe I follow up a commit to deprecate everything? I still have at least 6 other libraries that have to migrate first before I could even provide an alternative for someone to migrate to.

import java.io.OutputStream;
import java.util.concurrent.Callable;

public final class JaxRsTracers {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm assuming anything currently using tracing-jaxrs should migrate to tracing-jaxrs-jakarta and we'll deprecate/remove the non-Jakarta implementations in a future major breaking release, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sort of? Generally, people should migrate as gradually more and more things will start breaking, to the point that eventually we can retire the legacy versions of these. But until that happens across all libraries, we can't really say: You should do this.

Does that make sense?

Mike Glazer added 2 commits August 9, 2022 06:30
…-java into mglazer/migrate-jakarta

* 'mglazer/migrate-jakarta' of github.com:palantir/tracing-java:
  Add generated changelog entries
@mglazer
Copy link
Contributor Author

mglazer commented Aug 9, 2022

As for the question on open sourcing the internal plugin, I don't have strong opinions on this. However, given the seemingly endless set of edge cases, we ended up opting for the more discoverable: "Copy/Paste" approach which while a fair bit more work, ends up being easier to maintain in the long term for some of these stable libraries, ie: we can just delete directories wholesale in the future, rather than having to make further modifications to the legacy projects.

What we will very likely end up doing is writing automation to convert projects from the tracing-java-jaxrs to tracing-java-jaxrs-jakarta. That's conceivable, but again, isn't really attainable until we've migrated all upstream libraries first to produce compliant versions.

Copy link
Contributor

@carterkozak carterkozak left a comment

Choose a reason for hiding this comment

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

Great stuff, thank you!

@bulldozer-bot bulldozer-bot bot merged commit 8566954 into develop Aug 9, 2022
@bulldozer-bot bulldozer-bot bot deleted the mglazer/migrate-jakarta branch August 9, 2022 14:15
@svc-autorelease
Copy link
Collaborator

Released 6.13.0

@mglazer
Copy link
Contributor Author

mglazer commented Aug 9, 2022

Also, I realized I mis-spoke since we sort of have 2 jakarta-javax bridge plugins. The one you're thinking of probably makes sense to open source, but I think what I want to do first is add some functionality to it to ensure that component constraints are respected with regards to not having 2 tracing libraries on the classpath (for example).

carterkozak added a commit that referenced this pull request Aug 10, 2022
Revert "Migrate to Jakarta Equivalents (#959)"

This reverts commit 8566954.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants