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

Consider removing Guava #212

Open
abelsromero opened this issue Nov 3, 2021 · 5 comments
Open

Consider removing Guava #212

abelsromero opened this issue Nov 3, 2021 · 5 comments

Comments

@abelsromero
Copy link

In light of recent issues with wavefront-spring-boot maybe it would be good to consider removing guava from dependencies of this project too. This would alleviate packaging size and remove any need of shading.

I checked and it's using:

  • @VisibleForTesting
  • Throwables.getRootCause
  • Maps.newHashMap()
  • Strings.isNullOrEmpty
  • com.google.common.cache.*
  • RateLimiter

All but last 2 I think could easily be replaced by standard components & refactors.
For RateLimiting we could look into resilience4j, for cache I don't have any suggestion. But we could start removing the first ones at least.

@jonatan-ivanov
Copy link

@oppegard We have more and more issues with Guava in Micrometer. :(
What do you think, can it be removed from the Wavefront SDK or at least upgrade it?

@oppegard
Copy link
Contributor

@jonatan-ivanov wavefront-sdk-java v3.2.0 bumped guava to 32.0.1. Will bumping to 32.1.1 in #286 address the micrometer issues for now?

@jonatan-ivanov
Copy link

I think bumping to 32.1.1-jre would help, could you please also make sure that the SDK does not depend on com.google.collections:google-collections or com.google.guava:listenablefuture? See: https://github.com/google/guava/releases/tag/v32.1.0#user-content-overlap

@oppegard
Copy link
Contributor

@jonatan-ivanov I just released v3.2.1 with a bumped guava. I also verified we're not using com.google.collections:google-collections or com.google.guava:listenablefuture. Can you check if v3.2.1 resolves the issue with micrometer?

@jonatan-ivanov
Copy link

Thank you very much!
Older version of Guava had a dependency on listenablefuture but now it is gone, thank you!

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

No branches or pull requests

3 participants